Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755551AbcC2ASh (ORCPT ); Mon, 28 Mar 2016 20:18:37 -0400 Received: from mail-pa0-f67.google.com ([209.85.220.67]:34454 "EHLO mail-pa0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751585AbcC2ASc (ORCPT ); Mon, 28 Mar 2016 20:18:32 -0400 Subject: Re: [PATCH v3 1/4] perf config: Introduce perf_config_set class To: Arnaldo Carvalho de Melo References: <1458764193-15551-1-git-send-email-treeze.taeung@gmail.com> <1458764193-15551-2-git-send-email-treeze.taeung@gmail.com> <20160324060734.GA32072@sejong> <56F38D28.3000504@gmail.com> <20160328210629.GH8706@kernel.org> Cc: Namhyung Kim , linux-kernel@vger.kernel.org, Jiri Olsa , Ingo Molnar , Peter Zijlstra From: Taeung Song Message-ID: <56F9C9D4.706@gmail.com> Date: Tue, 29 Mar 2016 09:18:28 +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: <20160328210629.GH8706@kernel.org> Content-Type: text/plain; charset=windows-1252; 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: 9470 Lines: 330 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 >>>> Signed-off-by: Taeung Song >>> >>> Acked-by: Namhyung Kim >>> >> >> 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 >>>> #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, §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 *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, §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); >>>> + 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, >>>> + §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 >>>>