[PATCH 0/6] perf config: Add support for setting and getting functionalities

[PATCH 0/6] perf config: Add support for setting and getting functionalities

[Taeung Song]

Hello, :)

Add setting and getting features to perf-config.

I had worked at the related patchset https://lkml.org/lkml/2016/2/22/38 
But I remake new this patchset for only support for read/write config file.
And There're Namhyung's requests https://lkml.org/lkml/2016/10/24/47.

In particular, I agonized implement way for setting functionality.
I especially wonder other opinions of new perf_config_set__collect()
and bool from_system_config variable.

If someone review this patchset and give me some feedback,
I'd appreciated it. :)

Thanks,
Taeung

Taeung Song (6):
  perf config: Add support for getting config key-value pairs
  perf config: Document examples to get config key-value pairs in man
    page
  perf config: Parse config variable arguments before getting
    functionality
  perf config: Add support for writing configs to a config file
  perf config: Document examples to set config variables with values in
    man page
  perf config: Mark where are config items from (user or system)

 tools/perf/Documentation/perf-config.txt |  35 ++++++++
 tools/perf/builtin-config.c              | 135 ++++++++++++++++++++++++++++++-
 tools/perf/util/config.c                 |  20 +++++
 tools/perf/util/config.h                 |   4 +
 4 files changed, 191 insertions(+), 3 deletions(-)

-- 
2.7.4

[PATCH 1/6] perf config: Add support for getting config key-value pairs

[Taeung Song]

Add a functionality getting specific config key-value pairs.
For the syntax examples,

    perf config [<file-option>] [section.name ...]

e.g. To query config items 'report.queue-size' and 'report.children', do

    # perf config report.queue-size report.children

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Wang Nan <wangnan0@huawei.com>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/builtin-config.c | 40 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 37 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
index e4207a2..df3fa1c 100644
--- a/tools/perf/builtin-config.c
+++ b/tools/perf/builtin-config.c
@@ -17,7 +17,7 @@
 static bool use_system_config, use_user_config;
 
 static const char * const config_usage[] = {
-	"perf config [<file-option>] [options]",
+	"perf config [<file-option>] [options] [section.name ...]",
 	NULL
 };
 
@@ -33,6 +33,36 @@ static struct option config_options[] = {
 	OPT_END()
 };
 
+static int show_spec_config(struct perf_config_set *set, const char *var)
+{
+	struct perf_config_section *section;
+	struct perf_config_item *item;
+
+	if (set == NULL)
+		return -1;
+
+	perf_config_items__for_each_entry(&set->sections, section) {
+		if (prefixcmp(var, section->name) != 0)
+			continue;
+
+		perf_config_items__for_each_entry(&section->items, item) {
+			const char *name = var + strlen(section->name) + 1;
+
+			if (strcmp(name, item->name) == 0) {
+				char *value = item->value;
+
+				if (value) {
+					printf("%s=%s\n", var, value);
+					return 0;
+				}
+			}
+
+		}
+	}
+
+	return 0;
+}
+
 static int show_config(struct perf_config_set *set)
 {
 	struct perf_config_section *section;
@@ -54,7 +84,7 @@ static int show_config(struct perf_config_set *set)
 
 int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
 {
-	int ret = 0;
+	int i, ret = 0;
 	struct perf_config_set *set;
 	char *user_config = mkpath("%s/.perfconfig", getenv("HOME"));
 
@@ -100,7 +130,11 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
 		}
 		break;
 	default:
-		usage_with_options(config_usage, config_options);
+		if (argc)
+			for (i = 0; argv[i]; i++)
+				ret = show_spec_config(set, argv[i]);
+		else
+			usage_with_options(config_usage, config_options);
 	}
 
 	perf_config_set__delete(set);
-- 
2.7.4

Re: [PATCH 1/6] perf config: Add support for getting config key-value pairs

[Arnaldo Carvalho de Melo]

Em Fri, Nov 04, 2016 at 03:44:17PM +0900, Taeung Song escreveu:
> Add a functionality getting specific config key-value pairs.
> For the syntax examples,
> 
>     perf config [<file-option>] [section.name ...]
> 
> e.g. To query config items 'report.queue-size' and 'report.children', do
> 
>     # perf config report.queue-size report.children

So, I'm applying it, but while testing I noticed that it shows only the
options that were explicitely set:

[acme@jouet linux]$ perf config report.queue-size report.children
report.children=false
[acme@jouet linux]$

Perhaps we should, in a follow up patch, show this instead:

[acme@jouet linux]$ perf config report.queue-size report.children
report.children=false
# report.queue-size=18446744073709551615 # Default, not set in ~/.perfconfig
[acme@jouet linux]$

?

