From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1423723AbcE3WUL (ORCPT ); Mon, 30 May 2016 18:20:11 -0400 Received: from mail-pa0-f65.google.com ([209.85.220.65]:33435 "EHLO mail-pa0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1162723AbcE3WUC (ORCPT ); Mon, 30 May 2016 18:20:02 -0400 Subject: Re: [PATCH v3 3/7] perf config: Modify perf_config_set__delete() using global variable 'config_set' To: Arnaldo Carvalho de Melo References: <1464626651-1523-1-git-send-email-treeze.taeung@gmail.com> <1464626651-1523-4-git-send-email-treeze.taeung@gmail.com> <20160530192926.GB2563@kernel.org> Cc: linux-kernel@vger.kernel.org, Jiri Olsa , Namhyung Kim , Ingo Molnar , Peter Zijlstra , Alexander Shishkin , Masami Hiramatsu , Jiri Olsa From: Taeung Song Message-ID: <574CBC8D.3010200@gmail.com> Date: Tue, 31 May 2016 07:19:57 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 In-Reply-To: <20160530192926.GB2563@kernel.org> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, Arnaldo :-D On 05/31/2016 04:29 AM, Arnaldo Carvalho de Melo wrote: > Em Tue, May 31, 2016 at 01:44:07AM +0900, Taeung Song escreveu: >> This function deleted allocated config set but >> the global variable 'config_set' is used all around >> so this directly remove 'config_set' instead of using local variable 'set'. > > Keep it like it is, i.e. foo__delete() acts on something returned by > foo__new(), for consistency. > > We may want to, for instance, have two instances returned by foo__new() > and then call the destructor for each. > I understood it. I'll just keep the function as it is i.e. void perf_config_set__delete(struct perf_config_set *set) , for consistency. And at [PATCH v3 4/7] perf config: Reimplement perf_config() using perf_config_set__iter() when calling the function at run_builtin(), I'll modify it like below. @@ -391,6 +391,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv) perf_env__set_cmdline(&perf_env, argc, argv); status = p->fn(argc, argv, prefix); + perf_config_set__delete(config_set); exit_browser(status); perf_env__exit(&perf_env); bpf__clear(); Thanks, Taeung >> Cc: Namhyung Kim >> Cc: Jiri Olsa >> Cc: Masami Hiramatsu >> Cc: Alexander Shishkin >> Signed-off-by: Taeung Song >> --- >> tools/perf/builtin-config.c | 2 +- >> tools/perf/util/config.c | 8 ++++---- >> tools/perf/util/config.h | 2 +- >> 3 files changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c >> index b3bc01a..255015e 100644 >> --- a/tools/perf/builtin-config.c >> +++ b/tools/perf/builtin-config.c >> @@ -105,7 +105,7 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused) >> usage_with_options(config_usage, config_options); >> } >> >> - perf_config_set__delete(config_set); >> + perf_config_set__delete(); >> out_err: >> return ret; >> } >> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c >> index adf2bad..68def9a 100644 >> --- a/tools/perf/util/config.c >> +++ b/tools/perf/util/config.c >> @@ -642,7 +642,7 @@ static int collect_config(const char *var, const char *value, >> >> out_free: >> free(key); >> - perf_config_set__delete(set); >> + perf_config_set__delete(); >> return -1; >> } >> >> @@ -740,10 +740,10 @@ static void perf_config_set__purge(struct perf_config_set *set) >> } >> } >> >> -void perf_config_set__delete(struct perf_config_set *set) >> +void perf_config_set__delete(void) >> { >> - perf_config_set__purge(set); >> - free(set); >> + perf_config_set__purge(config_set); >> + zfree(&config_set); >> } >> >> /* >> diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h >> index ea157a4..e9b47b3 100644 >> --- a/tools/perf/util/config.h >> +++ b/tools/perf/util/config.h >> @@ -23,6 +23,6 @@ struct perf_config_set { >> extern struct perf_config_set *config_set; >> >> struct perf_config_set *perf_config_set__new(void); >> -void perf_config_set__delete(struct perf_config_set *set); >> +void perf_config_set__delete(void); >> >> #endif /* __PERF_CONFIG_H */ >> -- >> 2.5.0