linux-kernel - Re: [PATCH v4 1/4] perf config: Introduce perf_config

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <834D1C5E-6C2E-43BA-AB40-B600A09819B3@gmail.com>
Date:	Wed, 30 Mar 2016 11:09:11 +0900
From:	Namhyung Kim <namhyung@...il.com>
To:	Taeung Song <treeze.taeung@...il.com>,
	Namhyung Kim <namhyung@...nel.org>
CC:	Arnaldo Carvalho de Melo <arnaldo.melo@...il.com>,
	linux-kernel@...r.kernel.org, Jiri Olsa <jolsa@...nel.org>,
	Ingo Molnar <mingo@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH v4 1/4] perf config: Introduce perf_config_set class

On March 30, 2016 8:28:21 AM GMT+09:00, Taeung Song <treeze.taeung@...il.com> wrote:
>Hi, Arnaldo and Namhyung
>
>On 03/30/2016 01:12 AM, Arnaldo Carvalho de Melo wrote:
>> Em Tue, Mar 29, 2016 at 09:43:13AM +0900, Taeung Song escreveu:
>>> 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@...nel.org>
>>> Cc: Jiri Olsa <jolsa@...nel.org>
>>> Signed-off-by: Taeung Song <treeze.taeung@...il.com>
>>
>> Waiting for ack.
>
>Namhyung,
>The difference between v3 and v4 for this patch as below.
>(fill perf_config_set__delete() in collect_config() for state of error)
>
>Can you review this patch, again ?

Acked-by: Namhyung Kim <namhyung@...nel.org>

Thanks
Namhyung


>diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
>index 725015f..2cfafff 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 | 171
>+++++++++++++++++++++++++++++++++++++++++++++++
>>>   tools/perf/util/config.h |  26 +++++++
>>>   2 files changed, 197 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..2dbf47c 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,176 @@ 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 *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 perf_config_set *perf_configs = perf_config_set;
>>> +	struct list_head *sections = &perf_configs->sections;
>>> +
>>> +	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);
>>> +	perf_config_set__delete(perf_configs);
>>> +	return -1;
>>> +}
>>> +
>>> +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);
>>> +
>>> +	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


-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.