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

[Taeung Song <treeze.taeung@gmail.com>]

On 04/04/2016 09:49 PM, Masami Hiramatsu wrote:
> 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>
>

Thank you !! :-)

Taeung

>
>> 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
>>
>
>