Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755394AbcC1VGg (ORCPT ); Mon, 28 Mar 2016 17:06:36 -0400 Received: from mail.kernel.org ([198.145.29.136]:51601 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752252AbcC1VGe (ORCPT ); Mon, 28 Mar 2016 17:06:34 -0400 Date: Mon, 28 Mar 2016 18:06:29 -0300 From: Arnaldo Carvalho de Melo To: Taeung Song Cc: Namhyung Kim , linux-kernel@vger.kernel.org, Jiri Olsa , Ingo Molnar , Peter Zijlstra Subject: Re: [PATCH v3 1/4] perf config: Introduce perf_config_set class Message-ID: <20160328210629.GH8706@kernel.org> 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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56F38D28.3000504@gmail.com> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8979 Lines: 319 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, - 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 > >> #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 > >>