Date: Thu, 24 Mar 2016 15:08:03 +0900
From: Namhyung Kim <namhyung@kernel.org>
To: Taeung Song <treeze.taeung@gmail.com>
CC: Arnaldo Carvalho de Melo <acme@kernel.org>, linux-kernel@vger.kernel.org,
        Jiri Olsa <jolsa@kernel.org>, Ingo Molnar <mingo@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCH v3 2/4] perf config: Let show_config() work with
 perf_config_set
Message-ID: <20160324060803.GB32072@sejong>
References: <1458764193-15551-1-git-send-email-treeze.taeung@gmail.com>
 <1458764193-15551-3-git-send-email-treeze.taeung@gmail.com>
MIME-Version: 1.0
In-Reply-To: <1458764193-15551-3-git-send-email-treeze.taeung@gmail.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3692
Lines: 134

On Thu, Mar 24, 2016 at 05:16:31AM +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
> 
> Cc: Jiri Olsa <jolsa@kernel.org>
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung


> ---
>  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..c7cf34f 100644
> --- a/tools/perf/builtin-config.c
> +++ b/tools/perf/builtin-config.c
> @@ -12,6 +12,7 @@
>  #include <subcmd/parse-options.h>
>  #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 *perf_configs)
>  {
> -	if (value)
> -		printf("%s=%s\n", key, value);
> -	else
> -		printf("%s\n", key);
> +	struct perf_config_section *section;
> +	struct perf_config_item *config_item;
> +	struct list_head *sections = &perf_configs->sections;
> +
> +	if (list_empty(sections))
> +		return -1;
> +
> +	list_for_each_entry(section, sections, list) {
> +		list_for_each_entry(config_item, &section->config_items, list) {
> +			char *value = config_item->value;
> +
> +			if (value)
> +				printf("%s.%s=%s\n", section->name,
> +				       config_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 *perf_configs;
>  	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;
>  
> +	perf_configs = perf_config_set__new();
> +	if (!perf_configs) {
> +		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(perf_configs);
>  			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(perf_configs);
> +out_err:
>  	return ret;
>  }
> -- 
> 2.5.0
>