Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753302AbcCXGta (ORCPT ); Thu, 24 Mar 2016 02:49:30 -0400 Received: from mail-pf0-f193.google.com ([209.85.192.193]:34636 "EHLO mail-pf0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751005AbcCXGtV (ORCPT ); Thu, 24 Mar 2016 02:49:21 -0400 Subject: Re: [PATCH v3 4/4] perf config: Initialize perf_config_set with all default configs To: Namhyung Kim References: <1458764193-15551-1-git-send-email-treeze.taeung@gmail.com> <1458764193-15551-5-git-send-email-treeze.taeung@gmail.com> <20160324061317.GD32072@sejong> Cc: Arnaldo Carvalho de Melo , linux-kernel@vger.kernel.org, Jiri Olsa , Ingo Molnar , Peter Zijlstra From: Taeung Song Message-ID: <56F38DED.1010906@gmail.com> Date: Thu, 24 Mar 2016 15:49:17 +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: <20160324061317.GD32072@sejong> 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: 5409 Lines: 183 On 03/24/2016 03:13 PM, Namhyung Kim wrote: > On Thu, Mar 24, 2016 at 05:16:33AM +0900, Taeung Song wrote: >> To avoid duplicated config variables and >> use perf_config_set classifying between standard >> perf config variables and unknown or new config >> variables other than them, initialize perf_config_set >> with all default configs. >> >> And this will be needed when showing all configs with >> default value or checking correct type of a config >> variable in the near future. >> >> Cc: Namhyung Kim >> Cc: Jiri Olsa >> Signed-off-by: Taeung Song >> --- >> tools/perf/builtin-config.c | 11 +++++++---- >> tools/perf/util/config.c | 35 +++++++++++++++++++++++++++++------ >> tools/perf/util/config.h | 6 ++++++ >> 3 files changed, 42 insertions(+), 10 deletions(-) >> >> diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c >> index c7cf34f..9e2adad 100644 >> --- a/tools/perf/builtin-config.c >> +++ b/tools/perf/builtin-config.c >> @@ -35,23 +35,26 @@ static struct option config_options[] = { >> >> static int show_config(struct perf_config_set *perf_configs) >> { >> + bool has_value = false; >> struct perf_config_section *section; >> struct perf_config_item *config_item; >> struct list_head *sections = &perf_configs->sections; >> >> - if (list_empty(sections)) >> - return -1; >> - >> list_for_each_entry(section, sections, list) { >> list_for_each_entry(config_item, §ion->config_items, list) { >> char *value = config_item->value; >> >> - if (value) >> + if (value) { >> printf("%s.%s=%s\n", section->name, >> config_item->name, value); >> + has_value = true; >> + } >> } >> } >> >> + if (!has_value) >> + return -1; >> + >> return 0; >> } >> >> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c >> index e3e6ef4..725015f 100644 >> --- a/tools/perf/util/config.c >> +++ b/tools/perf/util/config.c >> @@ -746,6 +746,29 @@ out_free: >> return ret; >> } >> >> +static struct perf_config_set *perf_config_set__init(struct perf_config_set *perf_configs) >> +{ >> + int i, j; >> + struct perf_config_section *section; >> + struct perf_config_item *config_items; >> + struct list_head *sections = &perf_configs->sections; >> + >> + INIT_LIST_HEAD(&perf_configs->sections); >> + >> + for (i = 0; i != CONFIG_END; i++) { >> + section = &default_sections[i]; >> + INIT_LIST_HEAD(§ion->config_items); >> + >> + config_items = default_config_items[i]; >> + for (j = 0; config_items[j].name != NULL; j++) >> + list_add_tail(&config_items[j].list, §ion->config_items); >> + >> + list_add_tail(§ion->list, sections); >> + } >> + >> + return perf_configs; >> +} >> + >> struct perf_config_set *perf_config_set__new(void) >> { >> struct perf_config_set *perf_configs = zalloc(sizeof(*perf_configs)); >> @@ -753,7 +776,7 @@ struct perf_config_set *perf_config_set__new(void) >> if (!perf_configs) >> return NULL; >> >> - INIT_LIST_HEAD(&perf_configs->sections); >> + perf_config_set__init(perf_configs); >> perf_config(collect_config, &perf_configs->sections); >> >> return perf_configs; >> @@ -769,13 +792,13 @@ void perf_config_set__delete(struct perf_config_set *perf_configs) >> list_for_each_entry_safe(config_item, item_tmp, >> §ion->config_items, list) { >> list_del(&config_item->list); >> - free((char *)config_item->name); >> - free(config_item->value); >> - free(config_item); >> + if (config_item->is_custom) { > > Where did you set the ->is_custom? Sorry.. I missed it out I'll resend this patch that contain below change. 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 @@ -678,6 +678,7 @@ static struct perf_config_item *add_config_item(struct perf_config_section *sect if (!config_item) return NULL; + config_item->is_custom = true; config_item->name = strdup(name); if (!name) { pr_err("%s: strdup failed\n", __func__); > >> + free((char *)config_item->name); >> + free(config_item->value); >> + free(config_item); >> + } >> } >> list_del(§ion->list); >> - free((char *)section->name); >> - free(section); > > Hmm.. I think the section also has to have something like is_custom > flag since the config file may have some sections/items currently > don't aware of. That would be needed for forward-compatibility IMHO. > I got it. I'll resend this patch that contains your advice. Thanks, Taeung > > >> } >> >> free(perf_configs); >> diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h >> index aa4a5a2..33cf9db 100644 >> --- a/tools/perf/util/config.h >> +++ b/tools/perf/util/config.h >> @@ -14,6 +14,11 @@ enum perf_config_type { >> CONFIG_TYPE_STRING >> }; >> >> +/** >> + * struct perf_config_item - element of perf's configs >> + * >> + * @is_custom: unknown or new config other than default config >> + */ >> struct perf_config_item { >> const char *name; >> char *value; >> @@ -27,6 +32,7 @@ struct perf_config_item { >> const char *s; >> } default_value; >> enum perf_config_type type; >> + bool is_custom; >> struct list_head list; >> }; >> >> -- >> 2.5.0 >>