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

[Taeung Song]

On 04/04/2016 09:49 PM, Masami Hiramatsu wrote:
> On Mon,  4 Apr 2016 18:17:05 +0900
> Taeung Song <treeze.taeung@xxxxxxxxx> 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@xxxxxxxxxx>

Thank you !! :-)

Taeung

> > Cc: Namhyung Kim <namhyung@xxxxxxxxxx>
> > Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
> > Signed-off-by: Taeung Song <treeze.taeung@xxxxxxxxx>
> > ---
> >   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