From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754068AbcEaNnd (ORCPT ); Tue, 31 May 2016 09:43:33 -0400 Received: from mail.kernel.org ([198.145.29.136]:41489 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752935AbcEaNnc (ORCPT ); Tue, 31 May 2016 09:43:32 -0400 Date: Tue, 31 May 2016 10:43:24 -0300 From: Arnaldo Carvalho de Melo To: Taeung Song Cc: linux-kernel@vger.kernel.org, Jiri Olsa , Namhyung Kim , Ingo Molnar , Peter Zijlstra , Alexander Shishkin , Masami Hiramatsu , Jiri Olsa Subject: Re: [PATCH v4 1/6] perf config: Use new perf_config_set__init() to initialize config set Message-ID: <20160531134324.GJ2563@kernel.org> References: <1464657228-24170-1-git-send-email-treeze.taeung@gmail.com> <1464657228-24170-2-git-send-email-treeze.taeung@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1464657228-24170-2-git-send-email-treeze.taeung@gmail.com> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.6.1 (2016-04-27) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Em Tue, May 31, 2016 at 10:13:43AM +0900, Taeung Song escreveu: > Instead of perf_config(), This function initialize config set > collecting all configs from config files (i.e. user config > ~/.perfconfig and system config $(sysconfdir)/perfconfig). > > If there are the same config variable both user and system > config file, user config has higher priority than system config. > > Cc: Namhyung Kim > Cc: Jiri Olsa > Cc: Masami Hiramatsu > Cc: Alexander Shishkin > Signed-off-by: Taeung Song > --- > tools/perf/util/config.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 49 insertions(+), 1 deletion(-) > > diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c > index dad7d82..5d01899 100644 > --- a/tools/perf/util/config.c > +++ b/tools/perf/util/config.c > @@ -645,13 +645,61 @@ out_free: > return -1; > } > > +static int perf_config_set__init(struct perf_config_set *set) > +{ > + int ret = 0, found = 0; > + const char *home = NULL; > + > + /* Setting $PERF_CONFIG makes perf read _only_ the given config file. */ > + if (config_exclusive_filename) > + return perf_config_from_file(collect_config, config_exclusive_filename, set); > + if (perf_config_system() && !access(perf_etc_perfconfig(), R_OK)) { > + ret += perf_config_from_file(collect_config, perf_etc_perfconfig(), set); > + found += 1; > + } > + > + home = getenv("HOME"); > + if (perf_config_global() && home) { > + char *user_config = strdup(mkpath("%s/.perfconfig", home)); > + struct stat st; > + > + if (user_config == NULL) { > + warning("Not enough memory to process %s/.perfconfig, " > + "ignoring it.", home); > + goto out; > + } > + > + if (stat(user_config, &st) < 0) > + goto out_free; > + > + if (st.st_uid && (st.st_uid != geteuid())) { > + warning("File %s not owned by current user or root, " > + "ignoring it.", user_config); > + goto out_free; > + } > + > + if (!st.st_size) > + goto out_free; > + > + ret += perf_config_from_file(collect_config, user_config, set); > + found += 1; > +out_free: > + free(user_config); > + } > +out: > + if (found == 0) > + return -1; > + return ret; > +} > + > struct perf_config_set *perf_config_set__new(void) > { > struct perf_config_set *set = zalloc(sizeof(*set)); > > if (set) { > INIT_LIST_HEAD(&set->sections); > - perf_config(collect_config, set); > + if (perf_config_set__init(set) < 0) > + return NULL; So, the usual pattern is: alloc, init, fail? free, return NULL. I thought you could've been deviating from that pattern and went to look at perf_config_set__init() to see if that was doing the freeing in case of failure, which it shouldn't, it isn't, so I guess this is a leak on failure, no? - Arnaldo