Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757188AbcDEBzZ (ORCPT ); Mon, 4 Apr 2016 21:55:25 -0400 Received: from mail-pf0-f196.google.com ([209.85.192.196]:36528 "EHLO mail-pf0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755188AbcDEBzL (ORCPT ); Mon, 4 Apr 2016 21:55:11 -0400 Subject: Re: [PATCH v6 2/4] perf config: Let show_config() work with perf_config_set To: Masami Hiramatsu References: <1459761427-16214-1-git-send-email-treeze.taeung@gmail.com> <1459761427-16214-3-git-send-email-treeze.taeung@gmail.com> <20160404214935.1b627c54a7daf74ff12dae4c@kernel.org> Cc: Arnaldo Carvalho de Melo , linux-kernel@vger.kernel.org, Jiri Olsa , Namhyung Kim , Ingo Molnar , Peter Zijlstra From: Taeung Song Message-ID: <57031AEE.9090602@gmail.com> Date: Tue, 5 Apr 2016 10:54:54 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <20160404214935.1b627c54a7daf74ff12dae4c@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 Content-Length: 3908 Lines: 146 On 04/04/2016 09:49 PM, Masami Hiramatsu wrote: > On Mon, 4 Apr 2016 18:17:05 +0900 > Taeung Song wrote: > >> Current show_config() has a problem when user or >> system config files have same config variables i.e. >> >> # cat ~/.perfconfig >> [top] >> children = false >> >> when $(sysconfdir) is /usr/local/etc >> # cat /usr/local/etc/perfconfig >> [top] >> children = true >> >> Before: >> # perf config --user --list >> top.children=false >> >> # perf config --system --list >> top.children=true >> >> # perf config --list >> top.children=true >> top.children=false >> >> Because perf_config() can call show_config() >> each the config file (user and system). >> So fix it. >> >> After: >> # perf config --user --list >> top.children=false >> >> # perf config --system --list >> top.children=true >> >> # perf config --list >> top.children=false >> > > Looks good to me. > > Reviewed-by: Masami Hiramatsu > Thank you !! :-) Taeung > >> Cc: Namhyung Kim >> Cc: Jiri Olsa >> Signed-off-by: Taeung Song >> --- >> tools/perf/builtin-config.c | 35 ++++++++++++++++++++++++++++------- >> 1 file changed, 28 insertions(+), 7 deletions(-) >> >> diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c >> index c42448e..1133224 100644 >> --- a/tools/perf/builtin-config.c >> +++ b/tools/perf/builtin-config.c >> @@ -12,6 +12,7 @@ >> #include >> #include "util/util.h" >> #include "util/debug.h" >> +#include "util/config.h" >> >> static bool use_system_config, use_user_config; >> >> @@ -32,13 +33,24 @@ static struct option config_options[] = { >> OPT_END() >> }; >> >> -static int show_config(const char *key, const char *value, >> - void *cb __maybe_unused) >> +static int show_config(struct perf_config_set *set) >> { >> - if (value) >> - printf("%s=%s\n", key, value); >> - else >> - printf("%s\n", key); >> + struct perf_config_section *section; >> + struct perf_config_item *item; >> + struct list_head *sections = &set->sections; >> + >> + if (list_empty(sections)) >> + return -1; >> + >> + list_for_each_entry(section, sections, node) { >> + list_for_each_entry(item, §ion->items, node) { >> + char *value = item->value; >> + >> + if (value) >> + printf("%s.%s=%s\n", section->name, >> + item->name, value); >> + } >> + } >> >> return 0; >> } >> @@ -46,6 +58,7 @@ static int show_config(const char *key, const char *value, >> int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused) >> { >> int ret = 0; >> + struct perf_config_set *set; >> char *user_config = mkpath("%s/.perfconfig", getenv("HOME")); >> >> argc = parse_options(argc, argv, config_options, config_usage, >> @@ -63,13 +76,19 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused) >> else if (use_user_config) >> config_exclusive_filename = user_config; >> >> + set = perf_config_set__new(); >> + if (!set) { >> + ret = -1; >> + goto out_err; >> + } >> + >> switch (actions) { >> case ACTION_LIST: >> if (argc) { >> pr_err("Error: takes no arguments\n"); >> parse_options_usage(config_usage, config_options, "l", 1); >> } else { >> - ret = perf_config(show_config, NULL); >> + ret = show_config(set); >> if (ret < 0) { >> const char * config_filename = config_exclusive_filename; >> if (!config_exclusive_filename) >> @@ -83,5 +102,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(set); >> +out_err: >> return ret; >> } >> -- >> 2.5.0 >> > >