[RFC][PATCH v3 0/4] perf config: Infrastructure code for perf-config

[RFC][PATCH v3 0/4] perf config: Infrastructure code for perf-config

[Taeung Song]

Hi,

We can use the config files (i.e user wide ~/.perfconfig
and system wide $(sysconfdir)/perfconfig)
to configure perf tools. perf-config help user
manage the config files, not manually look into or edit them.

Introduce new infrastructure code for config
management features of perf-config subcommand.

This pathset contains basic code for various purposes of configuration management
showing current configs, in the near future,
showing all configs with default value,
getting current configs from the config files
or writing configs that user type on the config files, etc.

IMHO, I think this infrastructure code is needed
to add new funcationalities for config management of perf-config.

If anyone reviews this, I'd appreciate it.

Thanks,
Taeung

v3:
- use the section list that contains configs each section
  instead of the single config list (Namhyung)
- exclude a patch for '--list-all' option from this patchset

v2:
- remove perf_config_kind (user, system or both config files)
  and needless at this time, etc. (Namhyung)
- separate this patch as several patches (Namhyung)
- fix typing errors, etc.

Taeung Song (4):
  perf config: Introduce perf_config_set class
  perf config: Let show_config() work with perf_config_set
  perf config: Prepare all default configs
  perf config: Initialize perf_config_set with all default configs

 tools/perf/builtin-config.c |  38 ++++--
 tools/perf/util/config.c    | 298 ++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/config.h    |  90 +++++++++++++
 3 files changed, 419 insertions(+), 7 deletions(-)
 create mode 100644 tools/perf/util/config.h

-- 
2.5.0

[PATCH v3 1/4] perf config: Introduce perf_config_set class

[Taeung Song]

This infrastructure code was designed for
upcoming features of perf-config.

That collect config key-value pairs from user and
system config files (i.e. user wide ~/.perfconfig
and system wide $(sysconfdir)/perfconfig)
to manage perf's configs.

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/util/config.c | 169 +++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/config.h |  26 ++++++++
 2 files changed, 195 insertions(+)
 create mode 100644 tools/perf/util/config.h

diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index 4e72763..3beeceb 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -13,6 +13,7 @@
 #include <subcmd/exec-cmd.h>
 #include "util/hist.h"  /* perf_hist_config */
 #include "util/llvm-utils.h"   /* perf_llvm_config */
+#include "config.h"
 
 #define MAXNAME (256)
 
@@ -506,6 +507,174 @@ out:
 	return ret;
 }
 
+static struct perf_config_section *find_section(struct list_head *sections,
+						const char *section_name)
+{
+	struct perf_config_section *section;
+
+	list_for_each_entry(section, sections, list)
+		if (!strcmp(section->name, section_name))
+			return section;
+
+	return NULL;
+}
+
+static struct perf_config_item *find_config_item(const char *name,
+						 struct perf_config_section *section)
+{
+	struct perf_config_item *config_item;
+
+	list_for_each_entry(config_item, &section->config_items, list)
+		if (!strcmp(config_item->name, name))
+			return config_item;
+
+	return NULL;
+}
+
+static void find_config(struct list_head *sections,
+			struct perf_config_section **section,
+			struct perf_config_item **config_item,
+			const char *section_name, const char *name)
+{
+	*section = find_section(sections, section_name);
+
+	if (*section != NULL)
+		*config_item = find_config_item(name, *section);
+	else
+		*config_item = NULL;
+}
+
+static struct perf_config_section *add_section(struct list_head *sections,
+					       const char *section_name)
+{
+	struct perf_config_section *section = zalloc(sizeof(*section));
+
+	if (!section)
+		return NULL;
+
+	INIT_LIST_HEAD(&section->config_items);
+	section->name = strdup(section_name);
+	if (!section->name) {
+		pr_err("%s: strdup failed\n", __func__);
+		free(section);
+		return NULL;
+	}
+
+	list_add_tail(&section->list, sections);
+	return section;
+}
+
+static struct perf_config_item *add_config_item(struct perf_config_section *section,
+						const char *name)
+{
+	struct perf_config_item *config_item = zalloc(sizeof(*config_item));
+
+	if (!config_item)
+		return NULL;
+
+	config_item->name = strdup(name);
+	if (!name) {
+		pr_err("%s: strdup failed\n", __func__);
+		goto out_err;
+	}
+
+	list_add_tail(&config_item->list, &section->config_items);
+	return config_item;
+
+out_err:
+	free(config_item);
+	return NULL;
+}
+
+static int set_value(struct perf_config_item *config_item, const char *value)
+{
+	char *val = strdup(value);
+
+	if (!val)
+		return -1;
+
+	free(config_item->value);
+	config_item->value = val;
+	return 0;
+}
+
+static int collect_config(const char *var, const char *value,
+			  void *section_list)
+{
+	int ret = -1;
+	char *ptr, *key;
+	char *section_name, *name;
+	struct perf_config_section *section = NULL;
+	struct perf_config_item *config_item = NULL;
+	struct list_head *sections = section_list;
+
+	key = ptr = strdup(var);
+	if (!key) {
+		pr_err("%s: strdup failed\n", __func__);
+		return -1;
+	}
+
+	section_name = strsep(&ptr, ".");
+	name = ptr;
+	if (name == NULL || value == NULL)
+		goto out_free;
+
+	find_config(sections, &section, &config_item, section_name, name);
+
+	if (!section) {
+		section = add_section(sections, section_name);
+		if (!section)
+			goto out_free;
+	}
+	if (!config_item) {
+		config_item = add_config_item(section, name);
+		if (!config_item)
+			goto out_free;
+	}
+
+	ret = set_value(config_item, value);
+	return ret;
+
+out_free:
+	free(key);
+	return ret;
+}
+
+struct perf_config_set *perf_config_set__new(void)
+{
+	struct perf_config_set *perf_configs = zalloc(sizeof(*perf_configs));
+
+	if (!perf_configs)
+		return NULL;
+
+	INIT_LIST_HEAD(&perf_configs->sections);
+	perf_config(collect_config, &perf_configs->sections);
+
+	return perf_configs;
+}
+
+void perf_config_set__delete(struct perf_config_set *perf_configs)
+{
+	struct perf_config_section *section, *section_tmp;
+	struct perf_config_item *config_item, *item_tmp;
+
+	list_for_each_entry_safe(section, section_tmp,
+				 &perf_configs->sections, list) {
+		list_for_each_entry_safe(config_item, item_tmp,
+					 &section->config_items, list) {
+			list_del(&config_item->list);
+			free(config_item->name);
+			free(config_item->value);
+			free(config_item);
+		}
+		list_del(&section->list);
+		free(section->name);
+		free(section);
+	}
+
+	free(perf_configs);
+}
+
 /*
  * Call this to report error for your variable that should not
  * get a boolean value (i.e. "[my] var" means "true").
diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
new file mode 100644
index 0000000..e270e51
--- /dev/null
+++ b/tools/perf/util/config.h
@@ -0,0 +1,26 @@
+#ifndef __PERF_CONFIG_H
+#define __PERF_CONFIG_H
+
+#include <stdbool.h>
+#include <linux/list.h>
+
+struct perf_config_item {
+	char *name;
+	char *value;
+	struct list_head list;
+};
+
+struct perf_config_section {
+	char *name;
+	struct list_head config_items;
+	struct list_head list;
+};
+
+struct perf_config_set {
+	struct list_head sections;
+};
+
+struct perf_config_set *perf_config_set__new(void);
+void perf_config_set__delete(struct perf_config_set *perf_configs);
+
+#endif /* __PERF_CONFIG_H */
-- 
2.5.0

Re: [PATCH v3 1/4] perf config: Introduce perf_config_set class

[Namhyung Kim]

Hi Taeung,