- Arnaldo
 
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Wang Nan <wangnan0@huawei.com>
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
> ---
>  tools/perf/builtin-config.c | 40 +++++++++++++++++++++++++++++++++++++---
>  1 file changed, 37 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
> index e4207a2..df3fa1c 100644
> --- a/tools/perf/builtin-config.c
> +++ b/tools/perf/builtin-config.c
> @@ -17,7 +17,7 @@
>  static bool use_system_config, use_user_config;
>  
>  static const char * const config_usage[] = {
> -	"perf config [<file-option>] [options]",
> +	"perf config [<file-option>] [options] [section.name ...]",
>  	NULL
>  };
>  
> @@ -33,6 +33,36 @@ static struct option config_options[] = {
>  	OPT_END()
>  };
>  
> +static int show_spec_config(struct perf_config_set *set, const char *var)
> +{
> +	struct perf_config_section *section;
> +	struct perf_config_item *item;
> +
> +	if (set == NULL)
> +		return -1;
> +
> +	perf_config_items__for_each_entry(&set->sections, section) {
> +		if (prefixcmp(var, section->name) != 0)
> +			continue;
> +
> +		perf_config_items__for_each_entry(&section->items, item) {
> +			const char *name = var + strlen(section->name) + 1;
> +
> +			if (strcmp(name, item->name) == 0) {
> +				char *value = item->value;
> +
> +				if (value) {
> +					printf("%s=%s\n", var, value);
> +					return 0;
> +				}
> +			}
> +
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  static int show_config(struct perf_config_set *set)
>  {
>  	struct perf_config_section *section;
> @@ -54,7 +84,7 @@ static int show_config(struct perf_config_set *set)
>  
>  int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
>  {
> -	int ret = 0;
> +	int i, ret = 0;
>  	struct perf_config_set *set;
>  	char *user_config = mkpath("%s/.perfconfig", getenv("HOME"));
>  
> @@ -100,7 +130,11 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
>  		}
>  		break;
>  	default:
> -		usage_with_options(config_usage, config_options);
> +		if (argc)
> +			for (i = 0; argv[i]; i++)
> +				ret = show_spec_config(set, argv[i]);
> +		else
> +			usage_with_options(config_usage, config_options);
>  	}
>  
>  	perf_config_set__delete(set);
> -- 
> 2.7.4

Re: [PATCH 1/6] perf config: Add support for getting config key-value pairs

[Taeung Song]

Hi, Arnaldo

Thank you for your reply :)
I was worried lest you don't it..

On 11/15/2016 12:50 AM, Arnaldo Carvalho de Melo wrote:
> Em Fri, Nov 04, 2016 at 03:44:17PM +0900, Taeung Song escreveu:
>> Add a functionality getting specific config key-value pairs.
>> For the syntax examples,
>>
>>     perf config [<file-option>] [section.name ...]
>>
>> e.g. To query config items 'report.queue-size' and 'report.children', do
>>
>>     # perf config report.queue-size report.children
>
> So, I'm applying it, but while testing I noticed that it shows only the
> options that were explicitely set:
>
> [acme@jouet linux]$ perf config report.queue-size report.children
> report.children=false
> [acme@jouet linux]$
>
> Perhaps we should, in a follow up patch, show this instead:
>
> [acme@jouet linux]$ perf config report.queue-size report.children
> report.children=false
> # report.queue-size=18446744073709551615 # Default, not set in ~/.perfconfig
> [acme@jouet linux]$
>
> ?

Yes, I agree it.
I also think we should get not only config info in config files
but also inbuilt default config info.

After this patchset (support config read/wirte) is applied,
I'll send a follow up patch that contains default config array
to show inbuilt default config info as you said!!

Is it OK ?


Thanks,
Taeung

>
>> Cc: Namhyung Kim <namhyung@kernel.org>
>> Cc: Jiri Olsa <jolsa@kernel.org>
>> Cc: Wang Nan <wangnan0@huawei.com>
>> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
>> ---
>>  tools/perf/builtin-config.c | 40 +++++++++++++++++++++++++++++++++++++---
>>  1 file changed, 37 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
>> index e4207a2..df3fa1c 100644
>> --- a/tools/perf/builtin-config.c
>> +++ b/tools/perf/builtin-config.c
>> @@ -17,7 +17,7 @@
>>  static bool use_system_config, use_user_config;
>>
>>  static const char * const config_usage[] = {
>> -	"perf config [<file-option>] [options]",
>> +	"perf config [<file-option>] [options] [section.name ...]",
>>  	NULL
>>  };
>>
>> @@ -33,6 +33,36 @@ static struct option config_options[] = {
>>  	OPT_END()
>>  };
>>
>> +static int show_spec_config(struct perf_config_set *set, const char *var)
>> +{
>> +	struct perf_config_section *section;
>> +	struct perf_config_item *item;
>> +
>> +	if (set == NULL)
>> +		return -1;
>> +
>> +	perf_config_items__for_each_entry(&set->sections, section) {
>> +		if (prefixcmp(var, section->name) != 0)
>> +			continue;
>> +
>> +		perf_config_items__for_each_entry(&section->items, item) {
>> +			const char *name = var + strlen(section->name) + 1;
>> +
>> +			if (strcmp(name, item->name) == 0) {
>> +				char *value = item->value;
>> +
>> +				if (value) {
>> +					printf("%s=%s\n", var, value);
>> +					return 0;
>> +				}
>> +			}
>> +
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>  static int show_config(struct perf_config_set *set)
>>  {
>>  	struct perf_config_section *section;
>> @@ -54,7 +84,7 @@ static int show_config(struct perf_config_set *set)
>>
>>  int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
>>  {
>> -	int ret = 0;
>> +	int i, ret = 0;
>>  	struct perf_config_set *set;
>>  	char *user_config = mkpath("%s/.perfconfig", getenv("HOME"));
>>
>> @@ -100,7 +130,11 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
>>  		}
>>  		break;
>>  	default:
>> -		usage_with_options(config_usage, config_options);
>> +		if (argc)
>> +			for (i = 0; argv[i]; i++)
>> +				ret = show_spec_config(set, argv[i]);
>> +		else
>> +			usage_with_options(config_usage, config_options);
>>  	}
>>
>>  	perf_config_set__delete(set);
>> --
>> 2.7.4

Re: [PATCH 1/6] perf config: Add support for getting config key-value pairs

[Taeung Song]

Good morning!! Arnaldo :)


On 11/15/2016 12:50 AM, Arnaldo Carvalho de Melo wrote:
> Em Fri, Nov 04, 2016 at 03:44:17PM +0900, Taeung Song escreveu:
>> Add a functionality getting specific config key-value pairs.
>> For the syntax examples,
>>
>>     perf config [<file-option>] [section.name ...]
>>
>> e.g. To query config items 'report.queue-size' and 'report.children', do
>>
>>     # perf config report.queue-size report.children
>
> So, I'm applying it, but while testing I noticed that it shows only the
> options that were explicitely set:
>
> [acme@jouet linux]$ perf config report.queue-size report.children
> report.children=false
> [acme@jouet linux]$
>
> Perhaps we should, in a follow up patch, show this instead:
>
> [acme@jouet linux]$ perf config report.queue-size report.children
> report.children=false
> # report.queue-size=18446744073709551615 # Default, not set in ~/.perfconfig
> [acme@jouet linux]$
>
> ?
>
> - Arnaldo

To also show default config values, first of all,
I think we should have default config arrays.

So I sent v9 PATCH mail for default config arrays!
If you review the patchset, I'd appreciate it! :)


Thanks,
Taeung

[tip:perf/core] perf config: Add support for getting config key-value pairs

[tip-bot for Taeung Song]

Commit-ID:  909236083ee58399b371d085fef5cfac9bce3ec8
Gitweb:     http://git.kernel.org/tip/909236083ee58399b371d085fef5cfac9bce3ec8
Author:     Taeung Song <treeze.taeung@gmail.com>
AuthorDate: Fri, 4 Nov 2016 15:44:17 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 14 Nov 2016 12:52:17 -0300

perf config: Add support for getting config key-value pairs

Add a functionality getting specific config key-value pairs.
For the syntax examples,

    perf config [<file-option>] [section.name ...]

e.g. To query config items 'report.queue-size' and 'report.children', do

    # perf config report.queue-size report.children

Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Nambong Ha <over3025@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Cc: Wookje Kwon <aweee0@gmail.com>
Link: http://lkml.kernel.org/r/1478241862-31230-2-git-send-email-treeze.taeung@gmail.com
[ Combined patch with docs update with this one ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Documentation/perf-config.txt | 18 ++++++++++++++
 tools/perf/builtin-config.c              | 40 +++++++++++++++++++++++++++++---
 2 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt
index cb081ac5..1714b0c 100644
--- a/tools/perf/Documentation/perf-config.txt
+++ b/tools/perf/Documentation/perf-config.txt
@@ -8,6 +8,8 @@ perf-config - Get and set variables in a configuration file.
 SYNOPSIS
 --------
 [verse]
+'perf config' [<file-option>] [section.name ...]
+or
 'perf config' [<file-option>] -l | --list
 
 DESCRIPTION
@@ -118,6 +120,22 @@ Given a $HOME/.perfconfig like this:
 		children = true
 		group = true
 
+To query the record mode of call graph, do
+
+	% perf config call-graph.record-mode
+
+If you want to know multiple config key/value pairs, you can do like
+
+	% perf config report.queue-size call-graph.order report.children
+
+To query the config value of sort order of call graph in user config file (i.e. `~/.perfconfig`), do
+
+	% perf config --user call-graph.sort-order
+
+To query the config value of buildid directory in system config file (i.e. `$(sysconf)/perfconfig`), do
+
+	% perf config --system buildid.dir
+
 Variables
 ~~~~~~~~~
 
diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
index e4207a2..df3fa1c 100644
--- a/tools/perf/builtin-config.c
+++ b/tools/perf/builtin-config.c
@@ -17,7 +17,7 @@
 static bool use_system_config, use_user_config;
 
 static const char * const config_usage[] = {
-	"perf config [<file-option>] [options]",
+	"perf config [<file-option>] [options] [section.name ...]",
 	NULL
 };
 
@@ -33,6 +33,36 @@ static struct option config_options[] = {
 	OPT_END()
 };
 
+static int show_spec_config(struct perf_config_set *set, const char *var)
+{
+	struct perf_config_section *section;
+	struct perf_config_item *item;
+
+	if (set == NULL)
+		return -1;
+
+	perf_config_items__for_each_entry(&set->sections, section) {
+		if (prefixcmp(var, section->name) != 0)
+			continue;
+
+		perf_config_items__for_each_entry(&section->items, item) {
+			const char *name = var + strlen(section->name) + 1;
+
+			if (strcmp(name, item->name) == 0) {
+				char *value = item->value;
+
+				if (value) {
+					printf("%s=%s\n", var, value);
+					return 0;
+				}
+			}
+
+		}
+	}
+
+	return 0;
+}
+
 static int show_config(struct perf_config_set *set)
 {
 	struct perf_config_section *section;
@@ -54,7 +84,7 @@ static int show_config(struct perf_config_set *set)
 
 int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
 {
-	int ret = 0;
+	int i, ret = 0;
 	struct perf_config_set *set;
 	char *user_config = mkpath("%s/.perfconfig", getenv("HOME"));
 
@@ -100,7 +130,11 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
 		}
 		break;
 	default:
-		usage_with_options(config_usage, config_options);
+		if (argc)
+			for (i = 0; argv[i]; i++)
+				ret = show_spec_config(set, argv[i]);
+		else
+			usage_with_options(config_usage, config_options);
 	}
 
 	perf_config_set__delete(set);

[PATCH 2/6] perf config: Document examples to get config key-value pairs in man page

[Taeung Song]

Explain how to query particular config items in config file
and how to get several config items from user or system config file
using '--user' or '--system' options.

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Wang Nan <wangnan0@huawei.com>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/Documentation/perf-config.txt | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt
index cb081ac5..1714b0c 100644
--- a/tools/perf/Documentation/perf-config.txt
+++ b/tools/perf/Documentation/perf-config.txt
@@ -8,6 +8,8 @@ perf-config - Get and set variables in a configuration file.
 SYNOPSIS
 --------
 [verse]
+'perf config' [<file-option>] [section.name ...]
+or
 'perf config' [<file-option>] -l | --list
 
 DESCRIPTION
@@ -118,6 +120,22 @@ Given a $HOME/.perfconfig like this:
 		children = true
 		group = true
 
+To query the record mode of call graph, do
+
+	% perf config call-graph.record-mode
+
+If you want to know multiple config key/value pairs, you can do like
+
+	% perf config report.queue-size call-graph.order report.children
+
+To query the config value of sort order of call graph in user config file (i.e. `~/.perfconfig`), do
+
+	% perf config --user call-graph.sort-order
+
+To query the config value of buildid directory in system config file (i.e. `$(sysconf)/perfconfig`), do
+
+	% perf config --system buildid.dir
+
 Variables
 ~~~~~~~~~
 
-- 
2.7.4

Re: [PATCH 2/6] perf config: Document examples to get config key-value pairs in man page

[Arnaldo Carvalho de Melo]

Em Fri, Nov 04, 2016 at 03:44:18PM +0900, Taeung Song escreveu:
> Explain how to query particular config items in config file
> and how to get several config items from user or system config file
> using '--user' or '--system' options.

This one should be combined with the previous one, i.e. the patch that
introduces this feature, doing it myself.

- Arnaldo
 
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Wang Nan <wangnan0@huawei.com>
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
> ---
>  tools/perf/Documentation/perf-config.txt | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt
> index cb081ac5..1714b0c 100644
> --- a/tools/perf/Documentation/perf-config.txt
> +++ b/tools/perf/Documentation/perf-config.txt
> @@ -8,6 +8,8 @@ perf-config - Get and set variables in a configuration file.
>  SYNOPSIS
>  --------
>  [verse]
> +'perf config' [<file-option>] [section.name ...]
> +or
>  'perf config' [<file-option>] -l | --list
>  
>  DESCRIPTION
> @@ -118,6 +120,22 @@ Given a $HOME/.perfconfig like this:
>  		children = true
>  		group = true
>  
> +To query the record mode of call graph, do
> +
> +	% perf config call-graph.record-mode
> +
> +If you want to know multiple config key/value pairs, you can do like
> +
> +	% perf config report.queue-size call-graph.order report.children
> +
> +To query the config value of sort order of call graph in user config file (i.e. `~/.perfconfig`), do
> +
> +	% perf config --user call-graph.sort-order
> +
> +To query the config value of buildid directory in system config file (i.e. `$(sysconf)/perfconfig`), do
> +
> +	% perf config --system buildid.dir
> +
>  Variables
>  ~~~~~~~~~
>  
> -- 
> 2.7.4

Re: [PATCH 2/6] perf config: Document examples to get config key-value pairs in man page

[Taeung Song]

On 11/15/2016 12:51 AM, Arnaldo Carvalho de Melo wrote:
> Em Fri, Nov 04, 2016 at 03:44:18PM +0900, Taeung Song escreveu:
>> Explain how to query particular config items in config file
>> and how to get several config items from user or system config file
>> using '--user' or '--system' options.
>
> This one should be combined with the previous one, i.e. the patch that
> introduces this feature, doing it myself.
>

I got it! :)
I'll combine getting functionality and examples getting config info as a 
patch.

Thanks,
Taeung

>
>> Cc: Namhyung Kim <namhyung@kernel.org>
>> Cc: Jiri Olsa <jolsa@kernel.org>
>> Cc: Wang Nan <wangnan0@huawei.com>
>> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
>> ---
>>  tools/perf/Documentation/perf-config.txt | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>>
>> diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt
>> index cb081ac5..1714b0c 100644
>> --- a/tools/perf/Documentation/perf-config.txt
>> +++ b/tools/perf/Documentation/perf-config.txt
>> @@ -8,6 +8,8 @@ perf-config - Get and set variables in a configuration file.
>>  SYNOPSIS
>>  --------
>>  [verse]
>> +'perf config' [<file-option>] [section.name ...]
>> +or
>>  'perf config' [<file-option>] -l | --list
>>
>>  DESCRIPTION
>> @@ -118,6 +120,22 @@ Given a $HOME/.perfconfig like this:
>>  		children = true
>>  		group = true
>>
>> +To query the record mode of call graph, do
>> +
>> +	% perf config call-graph.record-mode
>> +
>> +If you want to know multiple config key/value pairs, you can do like
>> +
>> +	% perf config report.queue-size call-graph.order report.children
>> +
>> +To query the config value of sort order of call graph in user config file (i.e. `~/.perfconfig`), do
>> +
>> +	% perf config --user call-graph.sort-order
>> +
>> +To query the config value of buildid directory in system config file (i.e. `$(sysconf)/perfconfig`), do
>> +
>> +	% perf config --system buildid.dir
>> +
>>  Variables
>>  ~~~~~~~~~
>>
>> --
>> 2.7.4

[PATCH 3/6] perf config: Parse config variable arguments before getting functionality

[Taeung Song]

You can get several config items as below,

    # perf config report.queue-size call-graph.record-mode

but it would be needed to more precisely check arguments,
before show_spec_config() takes over the arguments.
The function would be also used when parse config key-value pairs
arguments in the near future.

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Wang Nan <wangnan0@huawei.com>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/builtin-config.c | 45 +++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 41 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
index df3fa1c..fe253f3 100644
--- a/tools/perf/builtin-config.c
+++ b/tools/perf/builtin-config.c
@@ -82,6 +82,27 @@ static int show_config(struct perf_config_set *set)
 	return 0;
 }
 
+static int parse_config_arg(char *arg, char **var)
+{
+	const char *last_dot = strchr(arg, '.');
+
+	/*
+	 * Since "var" actually contains the section name and the real
+	 * config variable name separated by a dot, we have to know where the dot is.
+	 */
+	if (last_dot == NULL || last_dot == arg) {
+		pr_err("The config variable does not contain a section: %s\n", arg);
+		return -1;
+	}
+	if (!last_dot[1]) {
+		pr_err("The config varible does not contain variable name: %s\n", arg);
+		return -1;
+	}
+
+	*var = arg;
+	return 0;
+}
+
 int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
 {
 	int i, ret = 0;
@@ -130,10 +151,26 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
 		}
 		break;
 	default:
-		if (argc)
-			for (i = 0; argv[i]; i++)
-				ret = show_spec_config(set, argv[i]);
-		else
+		if (argc) {
+			for (i = 0; argv[i]; i++) {
+				char *var, *arg = strdup(argv[i]);
+
+				if (!arg) {
+					pr_err("%s: strdup failed\n", __func__);
+					ret = -1;
+					break;
+				}
+
+				if (parse_config_arg(arg, &var) < 0) {
+					free(arg);
+					ret = -1;
+					break;
+				}
+
+				ret = show_spec_config(set, var);
+				free(arg);
+			}
+		} else
 			usage_with_options(config_usage, config_options);
 	}
 
-- 
2.7.4

[tip:perf/core] perf config: Validate config variable arguments before trying use them

[tip-bot for Taeung Song]

Commit-ID:  36662794bb520be828df8e2f3404264f5e7a7973
Gitweb:     http://git.kernel.org/tip/36662794bb520be828df8e2f3404264f5e7a7973
Author:     Taeung Song <treeze.taeung@gmail.com>
AuthorDate: Fri, 4 Nov 2016 15:44:19 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 14 Nov 2016 12:57:40 -0300

perf config: Validate config variable arguments before trying use them

You can show the values for several config items as below:

    # perf config report.queue-size call-graph.record-mode

but it is necessary to more precisely check arguments, before passing
them to show_spec_config().  This validation function would be also used
when parsing config key-value pairs arguments in the near future.

Committer notes:

Testing it:

  $ perf config bla.
  The config variable does not contain a variable name: bla.
  $ perf config .bla
  The config variable does not contain a section name: .bla
  $ perf config bla.bla
  $

Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Nambong Ha <over3025@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Cc: Wookje Kwon <aweee0@gmail.com>
Link: http://lkml.kernel.org/r/1478241862-31230-4-git-send-email-treeze.taeung@gmail.com
[ Fix some spelling errors ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-config.c | 45 +++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 41 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
index df3fa1c..88a43fe 100644
--- a/tools/perf/builtin-config.c
+++ b/tools/perf/builtin-config.c
@@ -82,6 +82,27 @@ static int show_config(struct perf_config_set *set)
 	return 0;
 }
 
+static int parse_config_arg(char *arg, char **var)
+{
+	const char *last_dot = strchr(arg, '.');
+
+	/*
+	 * Since "var" actually contains the section name and the real
+	 * config variable name separated by a dot, we have to know where the dot is.
+	 */
+	if (last_dot == NULL || last_dot == arg) {
+		pr_err("The config variable does not contain a section name: %s\n", arg);
+		return -1;
+	}
+	if (!last_dot[1]) {
+		pr_err("The config variable does not contain a variable name: %s\n", arg);
+		return -1;
+	}
+
+	*var = arg;
+	return 0;
+}
+
 int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
 {
 	int i, ret = 0;
@@ -130,10 +151,26 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
 		}
 		break;
 	default:
-		if (argc)
-			for (i = 0; argv[i]; i++)
-				ret = show_spec_config(set, argv[i]);
-		else
+		if (argc) {
+			for (i = 0; argv[i]; i++) {
+				char *var, *arg = strdup(argv[i]);
+
+				if (!arg) {
+					pr_err("%s: strdup failed\n", __func__);
+					ret = -1;
+					break;
+				}
+
+				if (parse_config_arg(arg, &var) < 0) {
+					free(arg);
+					ret = -1;
+					break;
+				}
+
+				ret = show_spec_config(set, var);
+				free(arg);
+			}
+		} else
 			usage_with_options(config_usage, config_options);
 	}
 

[PATCH 4/6] perf config: Add support for writing configs to a config file

[Taeung Song]

Add setting feature that can add config variables with their values
to a config file (i.e. user or system config file) or modify
config key-value pairs in a config file.
For the syntax examples,

    perf config [<file-option>] [section.name[=value] ...]

e.g. You can set the ui.show-headers to false with

    # perf config ui.show-headers=false

If you want to add or modify several config items, you can do like

    # perf config annotate.show_nr_jumps=false kmem.default=slab

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Wang Nan <wangnan0@huawei.com>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/builtin-config.c | 66 ++++++++++++++++++++++++++++++++++++++++-----
 tools/perf/util/config.c    |  6 +++++
 tools/perf/util/config.h    |  2 ++
 3 files changed, 68 insertions(+), 6 deletions(-)

diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
index fe253f3..5313702 100644
--- a/tools/perf/builtin-config.c
+++ b/tools/perf/builtin-config.c
@@ -17,7 +17,7 @@
 static bool use_system_config, use_user_config;
 
 static const char * const config_usage[] = {
-	"perf config [<file-option>] [options] [section.name ...]",
+	"perf config [<file-option>] [options] [section.name[=value] ...]",
 	NULL
 };
 
@@ -33,6 +33,37 @@ static struct option config_options[] = {
 	OPT_END()
 };
 
+static int set_config(struct perf_config_set *set, const char *file_name,
+		      const char *var, const char *value)
+{
+	struct perf_config_section *section = NULL;
+	struct perf_config_item *item = NULL;
+	const char *first_line = "# this file is auto-generated.";
+	FILE *fp = fopen(file_name, "w");
+
+	if (!fp)
+		return -1;
+	if (set == NULL)
+		return -1;
+
+	perf_config_set__collect(set, var, value);
+	fprintf(fp, "%s\n", first_line);
+
+	/* overwrite configvariables */
+	perf_config_items__for_each_entry(&set->sections, section) {
+		fprintf(fp, "[%s]\n", section->name);
+
+		perf_config_items__for_each_entry(&section->items, item) {
+			if (item->value)
+				fprintf(fp, "\t%s = %s\n",
+					item->name, item->value);
+		}
+	}
+	fclose(fp);
+
+	return 0;
+}
+
 static int show_spec_config(struct perf_config_set *set, const char *var)
 {
 	struct perf_config_section *section;
@@ -82,7 +113,7 @@ static int show_config(struct perf_config_set *set)
 	return 0;
 }
 
-static int parse_config_arg(char *arg, char **var)
+static int parse_config_arg(char *arg, char **var, char **value)
 {
 	const char *last_dot = strchr(arg, '.');
 
@@ -99,7 +130,21 @@ static int parse_config_arg(char *arg, char **var)
 		return -1;
 	}
 
-	*var = arg;
+	*value = strchr(arg, '=');
+	if (*value == NULL)
+		*var = arg;
+	else if (!strcmp(*value, "=")) {
+		pr_err("The config variable does not contain a value: %s\n", arg);
+		return -1;
+	} else {
+		*value = *value + 1; /* excluding a first character '=' */
+		*var = strsep(&arg, "=");
+		if (*var[0] == '\0') {
+			pr_err("invalid config variable: %s\n", arg);
+			return -1;
+		}
+	}
+
 	return 0;
 }
 
