From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1423686AbcE3WXY (ORCPT ); Mon, 30 May 2016 18:23:24 -0400 Received: from mail-pa0-f68.google.com ([209.85.220.68]:33558 "EHLO mail-pa0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1422718AbcE3WXV (ORCPT ); Mon, 30 May 2016 18:23:21 -0400 Subject: Re: [PATCH v3 6/7] perf config: Remove needless code about config set at cmd_config() To: Arnaldo Carvalho de Melo References: <1464626651-1523-1-git-send-email-treeze.taeung@gmail.com> <1464626651-1523-7-git-send-email-treeze.taeung@gmail.com> <20160530193500.GD2563@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: <574CBD54.1070708@gmail.com> Date: Tue, 31 May 2016 07:23:16 +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: <20160530193500.GD2563@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 On 05/31/2016 04:35 AM, Arnaldo Carvalho de Melo wrote: > Em Tue, May 31, 2016 at 01:44:10AM +0900, Taeung Song escreveu: >> show_config() was reimplemented using perf_config() >> so it isn't needed to use perf_config_set__new() at cmd_config(). >> And perf_config_set__delete() isn't needed at cmd_config() because of >> calling the function at run_builtin() when a sub-command finished. >> >> And it isn't also needed to declare 'config_set' as extern variable >> because the variable is only handled at util/config.c from now on. > > But the existing code looks more natural, i.e. before dealing with the > config_set, we try instantiating it, handling errors at constructor > calling time, etc. > > Then, finally, calling the destructor when we don't need it anymore. I understood it. The important thing is when it is instantiated or isn't needed, not where it is handled (i.e. at util/config.c). right ? So we need to use the config set as extern variable and it would be better to keep the existing code at cmd_config() as you said. I'll send v4 with modified code soon ! :-) Thanks, Taeung >> Cc: Namhyung Kim >> Cc: Jiri Olsa >> Cc: Masami Hiramatsu >> Cc: Alexander Shishkin >> Signed-off-by: Taeung Song >> --- >> tools/perf/builtin-config.c | 8 -------- >> tools/perf/util/config.h | 2 -- >> 2 files changed, 10 deletions(-) >> >> diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c >> index cf6c396..412c725 100644 >> --- a/tools/perf/builtin-config.c >> +++ b/tools/perf/builtin-config.c >> @@ -64,12 +64,6 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused) >> else if (use_user_config) >> config_exclusive_filename = user_config; >> >> - config_set = perf_config_set__new(); >> - if (!config_set) { >> - ret = -1; >> - goto out_err; >> - } >> - >> switch (actions) { >> case ACTION_LIST: >> if (argc) { >> @@ -90,7 +84,5 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused) >> usage_with_options(config_usage, config_options); >> } >> >> - perf_config_set__delete(); >> -out_err: >> return ret; >> } >> diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h >> index e9b47b3..be4e603 100644 >> --- a/tools/perf/util/config.h >> +++ b/tools/perf/util/config.h >> @@ -20,8 +20,6 @@ struct perf_config_set { >> struct list_head sections; >> }; >> >> -extern struct perf_config_set *config_set; >> - >> struct perf_config_set *perf_config_set__new(void); >> void perf_config_set__delete(void); >> >> -- >> 2.5.0