On Thu, Mar 24, 2016 at 05:16:30AM +0900, Taeung Song wrote:
> This infrastructure code was designed for
> upcoming features of perf-config.
> 
> That collect config key-value pairs from user and
> system config files (i.e. user wide ~/.perfconfig
> and system wide $(sysconfdir)/perfconfig)
> to manage perf's configs.
> 
> 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/util/config.c | 169 +++++++++++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/config.h |  26 ++++++++
>  2 files changed, 195 insertions(+)
>  create mode 100644 tools/perf/util/config.h
> 
> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
> index 4e72763..3beeceb 100644
> --- a/tools/perf/util/config.c
> +++ b/tools/perf/util/config.c
> @@ -13,6 +13,7 @@
>  #include <subcmd/exec-cmd.h>
>  #include "util/hist.h"  /* perf_hist_config */
>  #include "util/llvm-utils.h"   /* perf_llvm_config */
> +#include "config.h"
>  
>  #define MAXNAME (256)
>  
> @@ -506,6 +507,174 @@ out:
>  	return ret;
>  }
>  
> +static struct perf_config_section *find_section(struct list_head *sections,
> +						const char *section_name)
> +{
> +	struct perf_config_section *section;
> +
> +	list_for_each_entry(section, sections, list)
> +		if (!strcmp(section->name, section_name))
> +			return section;
> +
> +	return NULL;
> +}
> +
> +static struct perf_config_item *find_config_item(const char *name,
> +						 struct perf_config_section *section)
> +{
> +	struct perf_config_item *config_item;
> +
> +	list_for_each_entry(config_item, &section->config_items, list)
> +		if (!strcmp(config_item->name, name))
> +			return config_item;
> +
> +	return NULL;
> +}
> +
> +static void find_config(struct list_head *sections,
> +			struct perf_config_section **section,
> +			struct perf_config_item **config_item,
> +			const char *section_name, const char *name)
> +{
> +	*section = find_section(sections, section_name);
> +
> +	if (*section != NULL)
> +		*config_item = find_config_item(name, *section);
> +	else
> +		*config_item = NULL;
> +}
> +
> +static struct perf_config_section *add_section(struct list_head *sections,
> +					       const char *section_name)
> +{
> +	struct perf_config_section *section = zalloc(sizeof(*section));
> +
> +	if (!section)
> +		return NULL;
> +
> +	INIT_LIST_HEAD(&section->config_items);
> +	section->name = strdup(section_name);
> +	if (!section->name) {
> +		pr_err("%s: strdup failed\n", __func__);
> +		free(section);
> +		return NULL;
> +	}
> +
> +	list_add_tail(&section->list, sections);
> +	return section;
> +}
> +
> +static struct perf_config_item *add_config_item(struct perf_config_section *section,
> +						const char *name)
> +{
> +	struct perf_config_item *config_item = zalloc(sizeof(*config_item));
> +
> +	if (!config_item)
> +		return NULL;
> +
> +	config_item->name = strdup(name);
> +	if (!name) {
> +		pr_err("%s: strdup failed\n", __func__);
> +		goto out_err;
> +	}
> +
> +	list_add_tail(&config_item->list, &section->config_items);
> +	return config_item;
> +
> +out_err:
> +	free(config_item);
> +	return NULL;
> +}
> +
> +static int set_value(struct perf_config_item *config_item, const char *value)
> +{
> +	char *val = strdup(value);
> +
> +	if (!val)
> +		return -1;
> +
> +	free(config_item->value);
> +	config_item->value = val;
> +	return 0;
> +}
> +
> +static int collect_config(const char *var, const char *value,
> +			  void *section_list)
> +{
> +	int ret = -1;
> +	char *ptr, *key;
> +	char *section_name, *name;
> +	struct perf_config_section *section = NULL;
> +	struct perf_config_item *config_item = NULL;
> +	struct list_head *sections = section_list;
> +
> +	key = ptr = strdup(var);
> +	if (!key) {
> +		pr_err("%s: strdup failed\n", __func__);
> +		return -1;
> +	}
> +
> +	section_name = strsep(&ptr, ".");
> +	name = ptr;
> +	if (name == NULL || value == NULL)
> +		goto out_free;
> +
> +	find_config(sections, &section, &config_item, section_name, name);
> +
> +	if (!section) {
> +		section = add_section(sections, section_name);
> +		if (!section)
> +			goto out_free;
> +	}
> +	if (!config_item) {
> +		config_item = add_config_item(section, name);
> +		if (!config_item)
> +			goto out_free;
> +	}
> +
> +	ret = set_value(config_item, value);
> +	return ret;
> +
> +out_free:
> +	free(key);
> +	return ret;
> +}
> +
> +struct perf_config_set *perf_config_set__new(void)
> +{
> +	struct perf_config_set *perf_configs = zalloc(sizeof(*perf_configs));
> +
> +	if (!perf_configs)
> +		return NULL;
> +
> +	INIT_LIST_HEAD(&perf_configs->sections);
> +	perf_config(collect_config, &perf_configs->sections);
> +
> +	return perf_configs;
> +}
> +
> +void perf_config_set__delete(struct perf_config_set *perf_configs)
> +{
> +	struct perf_config_section *section, *section_tmp;
> +	struct perf_config_item *config_item, *item_tmp;
> +
> +	list_for_each_entry_safe(section, section_tmp,
> +				 &perf_configs->sections, list) {
> +		list_for_each_entry_safe(config_item, item_tmp,
> +					 &section->config_items, list) {
> +			list_del(&config_item->list);
> +			free(config_item->name);
> +			free(config_item->value);
> +			free(config_item);
> +		}
> +		list_del(&section->list);
> +		free(section->name);
> +		free(section);
> +	}
> +
> +	free(perf_configs);
> +}
> +
>  /*
>   * Call this to report error for your variable that should not
>   * get a boolean value (i.e. "[my] var" means "true").
> diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
> new file mode 100644
> index 0000000..e270e51
> --- /dev/null
> +++ b/tools/perf/util/config.h
> @@ -0,0 +1,26 @@
> +#ifndef __PERF_CONFIG_H
> +#define __PERF_CONFIG_H
> +
> +#include <stdbool.h>
> +#include <linux/list.h>
> +
> +struct perf_config_item {
> +	char *name;
> +	char *value;
> +	struct list_head list;
> +};
> +
> +struct perf_config_section {
> +	char *name;
> +	struct list_head config_items;
> +	struct list_head list;
> +};
> +
> +struct perf_config_set {
> +	struct list_head sections;
> +};
> +
> +struct perf_config_set *perf_config_set__new(void);
> +void perf_config_set__delete(struct perf_config_set *perf_configs);
> +
> +#endif /* __PERF_CONFIG_H */
> -- 
> 2.5.0
> 

Re: [PATCH v3 1/4] perf config: Introduce perf_config_set class

[Taeung Song]

Hi, Namhyung

On 03/24/2016 03:07 PM, Namhyung Kim wrote:
> Hi Taeung,
>
> On Thu, Mar 24, 2016 at 05:16:30AM +0900, Taeung Song wrote:
>> This infrastructure code was designed for
>> upcoming features of perf-config.
>>
>> That collect config key-value pairs from user and
>> system config files (i.e. user wide ~/.perfconfig
>> and system wide $(sysconfdir)/perfconfig)
>> to manage perf's configs.
>>
>> Cc: Jiri Olsa <jolsa@kernel.org>
>> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
>
> Acked-by: Namhyung Kim <namhyung@kernel.org>
>

Thank you!!

But I'm so sorry for my mistake.
I missed perf_config_set__delete() out when the error occur in 
collect_config().

(If returning -1 from collect_config(), perf process die with some error 
message at once
so it is needed to call perf_config_set__delete() before that moment)

So, I'll resend this patch after fixing it as below.

diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index 725015f..3831733 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -705,14 +706,15 @@ static int set_value(struct perf_config_item 
*config_item, const char *value)
  }

  static int collect_config(const char *var, const char *value,
-                          void *section_list)
+                          void *perf_config_set)
  {
          int ret = -1;
          char *ptr, *key;
          char *section_name, *name;
          struct perf_config_section *section = NULL;
          struct perf_config_item *config_item = NULL;
-        struct list_head *sections = section_list;
+        struct perf_config_set *perf_configs = perf_config_set;
+        struct list_head *sections = &perf_configs->sections;

          key = ptr = strdup(var);
          if (!key) {
@@ -743,7 +745,8 @@ static int collect_config(const char *var, const 
char *value,

  out_free:
          free(key);
-        return ret;
+        perf_config_set__delete(perf_configs);
+        return -1;
  }

  static struct perf_config_set *perf_config_set__init(struct 
perf_config_set *perf_configs)
@@ -777,7 +780,7 @@ struct perf_config_set *perf_config_set__new(void)
                  return NULL;

          perf_config_set__init(perf_configs);
-        perf_config(collect_config, &perf_configs->sections);
+        perf_config(collect_config, perf_configs);

          return perf_configs;
  }


Thanks,
Taeung

>
>
>> ---
>>   tools/perf/util/config.c | 169 +++++++++++++++++++++++++++++++++++++++++++++++
>>   tools/perf/util/config.h |  26 ++++++++
>>   2 files changed, 195 insertions(+)
>>   create mode 100644 tools/perf/util/config.h
>>
>> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
>> index 4e72763..3beeceb 100644
>> --- a/tools/perf/util/config.c
>> +++ b/tools/perf/util/config.c
>> @@ -13,6 +13,7 @@
>>   #include <subcmd/exec-cmd.h>
>>   #include "util/hist.h"  /* perf_hist_config */
>>   #include "util/llvm-utils.h"   /* perf_llvm_config */
>> +#include "config.h"
>>
>>   #define MAXNAME (256)
>>
>> @@ -506,6 +507,174 @@ out:
>>   	return ret;
>>   }
>>
>> +static struct perf_config_section *find_section(struct list_head *sections,
>> +						const char *section_name)
>> +{
>> +	struct perf_config_section *section;
>> +
>> +	list_for_each_entry(section, sections, list)
>> +		if (!strcmp(section->name, section_name))
>> +			return section;
>> +
>> +	return NULL;
>> +}
>> +
>> +static struct perf_config_item *find_config_item(const char *name,
>> +						 struct perf_config_section *section)
>> +{
>> +	struct perf_config_item *config_item;
>> +
>> +	list_for_each_entry(config_item, &section->config_items, list)
>> +		if (!strcmp(config_item->name, name))
>> +			return config_item;
>> +
>> +	return NULL;
>> +}
>> +
>> +static void find_config(struct list_head *sections,
>> +			struct perf_config_section **section,
>> +			struct perf_config_item **config_item,
>> +			const char *section_name, const char *name)
>> +{
>> +	*section = find_section(sections, section_name);
>> +
>> +	if (*section != NULL)
>> +		*config_item = find_config_item(name, *section);
>> +	else
>> +		*config_item = NULL;
>> +}
>> +
>> +static struct perf_config_section *add_section(struct list_head *sections,
>> +					       const char *section_name)
>> +{
>> +	struct perf_config_section *section = zalloc(sizeof(*section));
>> +
>> +	if (!section)
>> +		return NULL;
>> +
>> +	INIT_LIST_HEAD(&section->config_items);
>> +	section->name = strdup(section_name);
>> +	if (!section->name) {
>> +		pr_err("%s: strdup failed\n", __func__);
>> +		free(section);
>> +		return NULL;
>> +	}
>> +
>> +	list_add_tail(&section->list, sections);
>> +	return section;
>> +}
>> +
>> +static struct perf_config_item *add_config_item(struct perf_config_section *section,
>> +						const char *name)
>> +{
>> +	struct perf_config_item *config_item = zalloc(sizeof(*config_item));
>> +
>> +	if (!config_item)
>> +		return NULL;
>> +
>> +	config_item->name = strdup(name);
>> +	if (!name) {
>> +		pr_err("%s: strdup failed\n", __func__);
>> +		goto out_err;
>> +	}
>> +
>> +	list_add_tail(&config_item->list, &section->config_items);
>> +	return config_item;
>> +
>> +out_err:
>> +	free(config_item);
>> +	return NULL;
>> +}
>> +
>> +static int set_value(struct perf_config_item *config_item, const char *value)
>> +{
>> +	char *val = strdup(value);
>> +
>> +	if (!val)
>> +		return -1;
>> +
>> +	free(config_item->value);
>> +	config_item->value = val;
>> +	return 0;
>> +}
>> +
>> +static int collect_config(const char *var, const char *value,
>> +			  void *section_list)
>> +{
>> +	int ret = -1;
>> +	char *ptr, *key;
>> +	char *section_name, *name;
>> +	struct perf_config_section *section = NULL;
>> +	struct perf_config_item *config_item = NULL;
>> +	struct list_head *sections = section_list;
>> +
>> +	key = ptr = strdup(var);
>> +	if (!key) {
>> +		pr_err("%s: strdup failed\n", __func__);
>> +		return -1;
>> +	}
>> +
>> +	section_name = strsep(&ptr, ".");
>> +	name = ptr;
>> +	if (name == NULL || value == NULL)
>> +		goto out_free;
>> +
>> +	find_config(sections, &section, &config_item, section_name, name);
>> +
>> +	if (!section) {
>> +		section = add_section(sections, section_name);
>> +		if (!section)
>> +			goto out_free;
>> +	}
>> +	if (!config_item) {
>> +		config_item = add_config_item(section, name);
>> +		if (!config_item)
>> +			goto out_free;
>> +	}
>> +
>> +	ret = set_value(config_item, value);
>> +	return ret;
>> +
>> +out_free:
>> +	free(key);
>> +	return ret;
>> +}
>> +
>> +struct perf_config_set *perf_config_set__new(void)
>> +{
>> +	struct perf_config_set *perf_configs = zalloc(sizeof(*perf_configs));
>> +
>> +	if (!perf_configs)
>> +		return NULL;
>> +
>> +	INIT_LIST_HEAD(&perf_configs->sections);
>> +	perf_config(collect_config, &perf_configs->sections);
>> +
>> +	return perf_configs;
>> +}
>> +
>> +void perf_config_set__delete(struct perf_config_set *perf_configs)
>> +{
>> +	struct perf_config_section *section, *section_tmp;
>> +	struct perf_config_item *config_item, *item_tmp;
>> +
>> +	list_for_each_entry_safe(section, section_tmp,
>> +				 &perf_configs->sections, list) {
>> +		list_for_each_entry_safe(config_item, item_tmp,
>> +					 &section->config_items, list) {
>> +			list_del(&config_item->list);
>> +			free(config_item->name);
>> +			free(config_item->value);
>> +			free(config_item);
>> +		}
>> +		list_del(&section->list);
>> +		free(section->name);
>> +		free(section);
>> +	}
>> +
>> +	free(perf_configs);
>> +}
>> +
>>   /*
>>    * Call this to report error for your variable that should not
>>    * get a boolean value (i.e. "[my] var" means "true").
>> diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
>> new file mode 100644
>> index 0000000..e270e51
>> --- /dev/null
>> +++ b/tools/perf/util/config.h
>> @@ -0,0 +1,26 @@
>> +#ifndef __PERF_CONFIG_H
>> +#define __PERF_CONFIG_H
>> +
>> +#include <stdbool.h>
>> +#include <linux/list.h>
>> +
>> +struct perf_config_item {
>> +	char *name;
>> +	char *value;
>> +	struct list_head list;
>> +};
>> +
>> +struct perf_config_section {
>> +	char *name;
>> +	struct list_head config_items;
>> +	struct list_head list;
>> +};
>> +
>> +struct perf_config_set {
>> +	struct list_head sections;
>> +};
>> +
>> +struct perf_config_set *perf_config_set__new(void);
>> +void perf_config_set__delete(struct perf_config_set *perf_configs);
>> +
>> +#endif /* __PERF_CONFIG_H */
>> --
>> 2.5.0
>>

Re: [PATCH v3 1/4] perf config: Introduce perf_config_set class

[Arnaldo Carvalho de Melo]

Em Thu, Mar 24, 2016 at 03:46:00PM +0900, Taeung Song escreveu:
> Hi, Namhyung
> 
> On 03/24/2016 03:07 PM, Namhyung Kim wrote:
> >Hi Taeung,
> >
> >On Thu, Mar 24, 2016 at 05:16:30AM +0900, Taeung Song wrote:
> >>This infrastructure code was designed for
> >>upcoming features of perf-config.
> >>
> >>That collect config key-value pairs from user and
> >>system config files (i.e. user wide ~/.perfconfig
> >>and system wide $(sysconfdir)/perfconfig)
> >>to manage perf's configs.
> >>
> >>Cc: Jiri Olsa <jolsa@kernel.org>
> >>Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
> >
> >Acked-by: Namhyung Kim <namhyung@kernel.org>
> >
> 
> Thank you!!
> 
> But I'm so sorry for my mistake.
> I missed perf_config_set__delete() out when the error occur in
> collect_config().
> 
> (If returning -1 from collect_config(), perf process die with some error
> message at once
> so it is needed to call perf_config_set__delete() before that moment)
> 
> So, I'll resend this patch after fixing it as below.

Ok, when resubmitting, if no changes are made to a patch that got an
Acked-by, please add that Acked-by line,

Will wait for v4 then, thanks,

- Arnaldo
 
> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
> index 725015f..3831733 100644
> --- a/tools/perf/util/config.c
> +++ b/tools/perf/util/config.c
> @@ -705,14 +706,15 @@ static int set_value(struct perf_config_item
> *config_item, const char *value)
>  }
> 
>  static int collect_config(const char *var, const char *value,
> -                          void *section_list)
> +                          void *perf_config_set)
>  {
>          int ret = -1;
>          char *ptr, *key;
>          char *section_name, *name;
>          struct perf_config_section *section = NULL;
>          struct perf_config_item *config_item = NULL;
> -        struct list_head *sections = section_list;
> +        struct perf_config_set *perf_configs = perf_config_set;
> +        struct list_head *sections = &perf_configs->sections;
> 
>          key = ptr = strdup(var);
>          if (!key) {
> @@ -743,7 +745,8 @@ static int collect_config(const char *var, const char
> *value,
> 
>  out_free:
>          free(key);
> -        return ret;
> +        perf_config_set__delete(perf_configs);
> +        return -1;
>  }
> 
>  static struct perf_config_set *perf_config_set__init(struct perf_config_set
> *perf_configs)
> @@ -777,7 +780,7 @@ struct perf_config_set *perf_config_set__new(void)
>                  return NULL;
> 
>          perf_config_set__init(perf_configs);
> -        perf_config(collect_config, &perf_configs->sections);
> +        perf_config(collect_config, perf_configs);
> 
>          return perf_configs;
>  }
> 
> 
> Thanks,
> Taeung
> 
> >
> >
> >>---
> >>  tools/perf/util/config.c | 169 +++++++++++++++++++++++++++++++++++++++++++++++
> >>  tools/perf/util/config.h |  26 ++++++++
> >>  2 files changed, 195 insertions(+)
> >>  create mode 100644 tools/perf/util/config.h
> >>
> >>diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
> >>index 4e72763..3beeceb 100644
> >>--- a/tools/perf/util/config.c
> >>+++ b/tools/perf/util/config.c
> >>@@ -13,6 +13,7 @@
> >>  #include <subcmd/exec-cmd.h>
> >>  #include "util/hist.h"  /* perf_hist_config */
> >>  #include "util/llvm-utils.h"   /* perf_llvm_config */
> >>+#include "config.h"
> >>
> >>  #define MAXNAME (256)
> >>
> >>@@ -506,6 +507,174 @@ out:
> >>  	return ret;
> >>  }
> >>
> >>+static struct perf_config_section *find_section(struct list_head *sections,
> >>+						const char *section_name)
> >>+{
> >>+	struct perf_config_section *section;
> >>+
> >>+	list_for_each_entry(section, sections, list)
> >>+		if (!strcmp(section->name, section_name))
> >>+			return section;
> >>+
> >>+	return NULL;
> >>+}
> >>+
> >>+static struct perf_config_item *find_config_item(const char *name,
> >>+						 struct perf_config_section *section)
> >>+{
> >>+	struct perf_config_item *config_item;
> >>+
> >>+	list_for_each_entry(config_item, &section->config_items, list)
> >>+		if (!strcmp(config_item->name, name))
> >>+			return config_item;
> >>+
> >>+	return NULL;
> >>+}
> >>+
> >>+static void find_config(struct list_head *sections,
> >>+			struct perf_config_section **section,
> >>+			struct perf_config_item **config_item,
> >>+			const char *section_name, const char *name)
> >>+{
> >>+	*section = find_section(sections, section_name);
> >>+
> >>+	if (*section != NULL)
> >>+		*config_item = find_config_item(name, *section);
> >>+	else
> >>+		*config_item = NULL;
> >>+}
> >>+
> >>+static struct perf_config_section *add_section(struct list_head *sections,
> >>+					       const char *section_name)
> >>+{
> >>+	struct perf_config_section *section = zalloc(sizeof(*section));
> >>+
> >>+	if (!section)
> >>+		return NULL;
> >>+
> >>+	INIT_LIST_HEAD(&section->config_items);
> >>+	section->name = strdup(section_name);
> >>+	if (!section->name) {
> >>+		pr_err("%s: strdup failed\n", __func__);
> >>+		free(section);
> >>+		return NULL;
> >>+	}
> >>+
> >>+	list_add_tail(&section->list, sections);
> >>+	return section;
> >>+}
> >>+
> >>+static struct perf_config_item *add_config_item(struct perf_config_section *section,
> >>+						const char *name)
> >>+{
> >>+	struct perf_config_item *config_item = zalloc(sizeof(*config_item));
> >>+
> >>+	if (!config_item)
> >>+		return NULL;
> >>+
> >>+	config_item->name = strdup(name);
> >>+	if (!name) {
> >>+		pr_err("%s: strdup failed\n", __func__);
> >>+		goto out_err;
> >>+	}
> >>+
> >>+	list_add_tail(&config_item->list, &section->config_items);
> >>+	return config_item;
> >>+
> >>+out_err:
> >>+	free(config_item);
> >>+	return NULL;
> >>+}
> >>+
> >>+static int set_value(struct perf_config_item *config_item, const char *value)
> >>+{
> >>+	char *val = strdup(value);
> >>+
> >>+	if (!val)
> >>+		return -1;
> >>+
> >>+	free(config_item->value);
> >>+	config_item->value = val;
> >>+	return 0;
> >>+}
> >>+
> >>+static int collect_config(const char *var, const char *value,
> >>+			  void *section_list)
> >>+{
> >>+	int ret = -1;
> >>+	char *ptr, *key;
> >>+	char *section_name, *name;
> >>+	struct perf_config_section *section = NULL;
> >>+	struct perf_config_item *config_item = NULL;
> >>+	struct list_head *sections = section_list;
> >>+
> >>+	key = ptr = strdup(var);
> >>+	if (!key) {
> >>+		pr_err("%s: strdup failed\n", __func__);
> >>+		return -1;
> >>+	}
> >>+
> >>+	section_name = strsep(&ptr, ".");
> >>+	name = ptr;
> >>+	if (name == NULL || value == NULL)
> >>+		goto out_free;
> >>+
> >>+	find_config(sections, &section, &config_item, section_name, name);
> >>+
> >>+	if (!section) {
> >>+		section = add_section(sections, section_name);
> >>+		if (!section)
> >>+			goto out_free;
> >>+	}
> >>+	if (!config_item) {
> >>+		config_item = add_config_item(section, name);
> >>+		if (!config_item)
> >>+			goto out_free;
> >>+	}
> >>+
> >>+	ret = set_value(config_item, value);
> >>+	return ret;
> >>+
> >>+out_free:
> >>+	free(key);
> >>+	return ret;
> >>+}
> >>+
> >>+struct perf_config_set *perf_config_set__new(void)
> >>+{
> >>+	struct perf_config_set *perf_configs = zalloc(sizeof(*perf_configs));
> >>+
> >>+	if (!perf_configs)
> >>+		return NULL;
> >>+
> >>+	INIT_LIST_HEAD(&perf_configs->sections);
> >>+	perf_config(collect_config, &perf_configs->sections);
> >>+
> >>+	return perf_configs;
> >>+}
> >>+
> >>+void perf_config_set__delete(struct perf_config_set *perf_configs)
> >>+{
> >>+	struct perf_config_section *section, *section_tmp;
> >>+	struct perf_config_item *config_item, *item_tmp;
> >>+
> >>+	list_for_each_entry_safe(section, section_tmp,
> >>+				 &perf_configs->sections, list) {
> >>+		list_for_each_entry_safe(config_item, item_tmp,
> >>+					 &section->config_items, list) {
> >>+			list_del(&config_item->list);
> >>+			free(config_item->name);
> >>+			free(config_item->value);
> >>+			free(config_item);
> >>+		}
> >>+		list_del(&section->list);
> >>+		free(section->name);
> >>+		free(section);
> >>+	}
> >>+
> >>+	free(perf_configs);
> >>+}
> >>+
> >>  /*
> >>   * Call this to report error for your variable that should not
> >>   * get a boolean value (i.e. "[my] var" means "true").
> >>diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
> >>new file mode 100644
> >>index 0000000..e270e51
> >>--- /dev/null
> >>+++ b/tools/perf/util/config.h
> >>@@ -0,0 +1,26 @@
> >>+#ifndef __PERF_CONFIG_H
> >>+#define __PERF_CONFIG_H
> >>+
> >>+#include <stdbool.h>
> >>+#include <linux/list.h>
> >>+
> >>+struct perf_config_item {
> >>+	char *name;
> >>+	char *value;
> >>+	struct list_head list;
> >>+};
> >>+
> >>+struct perf_config_section {
> >>+	char *name;
> >>+	struct list_head config_items;
> >>+	struct list_head list;
> >>+};
> >>+
> >>+struct perf_config_set {
> >>+	struct list_head sections;
> >>+};
> >>+
> >>+struct perf_config_set *perf_config_set__new(void);
> >>+void perf_config_set__delete(struct perf_config_set *perf_configs);
> >>+
> >>+#endif /* __PERF_CONFIG_H */
> >>--
> >>2.5.0
> >>

Re: [PATCH v3 1/4] perf config: Introduce perf_config_set class

[Taeung Song]

On 03/29/2016 06:06 AM, Arnaldo Carvalho de Melo wrote:
> Em Thu, Mar 24, 2016 at 03:46:00PM +0900, Taeung Song escreveu:
>> Hi, Namhyung
>>
>> On 03/24/2016 03:07 PM, Namhyung Kim wrote:
>>> Hi Taeung,
>>>
>>> On Thu, Mar 24, 2016 at 05:16:30AM +0900, Taeung Song wrote:
>>>> This infrastructure code was designed for
>>>> upcoming features of perf-config.
>>>>
>>>> That collect config key-value pairs from user and
>>>> system config files (i.e. user wide ~/.perfconfig
>>>> and system wide $(sysconfdir)/perfconfig)
>>>> to manage perf's configs.
>>>>
>>>> Cc: Jiri Olsa <jolsa@kernel.org>
>>>> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
>>>
>>> Acked-by: Namhyung Kim <namhyung@kernel.org>
>>>
>>
>> Thank you!!
>>
>> But I'm so sorry for my mistake.
>> I missed perf_config_set__delete() out when the error occur in
>> collect_config().
>>
>> (If returning -1 from collect_config(), perf process die with some error
>> message at once
>> so it is needed to call perf_config_set__delete() before that moment)
>>
>> So, I'll resend this patch after fixing it as below.
>
> Ok, when resubmitting, if no changes are made to a patch that got an
> Acked-by, please add that Acked-by line,
>
> Will wait for v4 then, thanks,
>

I got it.
I hastily resent this patch after modifying it..

I'll resend the v4 patchset that contains Acked-by.

Thanks,
Taeung

