Re: [PATCH v6 2/4] perf config: Let show_config() work with perf_config_set

[Masami Hiramatsu <mhiramat@kernel.org>]

On Mon,  4 Apr 2016 18:17:05 +0900
Taeung Song <treeze.taeung@gmail.com> 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 <mhiramat@kernel.org>

Thanks!

> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
> ---
>  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 <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 *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, &section->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
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>