| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <56F38DED.1010906@gmail.com>
Date: Thu, 24 Mar 2016 15:49:17 +0900
From: Taeung Song <treeze.taeung@...il.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>,
linux-kernel@...r.kernel.org, Jiri Olsa <jolsa@...nel.org>,
Ingo Molnar <mingo@...nel.org>,
Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH v3 4/4] perf config: Initialize perf_config_set with all
default configs
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 <namhyung@...nel.org>
>> Cc: Jiri Olsa <jolsa@...nel.org>
>> Signed-off-by: Taeung Song <treeze.taeung@...il.com>
>> ---
>> 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
>>
Powered by blists - more mailing lists