Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751756AbcC3LEk (ORCPT ); Wed, 30 Mar 2016 07:04:40 -0400 Received: from mail-pf0-f194.google.com ([209.85.192.194]:34265 "EHLO mail-pf0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750954AbcC3LEi (ORCPT ); Wed, 30 Mar 2016 07:04:38 -0400 Subject: Re: [PATCH v4 1/4] perf config: Introduce perf_config_set class To: Namhyung Kim References: <1459212193-1095-1-git-send-email-treeze.taeung@gmail.com> <20160329161222.GA23816@kernel.org> <56FB0F95.3090503@gmail.com> <834D1C5E-6C2E-43BA-AB40-B600A09819B3@gmail.com> Cc: Arnaldo Carvalho de Melo , linux-kernel@vger.kernel.org, Jiri Olsa , Ingo Molnar , Peter Zijlstra From: Taeung Song Message-ID: <56FBB2C1.706@gmail.com> Date: Wed, 30 Mar 2016 20:04:33 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <834D1C5E-6C2E-43BA-AB40-B600A09819B3@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 9265 Lines: 325 Hi, Namhyung On 03/30/2016 11:09 AM, Namhyung Kim wrote: > On March 30, 2016 8:28:21 AM GMT+09:00, Taeung Song 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 >>>> Cc: Jiri Olsa >>>> Signed-off-by: Taeung Song >>> >>> 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 > Thank you !! :-) Taeung > > >> 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 >>>> #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, §ion->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(§ion->config_items); >>>> + section->name = strdup(section_name); >>>> + if (!section->name) { >>>> + pr_err("%s: strdup failed\n", __func__); >>>> + free(section); >>>> + return NULL; >>>> + } >>>> + >>>> + list_add_tail(§ion->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, §ion->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, §ion, &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, >>>> + §ion->config_items, list) { >>>> + list_del(&config_item->list); >>>> + free(config_item->name); >>>> + free(config_item->value); >>>> + free(config_item); >>>> + } >>>> + list_del(§ion->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 >>>> +#include >>>> + >>>> +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 > >