>
>> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
>> index 725015f..3831733 100644
>> --- a/tools/perf/util/config.c
>> +++ b/tools/perf/util/config.c
>> @@ -705,14 +706,15 @@ static int set_value(struct perf_config_item
>> *config_item, const char *value)
>>   }
>>
>>   static int collect_config(const char *var, const char *value,
>> -                          void *section_list)
>> +                          void *perf_config_set)
>>   {
>>           int ret = -1;
>>           char *ptr, *key;
>>           char *section_name, *name;
>>           struct perf_config_section *section = NULL;
>>           struct perf_config_item *config_item = NULL;
>> -        struct list_head *sections = section_list;
>> +        struct perf_config_set *perf_configs = perf_config_set;
>> +        struct list_head *sections = &perf_configs->sections;
>>
>>           key = ptr = strdup(var);
>>           if (!key) {
>> @@ -743,7 +745,8 @@ static int collect_config(const char *var, const char
>> *value,
>>
>>   out_free:
>>           free(key);
>> -        return ret;
>> +        perf_config_set__delete(perf_configs);
>> +        return -1;
>>   }
>>
>>   static struct perf_config_set *perf_config_set__init(struct perf_config_set
>> *perf_configs)
>> @@ -777,7 +780,7 @@ struct perf_config_set *perf_config_set__new(void)
>>                   return NULL;
>>
>>           perf_config_set__init(perf_configs);
>> -        perf_config(collect_config, &perf_configs->sections);
>> +        perf_config(collect_config, perf_configs);
>>
>>           return perf_configs;
>>   }
>>
>>
>> Thanks,
>> Taeung
>>
>>>
>>>
>>>> ---
>>>>   tools/perf/util/config.c | 169 +++++++++++++++++++++++++++++++++++++++++++++++
>>>>   tools/perf/util/config.h |  26 ++++++++
>>>>   2 files changed, 195 insertions(+)
>>>>   create mode 100644 tools/perf/util/config.h
>>>>
>>>> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
>>>> index 4e72763..3beeceb 100644
>>>> --- a/tools/perf/util/config.c
>>>> +++ b/tools/perf/util/config.c
>>>> @@ -13,6 +13,7 @@
>>>>   #include <subcmd/exec-cmd.h>
>>>>   #include "util/hist.h"  /* perf_hist_config */
>>>>   #include "util/llvm-utils.h"   /* perf_llvm_config */
>>>> +#include "config.h"
>>>>
>>>>   #define MAXNAME (256)
>>>>
>>>> @@ -506,6 +507,174 @@ out:
>>>>   	return ret;
>>>>   }
>>>>
>>>> +static struct perf_config_section *find_section(struct list_head *sections,
>>>> +						const char *section_name)
>>>> +{
>>>> +	struct perf_config_section *section;
>>>> +
>>>> +	list_for_each_entry(section, sections, list)
>>>> +		if (!strcmp(section->name, section_name))
>>>> +			return section;
>>>> +
>>>> +	return NULL;
>>>> +}
>>>> +
>>>> +static struct perf_config_item *find_config_item(const char *name,
>>>> +						 struct perf_config_section *section)
>>>> +{
>>>> +	struct perf_config_item *config_item;
>>>> +
>>>> +	list_for_each_entry(config_item, &section->config_items, list)
>>>> +		if (!strcmp(config_item->name, name))
>>>> +			return config_item;
>>>> +
>>>> +	return NULL;
>>>> +}
>>>> +
>>>> +static void find_config(struct list_head *sections,
>>>> +			struct perf_config_section **section,
>>>> +			struct perf_config_item **config_item,
>>>> +			const char *section_name, const char *name)
>>>> +{
>>>> +	*section = find_section(sections, section_name);
>>>> +
>>>> +	if (*section != NULL)
>>>> +		*config_item = find_config_item(name, *section);
>>>> +	else
>>>> +		*config_item = NULL;
>>>> +}
>>>> +
>>>> +static struct perf_config_section *add_section(struct list_head *sections,
>>>> +					       const char *section_name)
>>>> +{
>>>> +	struct perf_config_section *section = zalloc(sizeof(*section));
>>>> +
>>>> +	if (!section)
>>>> +		return NULL;
>>>> +
>>>> +	INIT_LIST_HEAD(&section->config_items);
>>>> +	section->name = strdup(section_name);
>>>> +	if (!section->name) {
>>>> +		pr_err("%s: strdup failed\n", __func__);
>>>> +		free(section);
>>>> +		return NULL;
>>>> +	}
>>>> +
>>>> +	list_add_tail(&section->list, sections);
>>>> +	return section;
>>>> +}
>>>> +
>>>> +static struct perf_config_item *add_config_item(struct perf_config_section *section,
>>>> +						const char *name)
>>>> +{
>>>> +	struct perf_config_item *config_item = zalloc(sizeof(*config_item));
>>>> +
>>>> +	if (!config_item)
>>>> +		return NULL;
>>>> +
>>>> +	config_item->name = strdup(name);
>>>> +	if (!name) {
>>>> +		pr_err("%s: strdup failed\n", __func__);
>>>> +		goto out_err;
>>>> +	}
>>>> +
>>>> +	list_add_tail(&config_item->list, &section->config_items);
>>>> +	return config_item;
>>>> +
>>>> +out_err:
>>>> +	free(config_item);
>>>> +	return NULL;
>>>> +}
>>>> +
>>>> +static int set_value(struct perf_config_item *config_item, const char *value)
>>>> +{
>>>> +	char *val = strdup(value);
>>>> +
>>>> +	if (!val)
>>>> +		return -1;
>>>> +
>>>> +	free(config_item->value);
>>>> +	config_item->value = val;
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static int collect_config(const char *var, const char *value,
>>>> +			  void *section_list)
>>>> +{
>>>> +	int ret = -1;
>>>> +	char *ptr, *key;
>>>> +	char *section_name, *name;
>>>> +	struct perf_config_section *section = NULL;
>>>> +	struct perf_config_item *config_item = NULL;
>>>> +	struct list_head *sections = section_list;
>>>> +
>>>> +	key = ptr = strdup(var);
>>>> +	if (!key) {
>>>> +		pr_err("%s: strdup failed\n", __func__);
>>>> +		return -1;
>>>> +	}
>>>> +
>>>> +	section_name = strsep(&ptr, ".");
>>>> +	name = ptr;
>>>> +	if (name == NULL || value == NULL)
>>>> +		goto out_free;
>>>> +
>>>> +	find_config(sections, &section, &config_item, section_name, name);
>>>> +
>>>> +	if (!section) {
>>>> +		section = add_section(sections, section_name);
>>>> +		if (!section)
>>>> +			goto out_free;
>>>> +	}
>>>> +	if (!config_item) {
>>>> +		config_item = add_config_item(section, name);
>>>> +		if (!config_item)
>>>> +			goto out_free;
>>>> +	}
>>>> +
>>>> +	ret = set_value(config_item, value);
>>>> +	return ret;
>>>> +
>>>> +out_free:
>>>> +	free(key);
>>>> +	return ret;
>>>> +}
>>>> +
>>>> +struct perf_config_set *perf_config_set__new(void)
>>>> +{
>>>> +	struct perf_config_set *perf_configs = zalloc(sizeof(*perf_configs));
>>>> +
>>>> +	if (!perf_configs)
>>>> +		return NULL;
>>>> +
>>>> +	INIT_LIST_HEAD(&perf_configs->sections);
>>>> +	perf_config(collect_config, &perf_configs->sections);
>>>> +
>>>> +	return perf_configs;
>>>> +}
>>>> +
>>>> +void perf_config_set__delete(struct perf_config_set *perf_configs)
>>>> +{
>>>> +	struct perf_config_section *section, *section_tmp;
>>>> +	struct perf_config_item *config_item, *item_tmp;
>>>> +
>>>> +	list_for_each_entry_safe(section, section_tmp,
>>>> +				 &perf_configs->sections, list) {
>>>> +		list_for_each_entry_safe(config_item, item_tmp,
>>>> +					 &section->config_items, list) {
>>>> +			list_del(&config_item->list);
>>>> +			free(config_item->name);
>>>> +			free(config_item->value);
>>>> +			free(config_item);
>>>> +		}
>>>> +		list_del(&section->list);
>>>> +		free(section->name);
>>>> +		free(section);
>>>> +	}
>>>> +
>>>> +	free(perf_configs);
>>>> +}
>>>> +
>>>>   /*
>>>>    * Call this to report error for your variable that should not
>>>>    * get a boolean value (i.e. "[my] var" means "true").
>>>> diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
>>>> new file mode 100644
>>>> index 0000000..e270e51
>>>> --- /dev/null
>>>> +++ b/tools/perf/util/config.h
>>>> @@ -0,0 +1,26 @@
>>>> +#ifndef __PERF_CONFIG_H
>>>> +#define __PERF_CONFIG_H
>>>> +
>>>> +#include <stdbool.h>
>>>> +#include <linux/list.h>
>>>> +
>>>> +struct perf_config_item {
>>>> +	char *name;
>>>> +	char *value;
>>>> +	struct list_head list;
>>>> +};
>>>> +
>>>> +struct perf_config_section {
>>>> +	char *name;
>>>> +	struct list_head config_items;
>>>> +	struct list_head list;
>>>> +};
>>>> +
>>>> +struct perf_config_set {
>>>> +	struct list_head sections;
>>>> +};
>>>> +
>>>> +struct perf_config_set *perf_config_set__new(void);
>>>> +void perf_config_set__delete(struct perf_config_set *perf_configs);
>>>> +
>>>> +#endif /* __PERF_CONFIG_H */
>>>> --
>>>> 2.5.0
>>>>

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

[Taeung Song]

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

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

[Namhyung Kim]

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
> 

[PATCH v3 3/4] perf config: Prepare all default configs

[Taeung Song]

To precisely manage configs,
prepare all default perf's configs that contain
default section name, variable name, value
and correct type, not string type.

In the near future, this will be used when
checking type of config variable or showing
all configs with default values, etc.

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/util/config.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++-
 tools/perf/util/config.h |  62 +++++++++++++++++++++++++-
 2 files changed, 168 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index 3beeceb..e3e6ef4 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -15,6 +15,7 @@
 #include "util/llvm-utils.h"   /* perf_llvm_config */
 #include "config.h"
 
+#define MAX_CONFIGS 64
 #define MAXNAME (256)
 
 #define DEBUG_CACHE_DIR ".debug"
@@ -29,6 +30,111 @@ static int config_file_eof;
 
 const char *config_exclusive_filename;
 
+struct perf_config_section default_sections[] = {
+	{ .name = "colors" },
+	{ .name = "tui" },
+	{ .name = "buildid" },
+	{ .name = "annotate" },
+	{ .name = "gtk" },
+	{ .name = "pager" },
+	{ .name = "help" },
+	{ .name = "hist" },
+	{ .name = "ui" },
+	{ .name = "call-graph" },
+	{ .name = "report" },
+	{ .name = "top" },
+	{ .name = "man" },
+	{ .name = "kmem" }
+};
+
+struct perf_config_item default_config_items[][MAX_CONFIGS] = {
+	[CONFIG_COLORS] = {
+		CONF_STR_VAR("top", "red, default"),
+		CONF_STR_VAR("medium", "green, default"),
+		CONF_STR_VAR("normal", "lightgray, default"),
+		CONF_STR_VAR("selected", "white, lightgray"),
+		CONF_STR_VAR("jump_arrows", "blue, default"),
+		CONF_STR_VAR("addr", "magenta, default"),
+		CONF_STR_VAR("root", "white, blue"),
+		CONF_END()
+	},
+	[CONFIG_TUI] = {
+		CONF_BOOL_VAR("report", true),
+		CONF_BOOL_VAR("annotate", true),
+		CONF_BOOL_VAR("top", true),
+		CONF_END()
+	},
+	[CONFIG_BUILDID] = {
+		CONF_STR_VAR("dir", "~/.debug"),
+		CONF_END()
+	},
+	[CONFIG_ANNOTATE] = {
+		CONF_BOOL_VAR("hide_src_code", false),
+		CONF_BOOL_VAR("use_offset", true),
+		CONF_BOOL_VAR("jump_arrows", true),
+		CONF_BOOL_VAR("show_nr_jumps", false),
+		CONF_BOOL_VAR("show_linenr", false),
+		CONF_BOOL_VAR("show_total_period", false),
+		CONF_END()
+	},
+	[CONFIG_GTK] = {
+		CONF_BOOL_VAR("annotate", false),
+		CONF_BOOL_VAR("report", false),
+		CONF_BOOL_VAR("top", false),
+		CONF_END()
+	},
+	[CONFIG_PAGER] = {
+		CONF_BOOL_VAR("cmd", true),
+		CONF_BOOL_VAR("report", true),
+		CONF_BOOL_VAR("annotate", true),
+		CONF_BOOL_VAR("top", true),
+		CONF_BOOL_VAR("diff", true),
+		CONF_END()
+	},
+	[CONFIG_HELP] = {
+		CONF_STR_VAR("format", "man"),
+		CONF_INT_VAR("autocorrect", 0),
+		CONF_END()
+	},
+	[CONFIG_HIST] = {
+		CONF_STR_VAR("percentage", "absolute"),
+		CONF_END()
+	},
+	[CONFIG_UI] = {
+		CONF_BOOL_VAR("show-headers", true),
+		CONF_END()
+	},
+	[CONFIG_CALL_GRAPH] = {
+		CONF_STR_VAR("record-mode", "fp"),
+		CONF_LONG_VAR("dump-size", 8192),
+		CONF_STR_VAR("print-type", "graph"),
+		CONF_STR_VAR("order", "callee"),
+		CONF_STR_VAR("sort-key", "function"),
+		CONF_DOUBLE_VAR("threshold", 0.5),
+		CONF_LONG_VAR("print-limit", 0),
+		CONF_END()
+	},
+	[CONFIG_REPORT] = {
+		CONF_BOOL_VAR("group", true),
+		CONF_BOOL_VAR("children", true),
+		CONF_FLOAT_VAR("percent-limit", 0),
+		CONF_U64_VAR("queue-size", 0),
+		CONF_END()
+	},
+	[CONFIG_TOP] = {
+		CONF_BOOL_VAR("children", true),
+		CONF_END()
+	},
+	[CONFIG_MAN] = {
+		CONF_STR_VAR("viewer", "man"),
+		CONF_END()
+	},
+	[CONFIG_KMEM] = {
+		CONF_STR_VAR("default", "slab"),
+		CONF_END()
+	}
+};
+
 static int get_next_char(void)
 {
 	int c;
@@ -663,12 +769,12 @@ void perf_config_set__delete(struct perf_config_set *perf_configs)
 		list_for_each_entry_safe(config_item, item_tmp,
 					 &section->config_items, list) {
 			list_del(&config_item->list);
-			free(config_item->name);
+			free((char *)config_item->name);
 			free(config_item->value);
 			free(config_item);
 		}
 		list_del(&section->list);
-		free(section->name);
+		free((char *)section->name);
 		free(section);
 	}
 
diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
index e270e51..aa4a5a2 100644
--- a/tools/perf/util/config.h
+++ b/tools/perf/util/config.h
@@ -4,14 +4,34 @@
 #include <stdbool.h>
 #include <linux/list.h>
 
+enum perf_config_type {
+	CONFIG_TYPE_BOOL,
+	CONFIG_TYPE_INT,
+	CONFIG_TYPE_LONG,
+	CONFIG_TYPE_U64,
+	CONFIG_TYPE_FLOAT,
+	CONFIG_TYPE_DOUBLE,
+	CONFIG_TYPE_STRING
+};
+
 struct perf_config_item {
-	char *name;
+	const char *name;
 	char *value;
+	union {
+		bool b;
+		int i;
+		u32 l;
+		u64 ll;
+		float f;
+		double d;
+		const char *s;
+	} default_value;
+	enum perf_config_type type;
 	struct list_head list;
 };
 
 struct perf_config_section {
-	char *name;
+	const char *name;
 	struct list_head config_items;
 	struct list_head list;
 };
@@ -20,6 +40,44 @@ struct perf_config_set {
 	struct list_head sections;
 };
 
+enum perf_config_secion_idx {
+	CONFIG_COLORS,
+	CONFIG_TUI,
+	CONFIG_BUILDID,
+	CONFIG_ANNOTATE,
+	CONFIG_GTK,
+	CONFIG_PAGER,
+	CONFIG_HELP,
+	CONFIG_HIST,
+	CONFIG_UI,
+	CONFIG_CALL_GRAPH,
+	CONFIG_REPORT,
+	CONFIG_TOP,
+	CONFIG_MAN,
+	CONFIG_KMEM,
+	CONFIG_END
+};
+
+#define CONF_VAR(_name, _field, _val, _type)				\
+	{ .name = _name, .default_value._field = _val, .type = _type }
+
+#define CONF_BOOL_VAR(_name, _val)			\
+	CONF_VAR(_name, b, _val, CONFIG_TYPE_BOOL)
+#define CONF_INT_VAR(_name, _val)			\
+	CONF_VAR(_name, i, _val, CONFIG_TYPE_INT)
+#define CONF_LONG_VAR(_name, _val)			\
+	CONF_VAR(_name, l, _val, CONFIG_TYPE_LONG)
+#define CONF_U64_VAR(_name, _val)			\
+	CONF_VAR(_name, ll, _val, CONFIG_TYPE_U64)
+#define CONF_FLOAT_VAR(_name, _val)			\
+	CONF_VAR(_name, f, _val, CONFIG_TYPE_FLOAT)
+#define CONF_DOUBLE_VAR(_name, _val)			\
+	CONF_VAR(_name, d, _val, CONFIG_TYPE_DOUBLE)
+#define CONF_STR_VAR(_name, _val)			\
+	CONF_VAR(_name, s, _val, CONFIG_TYPE_STRING)
+#define CONF_END()					\
+	{ .name = NULL }
+
 struct perf_config_set *perf_config_set__new(void);
 void perf_config_set__delete(struct perf_config_set *perf_configs);
 
-- 
2.5.0

Re: [PATCH v3 3/4] perf config: Prepare all default configs

[Namhyung Kim]

On Thu, Mar 24, 2016 at 05:16:32AM +0900, Taeung Song wrote:
> To precisely manage configs,
> prepare all default perf's configs that contain
> default section name, variable name, value
> and correct type, not string type.
> 
> In the near future, this will be used when
> checking type of config variable or showing
> all configs with default values, etc.
> 
> 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/util/config.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++-
>  tools/perf/util/config.h |  62 +++++++++++++++++++++++++-
>  2 files changed, 168 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
> index 3beeceb..e3e6ef4 100644
> --- a/tools/perf/util/config.c
> +++ b/tools/perf/util/config.c
> @@ -15,6 +15,7 @@
>  #include "util/llvm-utils.h"   /* perf_llvm_config */
>  #include "config.h"
>  
> +#define MAX_CONFIGS 64
>  #define MAXNAME (256)
>  
>  #define DEBUG_CACHE_DIR ".debug"
> @@ -29,6 +30,111 @@ static int config_file_eof;
>  
>  const char *config_exclusive_filename;
>  
> +struct perf_config_section default_sections[] = {
> +	{ .name = "colors" },
> +	{ .name = "tui" },
> +	{ .name = "buildid" },
> +	{ .name = "annotate" },
> +	{ .name = "gtk" },
> +	{ .name = "pager" },
> +	{ .name = "help" },
> +	{ .name = "hist" },
> +	{ .name = "ui" },
> +	{ .name = "call-graph" },
> +	{ .name = "report" },
> +	{ .name = "top" },
> +	{ .name = "man" },
> +	{ .name = "kmem" }
> +};
> +
> +struct perf_config_item default_config_items[][MAX_CONFIGS] = {
> +	[CONFIG_COLORS] = {
> +		CONF_STR_VAR("top", "red, default"),
> +		CONF_STR_VAR("medium", "green, default"),
> +		CONF_STR_VAR("normal", "lightgray, default"),
> +		CONF_STR_VAR("selected", "white, lightgray"),
> +		CONF_STR_VAR("jump_arrows", "blue, default"),
> +		CONF_STR_VAR("addr", "magenta, default"),
> +		CONF_STR_VAR("root", "white, blue"),
> +		CONF_END()
> +	},
> +	[CONFIG_TUI] = {
> +		CONF_BOOL_VAR("report", true),
> +		CONF_BOOL_VAR("annotate", true),
> +		CONF_BOOL_VAR("top", true),
> +		CONF_END()
> +	},
> +	[CONFIG_BUILDID] = {
> +		CONF_STR_VAR("dir", "~/.debug"),
> +		CONF_END()
> +	},
> +	[CONFIG_ANNOTATE] = {
> +		CONF_BOOL_VAR("hide_src_code", false),
> +		CONF_BOOL_VAR("use_offset", true),
> +		CONF_BOOL_VAR("jump_arrows", true),
> +		CONF_BOOL_VAR("show_nr_jumps", false),
> +		CONF_BOOL_VAR("show_linenr", false),
> +		CONF_BOOL_VAR("show_total_period", false),
> +		CONF_END()
> +	},
> +	[CONFIG_GTK] = {
> +		CONF_BOOL_VAR("annotate", false),
> +		CONF_BOOL_VAR("report", false),
> +		CONF_BOOL_VAR("top", false),
> +		CONF_END()
> +	},
> +	[CONFIG_PAGER] = {
> +		CONF_BOOL_VAR("cmd", true),
> +		CONF_BOOL_VAR("report", true),
> +		CONF_BOOL_VAR("annotate", true),
> +		CONF_BOOL_VAR("top", true),
> +		CONF_BOOL_VAR("diff", true),
> +		CONF_END()
> +	},
> +	[CONFIG_HELP] = {
> +		CONF_STR_VAR("format", "man"),
> +		CONF_INT_VAR("autocorrect", 0),
> +		CONF_END()
> +	},
> +	[CONFIG_HIST] = {
> +		CONF_STR_VAR("percentage", "absolute"),
> +		CONF_END()
> +	},
> +	[CONFIG_UI] = {
> +		CONF_BOOL_VAR("show-headers", true),
> +		CONF_END()
> +	},
> +	[CONFIG_CALL_GRAPH] = {
> +		CONF_STR_VAR("record-mode", "fp"),
> +		CONF_LONG_VAR("dump-size", 8192),
> +		CONF_STR_VAR("print-type", "graph"),
> +		CONF_STR_VAR("order", "callee"),
> +		CONF_STR_VAR("sort-key", "function"),
> +		CONF_DOUBLE_VAR("threshold", 0.5),
> +		CONF_LONG_VAR("print-limit", 0),
> +		CONF_END()
> +	},
> +	[CONFIG_REPORT] = {
> +		CONF_BOOL_VAR("group", true),
> +		CONF_BOOL_VAR("children", true),
> +		CONF_FLOAT_VAR("percent-limit", 0),
> +		CONF_U64_VAR("queue-size", 0),
> +		CONF_END()
> +	},
> +	[CONFIG_TOP] = {
> +		CONF_BOOL_VAR("children", true),
> +		CONF_END()
> +	},
> +	[CONFIG_MAN] = {
> +		CONF_STR_VAR("viewer", "man"),
> +		CONF_END()
> +	},
> +	[CONFIG_KMEM] = {
> +		CONF_STR_VAR("default", "slab"),
> +		CONF_END()
> +	}
> +};
> +
>  static int get_next_char(void)
>  {
>  	int c;
> @@ -663,12 +769,12 @@ void perf_config_set__delete(struct perf_config_set *perf_configs)
>  		list_for_each_entry_safe(config_item, item_tmp,
>  					 &section->config_items, list) {
>  			list_del(&config_item->list);
> -			free(config_item->name);
> +			free((char *)config_item->name);
>  			free(config_item->value);
>  			free(config_item);
>  		}
>  		list_del(&section->list);
> -		free(section->name);
> +		free((char *)section->name);
>  		free(section);
>  	}
>  
> diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
> index e270e51..aa4a5a2 100644
> --- a/tools/perf/util/config.h
> +++ b/tools/perf/util/config.h
> @@ -4,14 +4,34 @@
>  #include <stdbool.h>
>  #include <linux/list.h>
>  
> +enum perf_config_type {
> +	CONFIG_TYPE_BOOL,
> +	CONFIG_TYPE_INT,
> +	CONFIG_TYPE_LONG,
> +	CONFIG_TYPE_U64,
> +	CONFIG_TYPE_FLOAT,
> +	CONFIG_TYPE_DOUBLE,
> +	CONFIG_TYPE_STRING
> +};
> +
>  struct perf_config_item {
> -	char *name;
> +	const char *name;
>  	char *value;
> +	union {
> +		bool b;
> +		int i;
> +		u32 l;
> +		u64 ll;
> +		float f;
> +		double d;
> +		const char *s;
> +	} default_value;
> +	enum perf_config_type type;
>  	struct list_head list;
>  };
>  
>  struct perf_config_section {
> -	char *name;
> +	const char *name;
>  	struct list_head config_items;
>  	struct list_head list;
>  };
> @@ -20,6 +40,44 @@ struct perf_config_set {
>  	struct list_head sections;
>  };
>  
> +enum perf_config_secion_idx {
> +	CONFIG_COLORS,
> +	CONFIG_TUI,
> +	CONFIG_BUILDID,
> +	CONFIG_ANNOTATE,
> +	CONFIG_GTK,
> +	CONFIG_PAGER,
> +	CONFIG_HELP,
> +	CONFIG_HIST,
> +	CONFIG_UI,
> +	CONFIG_CALL_GRAPH,
> +	CONFIG_REPORT,
> +	CONFIG_TOP,
> +	CONFIG_MAN,
> +	CONFIG_KMEM,
> +	CONFIG_END
> +};
> +
> +#define CONF_VAR(_name, _field, _val, _type)				\
> +	{ .name = _name, .default_value._field = _val, .type = _type }
> +
> +#define CONF_BOOL_VAR(_name, _val)			\
> +	CONF_VAR(_name, b, _val, CONFIG_TYPE_BOOL)
> +#define CONF_INT_VAR(_name, _val)			\
> +	CONF_VAR(_name, i, _val, CONFIG_TYPE_INT)
> +#define CONF_LONG_VAR(_name, _val)			\
> +	CONF_VAR(_name, l, _val, CONFIG_TYPE_LONG)
> +#define CONF_U64_VAR(_name, _val)			\
> +	CONF_VAR(_name, ll, _val, CONFIG_TYPE_U64)
> +#define CONF_FLOAT_VAR(_name, _val)			\
> +	CONF_VAR(_name, f, _val, CONFIG_TYPE_FLOAT)
> +#define CONF_DOUBLE_VAR(_name, _val)			\
> +	CONF_VAR(_name, d, _val, CONFIG_TYPE_DOUBLE)
> +#define CONF_STR_VAR(_name, _val)			\
> +	CONF_VAR(_name, s, _val, CONFIG_TYPE_STRING)
> +#define CONF_END()					\
> +	{ .name = NULL }
> +
>  struct perf_config_set *perf_config_set__new(void);
>  void perf_config_set__delete(struct perf_config_set *perf_configs);
>  
> -- 
> 2.5.0
> 

[PATCH v3 4/4] perf config: Initialize perf_config_set with all default configs

[Taeung Song]

To avoid duplicated config variables and
use perf_config_set classifying between standard
perf config variables and unknown or new config
variables other than them, initialize perf_config_set
with all default configs.

And this will be needed when showing all configs with
default value or checking correct type of a config
variable in the near future.

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 | 11 +++++++----
 tools/perf/util/config.c    | 35 +++++++++++++++++++++++++++++------
 tools/perf/util/config.h    |  6 ++++++
 3 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
index c7cf34f..9e2adad 100644
--- a/tools/perf/builtin-config.c
+++ b/tools/perf/builtin-config.c
@@ -35,23 +35,26 @@ static struct option config_options[] = {
 
 static int show_config(struct perf_config_set *perf_configs)
 {
+	bool has_value = false;
 	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)
+			if (value) {
 				printf("%s.%s=%s\n", section->name,
 				       config_item->name, value);
+				has_value = true;
+			}
 		}
 	}
 