@@ -153,7 +198,8 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
 	default:
 		if (argc) {
 			for (i = 0; argv[i]; i++) {
-				char *var, *arg = strdup(argv[i]);
+				char *var, *value;
+				char *arg = strdup(argv[i]);
 
 				if (!arg) {
 					pr_err("%s: strdup failed\n", __func__);
@@ -161,13 +207,21 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
 					break;
 				}
 
-				if (parse_config_arg(arg, &var) < 0) {
+				if (parse_config_arg(arg, &var, &value) < 0) {
 					free(arg);
 					ret = -1;
 					break;
 				}
 
-				ret = show_spec_config(set, var);
+				if (value == NULL)
+					ret = show_spec_config(set, var);
+				else {
+					const char *config_filename = config_exclusive_filename;
+
+					if (!config_exclusive_filename)
+						config_filename = user_config;
+					ret = set_config(set, config_filename, var, value);
+				}
 				free(arg);
 			}
 		} else
diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index 18dae74..c8fb65d 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -602,6 +602,12 @@ static int collect_config(const char *var, const char *value,
 	return -1;
 }
 
+int perf_config_set__collect(struct perf_config_set *set,
+			     const char *var, const char *value)
+{
+	return collect_config(var, value, set);
+}
+
 static int perf_config_set__init(struct perf_config_set *set)
 {
 	int ret = -1;
diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
index 6f813d4..0fcdb8c 100644
--- a/tools/perf/util/config.h
+++ b/tools/perf/util/config.h
@@ -33,6 +33,8 @@ const char *perf_etc_perfconfig(void);
 
 struct perf_config_set *perf_config_set__new(void);
 void perf_config_set__delete(struct perf_config_set *set);
+int perf_config_set__collect(struct perf_config_set *set,
+			     const char *var, const char *value);
 void perf_config__init(void);
 void perf_config__exit(void);
 void perf_config__refresh(void);
-- 
2.7.4

Re: [PATCH 4/6] perf config: Add support for writing configs to a config file

[Arnaldo Carvalho de Melo]

Em Fri, Nov 04, 2016 at 03:44:20PM +0900, Taeung Song escreveu:
> Add setting feature that can add config variables with their values
> to a config file (i.e. user or system config file) or modify
> config key-value pairs in a config file.
> For the syntax examples,
> 
>     perf config [<file-option>] [section.name[=value] ...]
> 
> e.g. You can set the ui.show-headers to false with
> 
>     # perf config ui.show-headers=false
> 
> If you want to add or modify several config items, you can do like
> 
>     # perf config annotate.show_nr_jumps=false kmem.default=slab

This works, but has some problems, see below:
 
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Wang Nan <wangnan0@huawei.com>
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
> ---
>  tools/perf/builtin-config.c | 66 ++++++++++++++++++++++++++++++++++++++++-----
>  tools/perf/util/config.c    |  6 +++++
>  tools/perf/util/config.h    |  2 ++
>  3 files changed, 68 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
> index fe253f3..5313702 100644
> --- a/tools/perf/builtin-config.c
> +++ b/tools/perf/builtin-config.c
> @@ -17,7 +17,7 @@
>  static bool use_system_config, use_user_config;
>  
>  static const char * const config_usage[] = {
> -	"perf config [<file-option>] [options] [section.name ...]",
> +	"perf config [<file-option>] [options] [section.name[=value] ...]",
>  	NULL
>  };
>  
> @@ -33,6 +33,37 @@ static struct option config_options[] = {
>  	OPT_END()
>  };
>  
> +static int set_config(struct perf_config_set *set, const char *file_name,
> +		      const char *var, const char *value)
> +{
> +	struct perf_config_section *section = NULL;
> +	struct perf_config_item *item = NULL;
> +	const char *first_line = "# this file is auto-generated.";
> +	FILE *fp = fopen(file_name, "w");
> +
> +	if (!fp)
> +		return -1;
> +	if (set == NULL)
> +		return -1;

So, here fp is left open? I'm fixing this...

> +	perf_config_set__collect(set, var, value);
> +	fprintf(fp, "%s\n", first_line);
> +
> +	/* overwrite configvariables */
                         missing space?
> +	perf_config_items__for_each_entry(&set->sections, section) {
> +		fprintf(fp, "[%s]\n", section->name);
> +
> +		perf_config_items__for_each_entry(&section->items, item) {
> +			if (item->value)
> +				fprintf(fp, "\t%s = %s\n",
> +					item->name, item->value);
> +		}
> +	}
> +	fclose(fp);
> +
> +	return 0;
> +}
> +
>  static int show_spec_config(struct perf_config_set *set, const char *var)
>  {
>  	struct perf_config_section *section;
> @@ -82,7 +113,7 @@ static int show_config(struct perf_config_set *set)
>  	return 0;
>  }
>  
> -static int parse_config_arg(char *arg, char **var)
> +static int parse_config_arg(char *arg, char **var, char **value)
>  {
>  	const char *last_dot = strchr(arg, '.');
>  
> @@ -99,7 +130,21 @@ static int parse_config_arg(char *arg, char **var)
>  		return -1;
>  	}
>  
> -	*var = arg;
> +	*value = strchr(arg, '=');
> +	if (*value == NULL)
> +		*var = arg;
> +	else if (!strcmp(*value, "=")) {
> +		pr_err("The config variable does not contain a value: %s\n", arg);
> +		return -1;
> +	} else {
> +		*value = *value + 1; /* excluding a first character '=' */
> +		*var = strsep(&arg, "=");
> +		if (*var[0] == '\0') {
> +			pr_err("invalid config variable: %s\n", arg);
> +			return -1;
> +		}
> +	}
> +
>  	return 0;
>  }
>  
> @@ -153,7 +198,8 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
>  	default:
>  		if (argc) {
>  			for (i = 0; argv[i]; i++) {
> -				char *var, *arg = strdup(argv[i]);
> +				char *var, *value;
> +				char *arg = strdup(argv[i]);
>  
>  				if (!arg) {
>  					pr_err("%s: strdup failed\n", __func__);
> @@ -161,13 +207,21 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
>  					break;
>  				}
>  
> -				if (parse_config_arg(arg, &var) < 0) {
> +				if (parse_config_arg(arg, &var, &value) < 0) {
>  					free(arg);
>  					ret = -1;
>  					break;
>  				}
>  
> -				ret = show_spec_config(set, var);
> +				if (value == NULL)
> +					ret = show_spec_config(set, var);
> +				else {
> +					const char *config_filename = config_exclusive_filename;
> +
> +					if (!config_exclusive_filename)
> +						config_filename = user_config;
> +					ret = set_config(set, config_filename, var, value);
> +				}
>  				free(arg);
>  			}
>  		} else
> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
> index 18dae74..c8fb65d 100644
> --- a/tools/perf/util/config.c
> +++ b/tools/perf/util/config.c
> @@ -602,6 +602,12 @@ static int collect_config(const char *var, const char *value,
>  	return -1;
>  }
>  
> +int perf_config_set__collect(struct perf_config_set *set,
> +			     const char *var, const char *value)
> +{
> +	return collect_config(var, value, set);
> +}
> +
>  static int perf_config_set__init(struct perf_config_set *set)
>  {
>  	int ret = -1;
> diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
> index 6f813d4..0fcdb8c 100644
> --- a/tools/perf/util/config.h
> +++ b/tools/perf/util/config.h
> @@ -33,6 +33,8 @@ const char *perf_etc_perfconfig(void);
>  
>  struct perf_config_set *perf_config_set__new(void);
>  void perf_config_set__delete(struct perf_config_set *set);
> +int perf_config_set__collect(struct perf_config_set *set,
> +			     const char *var, const char *value);
>  void perf_config__init(void);
>  void perf_config__exit(void);
>  void perf_config__refresh(void);
> -- 
> 2.7.4

Re: [PATCH 4/6] perf config: Add support for writing configs to a config file

[Taeung Song]

Thank you for your review.

I have a question at the very bottom
(skip v2 I sent lately or not ?).

On 11/15/2016 01:04 AM, Arnaldo Carvalho de Melo wrote:
> Em Fri, Nov 04, 2016 at 03:44:20PM +0900, Taeung Song escreveu:
>> Add setting feature that can add config variables with their values
>> to a config file (i.e. user or system config file) or modify
>> config key-value pairs in a config file.
>> For the syntax examples,
>>
>>     perf config [<file-option>] [section.name[=value] ...]
>>
>> e.g. You can set the ui.show-headers to false with
>>
>>     # perf config ui.show-headers=false
>>
>> If you want to add or modify several config items, you can do like
>>
>>     # perf config annotate.show_nr_jumps=false kmem.default=slab
>
> This works, but has some problems, see below:
>
>> Cc: Namhyung Kim <namhyung@kernel.org>
>> Cc: Jiri Olsa <jolsa@kernel.org>
>> Cc: Wang Nan <wangnan0@huawei.com>
>> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
>> ---
>>  tools/perf/builtin-config.c | 66 ++++++++++++++++++++++++++++++++++++++++-----
>>  tools/perf/util/config.c    |  6 +++++
>>  tools/perf/util/config.h    |  2 ++
>>  3 files changed, 68 insertions(+), 6 deletions(-)
>>
>> diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
>> index fe253f3..5313702 100644
>> --- a/tools/perf/builtin-config.c
>> +++ b/tools/perf/builtin-config.c
>> @@ -17,7 +17,7 @@
>>  static bool use_system_config, use_user_config;
>>
>>  static const char * const config_usage[] = {
>> -	"perf config [<file-option>] [options] [section.name ...]",
>> +	"perf config [<file-option>] [options] [section.name[=value] ...]",
>>  	NULL
>>  };
>>
>> @@ -33,6 +33,37 @@ static struct option config_options[] = {
>>  	OPT_END()
>>  };
>>
>> +static int set_config(struct perf_config_set *set, const char *file_name,
>> +		      const char *var, const char *value)
>> +{
>> +	struct perf_config_section *section = NULL;
>> +	struct perf_config_item *item = NULL;
>> +	const char *first_line = "# this file is auto-generated.";
>> +	FILE *fp = fopen(file_name, "w");
>> +
>> +	if (!fp)
>> +		return -1;
>> +	if (set == NULL)
>> +		return -1;
>
> So, here fp is left open? I'm fixing this...

Understood. Sorry, I missed out it.

>> +	perf_config_set__collect(set, var, value);
>> +	fprintf(fp, "%s\n", first_line);
>> +
>> +	/* overwrite configvariables */
>                          missing space?

Oops.. I missed a white space between two words.

>> +	perf_config_items__for_each_entry(&set->sections, section) {
>> +		fprintf(fp, "[%s]\n", section->name);
>> +
>> +		perf_config_items__for_each_entry(&section->items, item) {
>> +			if (item->value)
>> +				fprintf(fp, "\t%s = %s\n",
>> +					item->name, item->value);
>> +		}
>> +	}
>> +	fclose(fp);
>> +
>> +	return 0;
>> +}
>> +
>>  static int show_spec_config(struct perf_config_set *set, const char *var)
>>  {
>>  	struct perf_config_section *section;
>> @@ -82,7 +113,7 @@ static int show_config(struct perf_config_set *set)
>>  	return 0;
>>  }
>>
>> -static int parse_config_arg(char *arg, char **var)
>> +static int parse_config_arg(char *arg, char **var, char **value)
>>  {
>>  	const char *last_dot = strchr(arg, '.');
>>
>> @@ -99,7 +130,21 @@ static int parse_config_arg(char *arg, char **var)
>>  		return -1;
>>  	}
>>
>> -	*var = arg;
>> +	*value = strchr(arg, '=');
>> +	if (*value == NULL)
>> +		*var = arg;
>> +	else if (!strcmp(*value, "=")) {
>> +		pr_err("The config variable does not contain a value: %s\n", arg);
>> +		return -1;
>> +	} else {
>> +		*value = *value + 1; /* excluding a first character '=' */
>> +		*var = strsep(&arg, "=");
>> +		if (*var[0] == '\0') {
>> +			pr_err("invalid config variable: %s\n", arg);
>> +			return -1;
>> +		}
>> +	}
>> +

Here and..

>>
>> @@ -153,7 +198,8 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
>>  	default:
>>  		if (argc) {
>>  			for (i = 0; argv[i]; i++) {
>> -				char *var, *arg = strdup(argv[i]);
>> +				char *var, *value;
>> +				char *arg = strdup(argv[i]);
>>
>>  				if (!arg) {
>>  					pr_err("%s: strdup failed\n", __func__);
>> @@ -161,13 +207,21 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
>>  					break;
>>  				}
>>
>> -				if (parse_config_arg(arg, &var) < 0) {
>> +				if (parse_config_arg(arg, &var, &value) < 0) {
>>  					free(arg);
>>  					ret = -1;
>>  					break;
>>  				}
>>
>> -				ret = show_spec_config(set, var);
>> +				if (value == NULL)
>> +					ret = show_spec_config(set, var);
>> +				else {
>> +					const char *config_filename = config_exclusive_filename;
>> +
>> +					if (!config_exclusive_filename)
>> +						config_filename = user_config;
>> +					ret = set_config(set, config_filename, var, value);
>> +				}
>>  				free(arg);

Here, the parts are a bit different than v2 patchset I sent lately.
I refactored parse_config_arg() and a bit modify parsing
config arguments in cmd_config().

Is it better to just skip v2 patchset I sent ?
And remake new patchset regarding today your feedback ?

Or would I make v3 that adopts v2 I sent?


Thanks,
Taeung

>>  			}
>>  		} else
>> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
>> index 18dae74..c8fb65d 100644
>> --- a/tools/perf/util/config.c
>> +++ b/tools/perf/util/config.c
>> @@ -602,6 +602,12 @@ static int collect_config(const char *var, const char *value,
>>  	return -1;
>>  }
>>
>> +int perf_config_set__collect(struct perf_config_set *set,
>> +			     const char *var, const char *value)
>> +{
>> +	return collect_config(var, value, set);
>> +}
>> +
>>  static int perf_config_set__init(struct perf_config_set *set)
>>  {
>>  	int ret = -1;
>> diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
>> index 6f813d4..0fcdb8c 100644
>> --- a/tools/perf/util/config.h
>> +++ b/tools/perf/util/config.h
>> @@ -33,6 +33,8 @@ const char *perf_etc_perfconfig(void);
>>
>>  struct perf_config_set *perf_config_set__new(void);
>>  void perf_config_set__delete(struct perf_config_set *set);
>> +int perf_config_set__collect(struct perf_config_set *set,
>> +			     const char *var, const char *value);
>>  void perf_config__init(void);
>>  void perf_config__exit(void);
>>  void perf_config__refresh(void);
>> --
>> 2.7.4

Re: [PATCH 4/6] perf config: Add support for writing configs to a config file

[Arnaldo Carvalho de Melo]

Em Tue, Nov 15, 2016 at 02:00:38AM +0900, Taeung Song escreveu:
> Thank you for your review.
> 
> I have a question at the very bottom
> (skip v2 I sent lately or not ?).

Humm, I combined some patches, fixed up the stuff I noticed, and pushed
to Ingo, please take a look at that and post patches for anything you
find applicable,

Thanks,

- Arnaldo
 
> On 11/15/2016 01:04 AM, Arnaldo Carvalho de Melo wrote:
> > Em Fri, Nov 04, 2016 at 03:44:20PM +0900, Taeung Song escreveu:
> > > Add setting feature that can add config variables with their values
> > > to a config file (i.e. user or system config file) or modify
> > > config key-value pairs in a config file.
> > > For the syntax examples,
> > > 
> > >     perf config [<file-option>] [section.name[=value] ...]
> > > 
> > > e.g. You can set the ui.show-headers to false with
> > > 
> > >     # perf config ui.show-headers=false
> > > 
> > > If you want to add or modify several config items, you can do like
> > > 
> > >     # perf config annotate.show_nr_jumps=false kmem.default=slab
> > 
> > This works, but has some problems, see below:
> > 
> > > Cc: Namhyung Kim <namhyung@kernel.org>
> > > Cc: Jiri Olsa <jolsa@kernel.org>
> > > Cc: Wang Nan <wangnan0@huawei.com>
> > > Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
> > > ---
> > >  tools/perf/builtin-config.c | 66 ++++++++++++++++++++++++++++++++++++++++-----
> > >  tools/perf/util/config.c    |  6 +++++
> > >  tools/perf/util/config.h    |  2 ++
> > >  3 files changed, 68 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
> > > index fe253f3..5313702 100644
> > > --- a/tools/perf/builtin-config.c
> > > +++ b/tools/perf/builtin-config.c
> > > @@ -17,7 +17,7 @@
> > >  static bool use_system_config, use_user_config;
> > > 
> > >  static const char * const config_usage[] = {
> > > -	"perf config [<file-option>] [options] [section.name ...]",
> > > +	"perf config [<file-option>] [options] [section.name[=value] ...]",
> > >  	NULL
> > >  };
> > > 
> > > @@ -33,6 +33,37 @@ static struct option config_options[] = {
> > >  	OPT_END()
> > >  };
> > > 
> > > +static int set_config(struct perf_config_set *set, const char *file_name,
> > > +		      const char *var, const char *value)
> > > +{
> > > +	struct perf_config_section *section = NULL;
> > > +	struct perf_config_item *item = NULL;
> > > +	const char *first_line = "# this file is auto-generated.";
> > > +	FILE *fp = fopen(file_name, "w");
> > > +
> > > +	if (!fp)
> > > +		return -1;
> > > +	if (set == NULL)
> > > +		return -1;
> > 
> > So, here fp is left open? I'm fixing this...
> 
> Understood. Sorry, I missed out it.
> 
> > > +	perf_config_set__collect(set, var, value);
> > > +	fprintf(fp, "%s\n", first_line);
> > > +
> > > +	/* overwrite configvariables */
> >                          missing space?
> 
> Oops.. I missed a white space between two words.
> 
> > > +	perf_config_items__for_each_entry(&set->sections, section) {
> > > +		fprintf(fp, "[%s]\n", section->name);
> > > +
> > > +		perf_config_items__for_each_entry(&section->items, item) {
> > > +			if (item->value)
> > > +				fprintf(fp, "\t%s = %s\n",
> > > +					item->name, item->value);
> > > +		}
> > > +	}
> > > +	fclose(fp);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > >  static int show_spec_config(struct perf_config_set *set, const char *var)
> > >  {
> > >  	struct perf_config_section *section;
> > > @@ -82,7 +113,7 @@ static int show_config(struct perf_config_set *set)
> > >  	return 0;
> > >  }
> > > 
> > > -static int parse_config_arg(char *arg, char **var)
> > > +static int parse_config_arg(char *arg, char **var, char **value)
> > >  {
> > >  	const char *last_dot = strchr(arg, '.');
> > > 
> > > @@ -99,7 +130,21 @@ static int parse_config_arg(char *arg, char **var)
> > >  		return -1;
> > >  	}
> > > 
> > > -	*var = arg;
> > > +	*value = strchr(arg, '=');
> > > +	if (*value == NULL)
> > > +		*var = arg;
> > > +	else if (!strcmp(*value, "=")) {
> > > +		pr_err("The config variable does not contain a value: %s\n", arg);
> > > +		return -1;
> > > +	} else {
> > > +		*value = *value + 1; /* excluding a first character '=' */
> > > +		*var = strsep(&arg, "=");
> > > +		if (*var[0] == '\0') {
> > > +			pr_err("invalid config variable: %s\n", arg);
> > > +			return -1;
> > > +		}
> > > +	}
> > > +
> 
> Here and..
> 
> > > 
> > > @@ -153,7 +198,8 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
> > >  	default:
> > >  		if (argc) {
> > >  			for (i = 0; argv[i]; i++) {
> > > -				char *var, *arg = strdup(argv[i]);
> > > +				char *var, *value;
> > > +				char *arg = strdup(argv[i]);
> > > 
> > >  				if (!arg) {
> > >  					pr_err("%s: strdup failed\n", __func__);
> > > @@ -161,13 +207,21 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
> > >  					break;
> > >  				}
> > > 
> > > -				if (parse_config_arg(arg, &var) < 0) {
> > > +				if (parse_config_arg(arg, &var, &value) < 0) {
> > >  					free(arg);
> > >  					ret = -1;
> > >  					break;
> > >  				}
> > > 
> > > -				ret = show_spec_config(set, var);
> > > +				if (value == NULL)
> > > +					ret = show_spec_config(set, var);
> > > +				else {
> > > +					const char *config_filename = config_exclusive_filename;
> > > +
> > > +					if (!config_exclusive_filename)
> > > +						config_filename = user_config;
> > > +					ret = set_config(set, config_filename, var, value);
> > > +				}
> > >  				free(arg);
> 
> Here, the parts are a bit different than v2 patchset I sent lately.
> I refactored parse_config_arg() and a bit modify parsing
> config arguments in cmd_config().
> 
> Is it better to just skip v2 patchset I sent ?
> And remake new patchset regarding today your feedback ?
> 
> Or would I make v3 that adopts v2 I sent?
> 
> 
> Thanks,
> Taeung
> 
> > >  			}
> > >  		} else
> > > diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
> > > index 18dae74..c8fb65d 100644
> > > --- a/tools/perf/util/config.c
> > > +++ b/tools/perf/util/config.c
> > > @@ -602,6 +602,12 @@ static int collect_config(const char *var, const char *value,
> > >  	return -1;
> > >  }
> > > 
> > > +int perf_config_set__collect(struct perf_config_set *set,
> > > +			     const char *var, const char *value)
> > > +{
> > > +	return collect_config(var, value, set);
> > > +}
> > > +
> > >  static int perf_config_set__init(struct perf_config_set *set)
> > >  {
> > >  	int ret = -1;
> > > diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
> > > index 6f813d4..0fcdb8c 100644
> > > --- a/tools/perf/util/config.h
> > > +++ b/tools/perf/util/config.h
> > > @@ -33,6 +33,8 @@ const char *perf_etc_perfconfig(void);
> > > 
> > >  struct perf_config_set *perf_config_set__new(void);
> > >  void perf_config_set__delete(struct perf_config_set *set);
> > > +int perf_config_set__collect(struct perf_config_set *set,
> > > +			     const char *var, const char *value);
> > >  void perf_config__init(void);
> > >  void perf_config__exit(void);
> > >  void perf_config__refresh(void);
> > > --
> > > 2.7.4

Re: [PATCH 4/6] perf config: Add support for writing configs to a config file

[Taeung Song]

Hi, Arnaldo :)

On 11/15/2016 10:49 AM, Arnaldo Carvalho de Melo wrote:
> Em Tue, Nov 15, 2016 at 02:00:38AM +0900, Taeung Song escreveu:
>> Thank you for your review.
>>
>> I have a question at the very bottom
>> (skip v2 I sent lately or not ?).
>
> Humm, I combined some patches, fixed up the stuff I noticed, and pushed
> to Ingo, please take a look at that and post patches for anything you
> find applicable,
>
> Thanks,
>
> - Arnaldo

You combined this patches by yourself.
Thank you so much!
I checked the new patchset, I think it is good! :)

However,

/* overwrite configvariables */

We need to add a space as you said..
But it is a very minor part, I thought it can be ignored.

Thanks,
Taeung

>> On 11/15/2016 01:04 AM, Arnaldo Carvalho de Melo wrote:
>>> Em Fri, Nov 04, 2016 at 03:44:20PM +0900, Taeung Song escreveu:
>>>> Add setting feature that can add config variables with their values
>>>> to a config file (i.e. user or system config file) or modify
>>>> config key-value pairs in a config file.
>>>> For the syntax examples,
>>>>
>>>>     perf config [<file-option>] [section.name[=value] ...]
>>>>
>>>> e.g. You can set the ui.show-headers to false with
>>>>
>>>>     # perf config ui.show-headers=false
>>>>
>>>> If you want to add or modify several config items, you can do like
>>>>
>>>>     # perf config annotate.show_nr_jumps=false kmem.default=slab
>>>
>>> This works, but has some problems, see below:
>>>
>>>> Cc: Namhyung Kim <namhyung@kernel.org>
>>>> Cc: Jiri Olsa <jolsa@kernel.org>
>>>> Cc: Wang Nan <wangnan0@huawei.com>
>>>> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
>>>> ---
>>>>  tools/perf/builtin-config.c | 66 ++++++++++++++++++++++++++++++++++++++++-----
>>>>  tools/perf/util/config.c    |  6 +++++
>>>>  tools/perf/util/config.h    |  2 ++
>>>>  3 files changed, 68 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
>>>> index fe253f3..5313702 100644
>>>> --- a/tools/perf/builtin-config.c
>>>> +++ b/tools/perf/builtin-config.c
>>>> @@ -17,7 +17,7 @@
>>>>  static bool use_system_config, use_user_config;
>>>>
>>>>  static const char * const config_usage[] = {
>>>> -	"perf config [<file-option>] [options] [section.name ...]",
>>>> +	"perf config [<file-option>] [options] [section.name[=value] ...]",
>>>>  	NULL
>>>>  };
>>>>
>>>> @@ -33,6 +33,37 @@ static struct option config_options[] = {
>>>>  	OPT_END()
>>>>  };
>>>>
>>>> +static int set_config(struct perf_config_set *set, const char *file_name,
>>>> +		      const char *var, const char *value)
>>>> +{
>>>> +	struct perf_config_section *section = NULL;
>>>> +	struct perf_config_item *item = NULL;
>>>> +	const char *first_line = "# this file is auto-generated.";
>>>> +	FILE *fp = fopen(file_name, "w");
>>>> +
>>>> +	if (!fp)
>>>> +		return -1;
>>>> +	if (set == NULL)
>>>> +		return -1;
>>>
>>> So, here fp is left open? I'm fixing this...
>>
>> Understood. Sorry, I missed out it.
>>
>>>> +	perf_config_set__collect(set, var, value);
>>>> +	fprintf(fp, "%s\n", first_line);
>>>> +
>>>> +	/* overwrite configvariables */
>>>                          missing space?
>>
>> Oops.. I missed a white space between two words.
>>
>>>> +	perf_config_items__for_each_entry(&set->sections, section) {
>>>> +		fprintf(fp, "[%s]\n", section->name);
>>>> +
>>>> +		perf_config_items__for_each_entry(&section->items, item) {
>>>> +			if (item->value)
>>>> +				fprintf(fp, "\t%s = %s\n",
>>>> +					item->name, item->value);
>>>> +		}
>>>> +	}
>>>> +	fclose(fp);
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>>  static int show_spec_config(struct perf_config_set *set, const char *var)
>>>>  {
>>>>  	struct perf_config_section *section;
>>>> @@ -82,7 +113,7 @@ static int show_config(struct perf_config_set *set)
>>>>  	return 0;
>>>>  }
>>>>
>>>> -static int parse_config_arg(char *arg, char **var)
>>>> +static int parse_config_arg(char *arg, char **var, char **value)
>>>>  {
>>>>  	const char *last_dot = strchr(arg, '.');
>>>>
>>>> @@ -99,7 +130,21 @@ static int parse_config_arg(char *arg, char **var)
>>>>  		return -1;
>>>>  	}
>>>>
>>>> -	*var = arg;
>>>> +	*value = strchr(arg, '=');
>>>> +	if (*value == NULL)
>>>> +		*var = arg;
>>>> +	else if (!strcmp(*value, "=")) {
>>>> +		pr_err("The config variable does not contain a value: %s\n", arg);
>>>> +		return -1;
>>>> +	} else {
>>>> +		*value = *value + 1; /* excluding a first character '=' */
>>>> +		*var = strsep(&arg, "=");
>>>> +		if (*var[0] == '\0') {
>>>> +			pr_err("invalid config variable: %s\n", arg);
>>>> +			return -1;
>>>> +		}
>>>> +	}
>>>> +
>>
>> Here and..
>>
>>>>
>>>> @@ -153,7 +198,8 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
>>>>  	default:
>>>>  		if (argc) {
>>>>  			for (i = 0; argv[i]; i++) {
>>>> -				char *var, *arg = strdup(argv[i]);
>>>> +				char *var, *value;
>>>> +				char *arg = strdup(argv[i]);
>>>>
>>>>  				if (!arg) {
>>>>  					pr_err("%s: strdup failed\n", __func__);
>>>> @@ -161,13 +207,21 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
>>>>  					break;
>>>>  				}
>>>>
>>>> -				if (parse_config_arg(arg, &var) < 0) {
>>>> +				if (parse_config_arg(arg, &var, &value) < 0) {
>>>>  					free(arg);
>>>>  					ret = -1;
>>>>  					break;
>>>>  				}
>>>>
>>>> -				ret = show_spec_config(set, var);
>>>> +				if (value == NULL)
>>>> +					ret = show_spec_config(set, var);
>>>> +				else {
>>>> +					const char *config_filename = config_exclusive_filename;
>>>> +
>>>> +					if (!config_exclusive_filename)
>>>> +						config_filename = user_config;
>>>> +					ret = set_config(set, config_filename, var, value);
>>>> +				}
>>>>  				free(arg);
>>
>> Here, the parts are a bit different than v2 patchset I sent lately.
>> I refactored parse_config_arg() and a bit modify parsing
>> config arguments in cmd_config().
>>
>> Is it better to just skip v2 patchset I sent ?
>> And remake new patchset regarding today your feedback ?
>>
>> Or would I make v3 that adopts v2 I sent?
>>
>>
>> Thanks,
>> Taeung
>>
>>>>  			}
>>>>  		} else
>>>> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
>>>> index 18dae74..c8fb65d 100644
>>>> --- a/tools/perf/util/config.c
>>>> +++ b/tools/perf/util/config.c
>>>> @@ -602,6 +602,12 @@ static int collect_config(const char *var, const char *value,
>>>>  	return -1;
>>>>  }
>>>>
>>>> +int perf_config_set__collect(struct perf_config_set *set,
>>>> +			     const char *var, const char *value)
>>>> +{
>>>> +	return collect_config(var, value, set);
>>>> +}
>>>> +
>>>>  static int perf_config_set__init(struct perf_config_set *set)
>>>>  {
>>>>  	int ret = -1;
>>>> diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
>>>> index 6f813d4..0fcdb8c 100644
>>>> --- a/tools/perf/util/config.h
>>>> +++ b/tools/perf/util/config.h
>>>> @@ -33,6 +33,8 @@ const char *perf_etc_perfconfig(void);
>>>>
>>>>  struct perf_config_set *perf_config_set__new(void);
>>>>  void perf_config_set__delete(struct perf_config_set *set);
>>>> +int perf_config_set__collect(struct perf_config_set *set,
>>>> +			     const char *var, const char *value);
>>>>  void perf_config__init(void);
>>>>  void perf_config__exit(void);
>>>>  void perf_config__refresh(void);
>>>> --
>>>> 2.7.4

[tip:perf/core] perf config: Add support setting variables in a config file

[tip-bot for Taeung Song]

Commit-ID:  c6fc018a7a64c2c3ea56529fd8d0ca0f43408b0f
Gitweb:     http://git.kernel.org/tip/c6fc018a7a64c2c3ea56529fd8d0ca0f43408b0f
Author:     Taeung Song <treeze.taeung@gmail.com>
AuthorDate: Fri, 4 Nov 2016 15:44:20 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 14 Nov 2016 13:08:11 -0300

perf config: Add support setting variables in a config file

Add setting feature that can add config variables with their values to a
config file (i.e. user or system config file) or modify config key-value
pairs in a config file.  For the syntax examples:

    perf config [<file-option>] [section.name[=value] ...]

e.g. You can set the ui.show-headers to false with

    # perf config ui.show-headers=false

If you want to add or modify several config items, you can do like

    # perf config annotate.show_nr_jumps=false kmem.default=slab

Committer notes:

Testing it:

  $ perf config -l
  top.children=true
  report.children=false
  $
  $ perf config top.children=false
  $ perf config -l
  top.children=false
  report.children=false
  $
  $ perf config kmem.default=slab
  $ perf config -l
  top.children=false
  report.children=false
  kmem.default=slab
  $

Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Nambong Ha <over3025@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Cc: Wookje Kwon <aweee0@gmail.com>
Link: http://lkml.kernel.org/r/1478241862-31230-5-git-send-email-treeze.taeung@gmail.com
[ Combined patch with docs update with this one ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Documentation/perf-config.txt | 19 ++++++++-
 tools/perf/builtin-config.c              | 68 +++++++++++++++++++++++++++++---
 tools/perf/util/config.c                 |  6 +++
 tools/perf/util/config.h                 |  2 +
 4 files changed, 88 insertions(+), 7 deletions(-)

diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt
index 1714b0c..9365b75 100644
--- a/tools/perf/Documentation/perf-config.txt
+++ b/tools/perf/Documentation/perf-config.txt
@@ -8,7 +8,7 @@ perf-config - Get and set variables in a configuration file.
 SYNOPSIS
 --------
 [verse]
-'perf config' [<file-option>] [section.name ...]
+'perf config' [<file-option>] [section.name[=value] ...]
 or
 'perf config' [<file-option>] -l | --list
 
@@ -120,6 +120,23 @@ Given a $HOME/.perfconfig like this:
 		children = true
 		group = true
 
+You can hide source code of annotate feature setting the config to false with
+
+	% perf config annotate.hide_src_code=true
+
+If you want to add or modify several config items, you can do like
+
+	% perf config ui.show-headers=false kmem.default=slab
+
+To modify the sort order of report functionality in user config file(i.e. `~/.perfconfig`), do
+
+	% perf config --user report sort-order=srcline
+
+To change colors of selected line to other foreground and background colors
+in system config file (i.e. `$(sysconf)/perfconfig`), do
+
+	% perf config --system colors.selected=yellow,green
+
 To query the record mode of call graph, do
 
 	% perf config call-graph.record-mode
diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
index 88a43fe..7c861b5 100644
--- a/tools/perf/builtin-config.c
+++ b/tools/perf/builtin-config.c
@@ -17,7 +17,7 @@
 static bool use_system_config, use_user_config;
 
 static const char * const config_usage[] = {
-	"perf config [<file-option>] [options] [section.name ...]",
+	"perf config [<file-option>] [options] [section.name[=value] ...]",
 	NULL
 };
 
@@ -33,6 +33,39 @@ static struct option config_options[] = {
 	OPT_END()
 };
 
+static int set_config(struct perf_config_set *set, const char *file_name,
+		      const char *var, const char *value)
+{
+	struct perf_config_section *section = NULL;
+	struct perf_config_item *item = NULL;
+	const char *first_line = "# this file is auto-generated.";
+	FILE *fp;
+
+	if (set == NULL)
+		return -1;
+
+	fp = fopen(file_name, "w");
+	if (!fp)
+		return -1;
+
+	perf_config_set__collect(set, var, value);
+	fprintf(fp, "%s\n", first_line);
+
+	/* overwrite configvariables */
+	perf_config_items__for_each_entry(&set->sections, section) {
+		fprintf(fp, "[%s]\n", section->name);
+
+		perf_config_items__for_each_entry(&section->items, item) {
+			if (item->value)
+				fprintf(fp, "\t%s = %s\n",
+					item->name, item->value);
+		}
+	}
+	fclose(fp);
+
+	return 0;
+}
+
 static int show_spec_config(struct perf_config_set *set, const char *var)
 {
 	struct perf_config_section *section;
@@ -82,7 +115,7 @@ static int show_config(struct perf_config_set *set)
 	return 0;
 }
 
-static int parse_config_arg(char *arg, char **var)
+static int parse_config_arg(char *arg, char **var, char **value)
 {
 	const char *last_dot = strchr(arg, '.');
 
@@ -99,7 +132,21 @@ static int parse_config_arg(char *arg, char **var)
 		return -1;
 	}
 
-	*var = arg;
+	*value = strchr(arg, '=');
+	if (*value == NULL)
+		*var = arg;
+	else if (!strcmp(*value, "=")) {
+		pr_err("The config variable does not contain a value: %s\n", arg);
+		return -1;
+	} else {
+		*value = *value + 1; /* excluding a first character '=' */
+		*var = strsep(&arg, "=");
+		if (*var[0] == '\0') {
+			pr_err("invalid config variable: %s\n", arg);
+			return -1;
+		}
+	}
+
 	return 0;
 }
 
@@ -153,7 +200,8 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
 	default:
 		if (argc) {
 			for (i = 0; argv[i]; i++) {
-				char *var, *arg = strdup(argv[i]);
+				char *var, *value;
+				char *arg = strdup(argv[i]);
 
 				if (!arg) {
 					pr_err("%s: strdup failed\n", __func__);
@@ -161,13 +209,21 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
 					break;
 				}
 
-				if (parse_config_arg(arg, &var) < 0) {
+				if (parse_config_arg(arg, &var, &value) < 0) {
 					free(arg);
 					ret = -1;
 					break;
 				}
 
-				ret = show_spec_config(set, var);
+				if (value == NULL)
+					ret = show_spec_config(set, var);
+				else {
+					const char *config_filename = config_exclusive_filename;
+
+					if (!config_exclusive_filename)
+						config_filename = user_config;
+					ret = set_config(set, config_filename, var, value);
+				}
 				free(arg);
 			}
 		} else
diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index 18dae74..c8fb65d 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -602,6 +602,12 @@ out_free:
 	return -1;
 }
 
+int perf_config_set__collect(struct perf_config_set *set,
+			     const char *var, const char *value)
+{
+	return collect_config(var, value, set);
+}
+
 static int perf_config_set__init(struct perf_config_set *set)
 {
 	int ret = -1;
diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
index 6f813d4..0fcdb8c 100644
--- a/tools/perf/util/config.h
+++ b/tools/perf/util/config.h
@@ -33,6 +33,8 @@ const char *perf_etc_perfconfig(void);
 
 struct perf_config_set *perf_config_set__new(void);
 void perf_config_set__delete(struct perf_config_set *set);
+int perf_config_set__collect(struct perf_config_set *set,
+			     const char *var, const char *value);
 void perf_config__init(void);
 void perf_config__exit(void);
 void perf_config__refresh(void);

[PATCH 5/6] perf config: Document examples to set config variables with values in man page

[Taeung Song]

Explain how to add or modify particular config items in config file
and how to set several config items from user or system config file
using '--user' or '--system' options.

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Wang Nan <wangnan0@huawei.com>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/Documentation/perf-config.txt | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt
index 1714b0c..9365b75 100644
--- a/tools/perf/Documentation/perf-config.txt
+++ b/tools/perf/Documentation/perf-config.txt
@@ -8,7 +8,7 @@ perf-config - Get and set variables in a configuration file.
 SYNOPSIS
 --------
 [verse]
-'perf config' [<file-option>] [section.name ...]
+'perf config' [<file-option>] [section.name[=value] ...]
 or
 'perf config' [<file-option>] -l | --list
 
@@ -120,6 +120,23 @@ Given a $HOME/.perfconfig like this:
 		children = true
 		group = true
 
+You can hide source code of annotate feature setting the config to false with
+
+	% perf config annotate.hide_src_code=true
+
+If you want to add or modify several config items, you can do like
+
+	% perf config ui.show-headers=false kmem.default=slab
+
+To modify the sort order of report functionality in user config file(i.e. `~/.perfconfig`), do
+
+	% perf config --user report sort-order=srcline
+
+To change colors of selected line to other foreground and background colors
+in system config file (i.e. `$(sysconf)/perfconfig`), do
+
+	% perf config --system colors.selected=yellow,green
+
 To query the record mode of call graph, do
 
 	% perf config call-graph.record-mode
-- 
2.7.4

[PATCH 6/6] perf config: Mark where are config items from (user or system)

[Taeung Song]

To write config items to a particular config file,
we should know where is each config section and item from.

Current setting functionality of perf-config use autogenerating
way by overwriting collected config items to a config file.
For example,
When collecting config items from user and system config
files (i.e. ~/.perfconfig and $(sysconf)/perfconfig),
perf_config_set can contain both user and system config items.
So we should know where each value is from to avoid merging
user and system config items on user config file.

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Wang Nan <wangnan0@huawei.com>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/builtin-config.c |  6 +++++-
 tools/perf/util/config.c    | 16 +++++++++++++++-
 tools/perf/util/config.h    |  4 +++-
 3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
index 5313702..f4541d7 100644
--- a/tools/perf/builtin-config.c
+++ b/tools/perf/builtin-config.c
@@ -46,14 +46,18 @@ static int set_config(struct perf_config_set *set, const char *file_name,
 	if (set == NULL)
 		return -1;
 
-	perf_config_set__collect(set, var, value);
+	perf_config_set__collect(set, file_name, var, value);
 	fprintf(fp, "%s\n", first_line);
 
 	/* overwrite configvariables */
 	perf_config_items__for_each_entry(&set->sections, section) {
+		if (!use_system_config && section->from_system_config)
+			continue;
 		fprintf(fp, "[%s]\n", section->name);
 
 		perf_config_items__for_each_entry(&section->items, item) {
+			if (!use_system_config && section->from_system_config)
+				continue;
 			if (item->value)
 				fprintf(fp, "\t%s = %s\n",
 					item->name, item->value);
diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index c8fb65d..3d906db 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -594,6 +594,19 @@ static int collect_config(const char *var, const char *value,
 			goto out_free;
 	}
 
+	/* perf_config_set can contain both user and system config items.
+	 * So we should know where each value is from.
+	 * The classification would be needed when a particular config file
+	 * is overwrited by setting feature i.e. set_config().
+	 */
+	if (strcmp(config_file_name, perf_etc_perfconfig()) == 0) {
+		section->from_system_config = true;
+		item->from_system_config = true;
+	} else {
+		section->from_system_config = false;
+		item->from_system_config = false;
+	}
+
 	ret = set_value(item, value);
 	return ret;
 
@@ -602,9 +615,10 @@ static int collect_config(const char *var, const char *value,
 	return -1;
 }
 
-int perf_config_set__collect(struct perf_config_set *set,
+int perf_config_set__collect(struct perf_config_set *set, const char *file_name,
 			     const char *var, const char *value)
 {
+	config_file_name = file_name;
 	return collect_config(var, value, set);
 }
 
diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
index 0fcdb8c..1a59a6b 100644
--- a/tools/perf/util/config.h
+++ b/tools/perf/util/config.h
@@ -7,12 +7,14 @@
 struct perf_config_item {
 	char *name;
 	char *value;
+	bool from_system_config;
 	struct list_head node;
 };
 
 struct perf_config_section {
 	char *name;
 	struct list_head items;
+	bool from_system_config;
 	struct list_head node;
 };
 
@@ -33,7 +35,7 @@ const char *perf_etc_perfconfig(void);
 
 struct perf_config_set *perf_config_set__new(void);
 void perf_config_set__delete(struct perf_config_set *set);
-int perf_config_set__collect(struct perf_config_set *set,
+int perf_config_set__collect(struct perf_config_set *set, const char *file_name,
 			     const char *var, const char *value);
 void perf_config__init(void);
 void perf_config__exit(void);
-- 
2.7.4

[tip:perf/core] perf config: Mark where are config items from (user or system)

[tip-bot for Taeung Song]

Commit-ID:  08d090cfed8cc2ce5821ddb2b91118979e511019
Gitweb:     http://git.kernel.org/tip/08d090cfed8cc2ce5821ddb2b91118979e511019
Author:     Taeung Song <treeze.taeung@gmail.com>
AuthorDate: Fri, 4 Nov 2016 15:44:22 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 14 Nov 2016 13:10:37 -0300

perf config: Mark where are config items from (user or system)

To write config items to a particular config file, we should know where
is each config section and item from.

Current setting functionality of perf-config use autogenerating way by
overwriting collected config items to a config file.

For example, when collecting config items from user and system config
files (i.e. ~/.perfconfig and $(sysconf)/perfconfig), perf_config_set
can contain both user and system config items.  So we should know where
each value is from to avoid merging user and system config items on user
config file.

Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Nambong Ha <over3025@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Cc: Wookje Kwon <aweee0@gmail.com>
Link: http://lkml.kernel.org/r/1478241862-31230-7-git-send-email-treeze.taeung@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-config.c |  6 +++++-
 tools/perf/util/config.c    | 16 +++++++++++++++-
 tools/perf/util/config.h    |  4 +++-
 3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
index 7c861b5..8c0d93b 100644
--- a/tools/perf/builtin-config.c
+++ b/tools/perf/builtin-config.c
@@ -48,14 +48,18 @@ static int set_config(struct perf_config_set *set, const char *file_name,
 	if (!fp)
 		return -1;
 
-	perf_config_set__collect(set, var, value);
+	perf_config_set__collect(set, file_name, var, value);
 	fprintf(fp, "%s\n", first_line);
 
 	/* overwrite configvariables */
 	perf_config_items__for_each_entry(&set->sections, section) {
+		if (!use_system_config && section->from_system_config)
+			continue;
 		fprintf(fp, "[%s]\n", section->name);
 
 		perf_config_items__for_each_entry(&section->items, item) {
+			if (!use_system_config && section->from_system_config)
+				continue;
 			if (item->value)
 				fprintf(fp, "\t%s = %s\n",
 					item->name, item->value);
diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index c8fb65d..3d906db 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -594,6 +594,19 @@ static int collect_config(const char *var, const char *value,
 			goto out_free;
 	}
 
+	/* perf_config_set can contain both user and system config items.
+	 * So we should know where each value is from.
+	 * The classification would be needed when a particular config file
+	 * is overwrited by setting feature i.e. set_config().
+	 */
+	if (strcmp(config_file_name, perf_etc_perfconfig()) == 0) {
+		section->from_system_config = true;
+		item->from_system_config = true;
+	} else {
+		section->from_system_config = false;
+		item->from_system_config = false;
+	}
+
 	ret = set_value(item, value);
 	return ret;
 
@@ -602,9 +615,10 @@ out_free:
 	return -1;
 }
 
-int perf_config_set__collect(struct perf_config_set *set,
+int perf_config_set__collect(struct perf_config_set *set, const char *file_name,
 			     const char *var, const char *value)
 {
+	config_file_name = file_name;
 	return collect_config(var, value, set);
 }
 
diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
index 0fcdb8c..1a59a6b 100644
--- a/tools/perf/util/config.h
+++ b/tools/perf/util/config.h
@@ -7,12 +7,14 @@
 struct perf_config_item {
 	char *name;
 	char *value;
+	bool from_system_config;
 	struct list_head node;
 };
 
 struct perf_config_section {
 	char *name;
 	struct list_head items;
+	bool from_system_config;
 	struct list_head node;
 };
 
@@ -33,7 +35,7 @@ const char *perf_etc_perfconfig(void);
 
 struct perf_config_set *perf_config_set__new(void);
 void perf_config_set__delete(struct perf_config_set *set);
-int perf_config_set__collect(struct perf_config_set *set,
+int perf_config_set__collect(struct perf_config_set *set, const char *file_name,
 			     const char *var, const char *value);
 void perf_config__init(void);
 void perf_config__exit(void);