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

From: Namhyung Kim
Date: Tue Mar 29 2016 - 22:09:24 EST

On March 30, 2016 8:28:21 AM GMT+09:00, Taeung Song <treeze.taeung@xxxxxxxxx> 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@xxxxxxxxxx>
>>> Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
>>> Signed-off-by: Taeung Song <treeze.taeung@xxxxxxxxx>
>>
>> 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@xxxxxxxxxx>

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.