+	if (!has_value)
+		return -1;
+
 	return 0;
 }
 
diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index e3e6ef4..725015f 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -746,6 +746,29 @@ out_free:
 	return ret;
 }
 
+static struct perf_config_set *perf_config_set__init(struct perf_config_set *perf_configs)
+{
+	int i, j;
+	struct perf_config_section *section;
+	struct perf_config_item *config_items;
+	struct list_head *sections = &perf_configs->sections;
+
+	INIT_LIST_HEAD(&perf_configs->sections);
+
+	for (i = 0; i != CONFIG_END; i++) {
+		section = &default_sections[i];
+		INIT_LIST_HEAD(&section->config_items);
+
+		config_items = default_config_items[i];
+		for (j = 0; config_items[j].name != NULL; j++)
+			list_add_tail(&config_items[j].list, &section->config_items);
+
+		list_add_tail(&section->list, sections);
+	}
+
+	return perf_configs;
+}
+
 struct perf_config_set *perf_config_set__new(void)
 {
 	struct perf_config_set *perf_configs = zalloc(sizeof(*perf_configs));
@@ -753,7 +776,7 @@ struct perf_config_set *perf_config_set__new(void)
 	if (!perf_configs)
 		return NULL;
 
-	INIT_LIST_HEAD(&perf_configs->sections);
+	perf_config_set__init(perf_configs);
 	perf_config(collect_config, &perf_configs->sections);
 
 	return perf_configs;
@@ -769,13 +792,13 @@ void perf_config_set__delete(struct perf_config_set *perf_configs)
 		list_for_each_entry_safe(config_item, item_tmp,
 					 &section->config_items, list) {
 			list_del(&config_item->list);
-			free((char *)config_item->name);
-			free(config_item->value);
-			free(config_item);
+			if (config_item->is_custom) {
+				free((char *)config_item->name);
+				free(config_item->value);
+				free(config_item);
+			}
 		}
 		list_del(&section->list);
-		free((char *)section->name);
-		free(section);
 	}
 
 	free(perf_configs);
diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
index aa4a5a2..33cf9db 100644
--- a/tools/perf/util/config.h
+++ b/tools/perf/util/config.h
@@ -14,6 +14,11 @@ enum perf_config_type {
 	CONFIG_TYPE_STRING
 };
 
+/**
+ * struct perf_config_item - element of perf's configs
+ *
+ * @is_custom: unknown or new config other than default config
+ */
 struct perf_config_item {
 	const char *name;
 	char *value;
@@ -27,6 +32,7 @@ struct perf_config_item {
 		const char *s;
 	} default_value;
 	enum perf_config_type type;
+	bool is_custom;
 	struct list_head list;
 };
 
-- 
2.5.0

Re: [PATCH v3 4/4] perf config: Initialize perf_config_set with all default configs

[Namhyung Kim]

On Thu, Mar 24, 2016 at 05:16:33AM +0900, Taeung Song wrote:
> To avoid duplicated config variables and
> use perf_config_set classifying between standard
> perf config variables and unknown or new config
> variables other than them, initialize perf_config_set
> with all default configs.
> 
> And this will be needed when showing all configs with
> default value or checking correct type of a config
> variable in the near future.
> 
> 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 | 11 +++++++----
>  tools/perf/util/config.c    | 35 +++++++++++++++++++++++++++++------
>  tools/perf/util/config.h    |  6 ++++++
>  3 files changed, 42 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
> index c7cf34f..9e2adad 100644
> --- a/tools/perf/builtin-config.c
> +++ b/tools/perf/builtin-config.c
> @@ -35,23 +35,26 @@ static struct option config_options[] = {
>  
>  static int show_config(struct perf_config_set *perf_configs)
>  {
> +	bool has_value = false;
>  	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)
> +			if (value) {
>  				printf("%s.%s=%s\n", section->name,
>  				       config_item->name, value);
> +				has_value = true;
> +			}
>  		}
>  	}
>  
> +	if (!has_value)
> +		return -1;
> +
>  	return 0;
>  }
>  
> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
> index e3e6ef4..725015f 100644
> --- a/tools/perf/util/config.c
> +++ b/tools/perf/util/config.c
> @@ -746,6 +746,29 @@ out_free:
>  	return ret;
>  }
>  
> +static struct perf_config_set *perf_config_set__init(struct perf_config_set *perf_configs)
> +{
> +	int i, j;
> +	struct perf_config_section *section;
> +	struct perf_config_item *config_items;
> +	struct list_head *sections = &perf_configs->sections;
> +
> +	INIT_LIST_HEAD(&perf_configs->sections);
> +
> +	for (i = 0; i != CONFIG_END; i++) {
> +		section = &default_sections[i];
> +		INIT_LIST_HEAD(&section->config_items);
> +
> +		config_items = default_config_items[i];
> +		for (j = 0; config_items[j].name != NULL; j++)
> +			list_add_tail(&config_items[j].list, &section->config_items);
> +
> +		list_add_tail(&section->list, sections);
> +	}
> +
> +	return perf_configs;
> +}
> +
>  struct perf_config_set *perf_config_set__new(void)
>  {
>  	struct perf_config_set *perf_configs = zalloc(sizeof(*perf_configs));
> @@ -753,7 +776,7 @@ struct perf_config_set *perf_config_set__new(void)
>  	if (!perf_configs)
>  		return NULL;
>  
> -	INIT_LIST_HEAD(&perf_configs->sections);
> +	perf_config_set__init(perf_configs);
>  	perf_config(collect_config, &perf_configs->sections);
>  
>  	return perf_configs;
> @@ -769,13 +792,13 @@ void perf_config_set__delete(struct perf_config_set *perf_configs)
>  		list_for_each_entry_safe(config_item, item_tmp,
>  					 &section->config_items, list) {
>  			list_del(&config_item->list);
> -			free((char *)config_item->name);
> -			free(config_item->value);
> -			free(config_item);
> +			if (config_item->is_custom) {

Where did you set the ->is_custom?


> +				free((char *)config_item->name);
> +				free(config_item->value);
> +				free(config_item);
> +			}
>  		}
>  		list_del(&section->list);
> -		free((char *)section->name);
> -		free(section);

Hmm.. I think the section also has to have something like is_custom
flag since the config file may have some sections/items currently
don't aware of.  That would be needed for forward-compatibility IMHO.

Thanks,
Namhyung


>  	}
>  
>  	free(perf_configs);
> diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
> index aa4a5a2..33cf9db 100644
> --- a/tools/perf/util/config.h
> +++ b/tools/perf/util/config.h
> @@ -14,6 +14,11 @@ enum perf_config_type {
>  	CONFIG_TYPE_STRING
>  };
>  
> +/**
> + * struct perf_config_item - element of perf's configs
> + *
> + * @is_custom: unknown or new config other than default config
> + */
>  struct perf_config_item {
>  	const char *name;
>  	char *value;
> @@ -27,6 +32,7 @@ struct perf_config_item {
>  		const char *s;
>  	} default_value;
>  	enum perf_config_type type;
> +	bool is_custom;
>  	struct list_head list;
>  };
>  
> -- 
> 2.5.0
> 

Re: [PATCH v3 4/4] perf config: Initialize perf_config_set with all default configs

[Taeung Song]

On 03/24/2016 03:13 PM, Namhyung Kim wrote:
> On Thu, Mar 24, 2016 at 05:16:33AM +0900, Taeung Song wrote:
>> To avoid duplicated config variables and
>> use perf_config_set classifying between standard
>> perf config variables and unknown or new config
>> variables other than them, initialize perf_config_set
>> with all default configs.
>>
>> And this will be needed when showing all configs with
>> default value or checking correct type of a config
>> variable in the near future.
>>
>> 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 | 11 +++++++----
>>   tools/perf/util/config.c    | 35 +++++++++++++++++++++++++++++------
>>   tools/perf/util/config.h    |  6 ++++++
>>   3 files changed, 42 insertions(+), 10 deletions(-)
>>
>> diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
>> index c7cf34f..9e2adad 100644
>> --- a/tools/perf/builtin-config.c
>> +++ b/tools/perf/builtin-config.c
>> @@ -35,23 +35,26 @@ static struct option config_options[] = {
>>
>>   static int show_config(struct perf_config_set *perf_configs)
>>   {
>> +	bool has_value = false;
>>   	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)
>> +			if (value) {
>>   				printf("%s.%s=%s\n", section->name,
>>   				       config_item->name, value);
>> +				has_value = true;
>> +			}
>>   		}
>>   	}
>>
>> +	if (!has_value)
>> +		return -1;
>> +
>>   	return 0;
>>   }
>>
>> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
>> index e3e6ef4..725015f 100644
>> --- a/tools/perf/util/config.c
>> +++ b/tools/perf/util/config.c
>> @@ -746,6 +746,29 @@ out_free:
>>   	return ret;
>>   }
>>
>> +static struct perf_config_set *perf_config_set__init(struct perf_config_set *perf_configs)
>> +{
>> +	int i, j;
>> +	struct perf_config_section *section;
>> +	struct perf_config_item *config_items;
>> +	struct list_head *sections = &perf_configs->sections;
>> +
>> +	INIT_LIST_HEAD(&perf_configs->sections);
>> +
>> +	for (i = 0; i != CONFIG_END; i++) {
>> +		section = &default_sections[i];
>> +		INIT_LIST_HEAD(&section->config_items);
>> +
>> +		config_items = default_config_items[i];
>> +		for (j = 0; config_items[j].name != NULL; j++)
>> +			list_add_tail(&config_items[j].list, &section->config_items);
>> +
>> +		list_add_tail(&section->list, sections);
>> +	}
>> +
>> +	return perf_configs;
>> +}
>> +
>>   struct perf_config_set *perf_config_set__new(void)
>>   {
>>   	struct perf_config_set *perf_configs = zalloc(sizeof(*perf_configs));
>> @@ -753,7 +776,7 @@ struct perf_config_set *perf_config_set__new(void)
>>   	if (!perf_configs)
>>   		return NULL;
>>
>> -	INIT_LIST_HEAD(&perf_configs->sections);
>> +	perf_config_set__init(perf_configs);
>>   	perf_config(collect_config, &perf_configs->sections);
>>
>>   	return perf_configs;
>> @@ -769,13 +792,13 @@ void perf_config_set__delete(struct perf_config_set *perf_configs)
>>   		list_for_each_entry_safe(config_item, item_tmp,
>>   					 &section->config_items, list) {
>>   			list_del(&config_item->list);
>> -			free((char *)config_item->name);
>> -			free(config_item->value);
>> -			free(config_item);
>> +			if (config_item->is_custom) {
>
> Where did you set the ->is_custom?

Sorry.. I missed it out
I'll resend this patch that contain below change.

diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index 725015f..3831733 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -678,6 +678,7 @@ static struct perf_config_item 
*add_config_item(struct perf_config_section *sect
          if (!config_item)
                  return NULL;

+        config_item->is_custom = true;
          config_item->name = strdup(name);
          if (!name) {
                  pr_err("%s: strdup failed\n", __func__);


>
>> +				free((char *)config_item->name);
>> +				free(config_item->value);
>> +				free(config_item);
>> +			}
>>   		}
>>   		list_del(&section->list);
>> -		free((char *)section->name);
>> -		free(section);
>
> Hmm.. I think the section also has to have something like is_custom
> flag since the config file may have some sections/items currently
> don't aware of.  That would be needed for forward-compatibility IMHO.
>

I got it.
I'll resend this patch that contains your advice.

Thanks,
Taeung

>
>
>>   	}
>>
>>   	free(perf_configs);
>> diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
>> index aa4a5a2..33cf9db 100644
>> --- a/tools/perf/util/config.h
>> +++ b/tools/perf/util/config.h
>> @@ -14,6 +14,11 @@ enum perf_config_type {
>>   	CONFIG_TYPE_STRING
>>   };
>>
>> +/**
>> + * struct perf_config_item - element of perf's configs
>> + *
>> + * @is_custom: unknown or new config other than default config
>> + */
>>   struct perf_config_item {
>>   	const char *name;
>>   	char *value;
>> @@ -27,6 +32,7 @@ struct perf_config_item {
>>   		const char *s;
>>   	} default_value;
>>   	enum perf_config_type type;
>> +	bool is_custom;
>>   	struct list_head list;
>>   };
>>
>> --
>> 2.5.0
>>