| 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: <57179C35.7050204@gmail.com>
Date: Thu, 21 Apr 2016 00:11:49 +0900
From: Taeung Song <treeze.taeung@...il.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Arnaldo Carvalho de Melo <arnaldo.melo@...il.com>,
linux-kernel@...r.kernel.org, Jiri Olsa <jolsa@...nel.org>,
Ingo Molnar <mingo@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
Masami Hiramatsu <mhiramat@...nel.org>
Subject: Re: [PATCH v8 3/4] perf config: Prepare all default configs
Hi, Namhyung
On 04/20/2016 09:44 PM, Namhyung Kim wrote:
> Hi Taeung,
>
> On Mon, Apr 18, 2016 at 11:55:18PM +0900, Taeung Song wrote:
>> Hi, Namhyung
>>
>> On 04/15/2016 01:42 AM, Taeung Song wrote:
>>> Hi, Arnaldo
>>>
>>> On 04/14/2016 09:19 PM, Arnaldo Carvalho de Melo wrote:
>>>> Em Thu, Apr 14, 2016 at 04:53:20PM +0900, Taeung Song escreveu:
>>>>> To precisely manage configs,
>>>>> prepare all default perf's configs that contain
>>>>> default section name, variable name, value
>>>>> and correct type, not string type.
>>>>>
>>>>> In the near future, this will be used when
>>>>> checking type of config variable or showing
>>>>> all configs with default values, etc.
>>>>>
>>>>> Acked-by: Namhyung Kim <namhyung@...nel.org>
>>>>> Cc: Masami Hiramatsu <mhiramat@...nel.org>
>>>>> Cc: Jiri Olsa <jolsa@...nel.org>
>>>>> Signed-off-by: Taeung Song <treeze.taeung@...il.com>
>>>>> ---
>>>>> tools/perf/util/config.c | 110
>>>>> ++++++++++++++++++++++++++++++++++++++++++++++-
>>>>> tools/perf/util/config.h | 62 +++++++++++++++++++++++++-
>>>>> 2 files changed, 168 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
>>>>> index dad7d82..5604392 100644
>>>>> --- a/tools/perf/util/config.c
>>>>> +++ b/tools/perf/util/config.c
>>>>> @@ -15,6 +15,7 @@
>>>>> #include "util/llvm-utils.h" /* perf_llvm_config */
>>>>> #include "config.h"
>>>>>
>>>>> +#define MAX_CONFIGS 64
>>>>
>>>> Do we have to add another arbitrary maximum? Where is it used?
>>>>
>>>
>>> IMHO, it is my idea. If you want to avoid using this arbitrary maxinum,
>>> I'd modify the code.
>>>
>>> MAX_CONFIGS is used in order to declare two-dimensional arrays
>>> 'default_config_items'
>>> as below.
>>>
>>> struct perf_config_item default_config_items[][MAX_CONFIGS] = {
>>> [CONFIG_COLORS] = {
>>> CONF_STR_VAR("top", "red, default"),
>>> CONF_STR_VAR("medium", "green, default"),
>>> ...(omitted)...
>>>
>>> IMHO, it is because of perf_config_set__init() in [PATCH v8 4/4].
>>> If we don't use two-dimensional arrays, I think the code of
>>> perf_config_set__init() could be complicated.
>>>
>>
>> As above, I used MAX_CONFIGS because of two-dimensinal arrays
>> 'default_config_items'.
>>
>> What do you think about it ?
>
> I also agree that we'd better to avoid the arbitrary maximum.
>
>>
>> We don't need to add this arbitrary maximum ?
>> or would you mind, if I look for other way about
>> 'default_config_item' ?
>
> What about this?
>
> struct perf_config_item color_config_items[] = {
> CONF_STR_VAR("top", "red, default"),
> CONF_STR_VAR("medium", "green, default"),
> ...
> };
>
> struct perf_config_item tui_config_items[] = {
> CONF_BOOL_VAR("report", true),
> CONF_BOOL_VAR("annotate", true),
> ...
> };
>
> struct perf_config_item *default_config_items[] = {
> &color_config_items,
> &tui_config_items,
> ...
> };
>
> This way we can access the config array by using constant index
> without the hard-coded maximum size IMHO.
>
OK, I got it.
I was hesitating a bit but I'll change this patch by this way you told. :-)
Thanks,
Taeung
>
>
>>>>> + [CONFIG_COLORS] = {
>>>>> + CONF_STR_VAR("top", "red, default"),
>>>>> + CONF_STR_VAR("medium", "green, default"),
>>>>> + CONF_STR_VAR("normal", "lightgray, default"),
>>>>> + CONF_STR_VAR("selected", "white, lightgray"),
>>>>> + CONF_STR_VAR("jump_arrows", "blue, default"),
>>>>> + CONF_STR_VAR("addr", "magenta, default"),
>>>>> + CONF_STR_VAR("root", "white, blue"),
>>>>> + CONF_END()
>>>>> + },
>>>>> + [CONFIG_TUI] = {
>>>>> + CONF_BOOL_VAR("report", true),
>>>>> + CONF_BOOL_VAR("annotate", true),
>>>>> + CONF_BOOL_VAR("top", true),
>>>>> + CONF_END()
>>>>> + },
>
>
>>
>> Thanks,
>> Taeung
>>
>>>>> #define MAXNAME (256)
>>>>>
>>>>> #define DEBUG_CACHE_DIR ".debug"
>>>>> @@ -29,6 +30,111 @@ static int config_file_eof;
>>>>>
>>>>> const char *config_exclusive_filename;
>>>>>
>>>>> +struct perf_config_section default_sections[] = {
>>>>> + { .name = "colors" },
>>>>> + { .name = "tui" },
>>>>> + { .name = "buildid" },
>>>>> + { .name = "annotate" },
>>>>> + { .name = "gtk" },
>>>>> + { .name = "pager" },
>>>>> + { .name = "help" },
>>>>> + { .name = "hist" },
>>>>> + { .name = "ui" },
>>>>> + { .name = "call-graph" },
>>>>> + { .name = "report" },
>>>>> + { .name = "top" },
>>>>> + { .name = "man" },
>>>>> + { .name = "kmem" }
>>>>> +};
>>>>> +
>>>>> +struct perf_config_item default_config_items[][MAX_CONFIGS] = {
>>>>> + [CONFIG_COLORS] = {
>>>>> + CONF_STR_VAR("top", "red, default"),
>>>>> + CONF_STR_VAR("medium", "green, default"),
>>>>> + CONF_STR_VAR("normal", "lightgray, default"),
>>>>> + CONF_STR_VAR("selected", "white, lightgray"),
>>>>> + CONF_STR_VAR("jump_arrows", "blue, default"),
>>>>> + CONF_STR_VAR("addr", "magenta, default"),
>>>>> + CONF_STR_VAR("root", "white, blue"),
>>>>> + CONF_END()
>>>>> + },
>>>>> + [CONFIG_TUI] = {
>>>>> + CONF_BOOL_VAR("report", true),
>>>>> + CONF_BOOL_VAR("annotate", true),
>>>>> + CONF_BOOL_VAR("top", true),
>>>>> + CONF_END()
>>>>> + },
>>>>> + [CONFIG_BUILDID] = {
>>>>> + CONF_STR_VAR("dir", "~/.debug"),
>>>>> + CONF_END()
>>>>> + },
>>>>> + [CONFIG_ANNOTATE] = {
>>>>> + CONF_BOOL_VAR("hide_src_code", false),
>>>>> + CONF_BOOL_VAR("use_offset", true),
>>>>> + CONF_BOOL_VAR("jump_arrows", true),
>>>>> + CONF_BOOL_VAR("show_nr_jumps", false),
>>>>> + CONF_BOOL_VAR("show_linenr", false),
>>>>> + CONF_BOOL_VAR("show_total_period", false),
>>>>> + CONF_END()
>>>>> + },
>>>>> + [CONFIG_GTK] = {
>>>>> + CONF_BOOL_VAR("annotate", false),
>>>>> + CONF_BOOL_VAR("report", false),
>>>>> + CONF_BOOL_VAR("top", false),
>>>>> + CONF_END()
>>>>> + },
>>>>> + [CONFIG_PAGER] = {
>>>>> + CONF_BOOL_VAR("cmd", true),
>>>>> + CONF_BOOL_VAR("report", true),
>>>>> + CONF_BOOL_VAR("annotate", true),
>>>>> + CONF_BOOL_VAR("top", true),
>>>>> + CONF_BOOL_VAR("diff", true),
>>>>> + CONF_END()
>>>>> + },
>>>>> + [CONFIG_HELP] = {
>>>>> + CONF_STR_VAR("format", "man"),
>>>>> + CONF_INT_VAR("autocorrect", 0),
>>>>> + CONF_END()
>>>>> + },
>>>>> + [CONFIG_HIST] = {
>>>>> + CONF_STR_VAR("percentage", "absolute"),
>>>>> + CONF_END()
>>>>> + },
>>>>> + [CONFIG_UI] = {
>>>>> + CONF_BOOL_VAR("show-headers", true),
>>>>> + CONF_END()
>>>>> + },
>>>>> + [CONFIG_CALL_GRAPH] = {
>>>>> + CONF_STR_VAR("record-mode", "fp"),
>>>>> + CONF_LONG_VAR("dump-size", 8192),
>>>>> + CONF_STR_VAR("print-type", "graph"),
>>>>> + CONF_STR_VAR("order", "callee"),
>>>>> + CONF_STR_VAR("sort-key", "function"),
>>>>> + CONF_DOUBLE_VAR("threshold", 0.5),
>>>>> + CONF_LONG_VAR("print-limit", 0),
>>>>> + CONF_END()
>>>>> + },
>>>>> + [CONFIG_REPORT] = {
>>>>> + CONF_BOOL_VAR("group", true),
>>>>> + CONF_BOOL_VAR("children", true),
>>>>> + CONF_FLOAT_VAR("percent-limit", 0),
>>>>> + CONF_U64_VAR("queue-size", 0),
>>>>> + CONF_END()
>>>>> + },
>>>>> + [CONFIG_TOP] = {
>>>>> + CONF_BOOL_VAR("children", true),
>>>>> + CONF_END()
>>>>> + },
>>>>> + [CONFIG_MAN] = {
>>>>> + CONF_STR_VAR("viewer", "man"),
>>>>> + CONF_END()
>>>>> + },
>>>>> + [CONFIG_KMEM] = {
>>>>> + CONF_STR_VAR("default", "slab"),
>>>>> + CONF_END()
>>>>> + }
>>>>> +};
>>>>> +
>>>>> static int get_next_char(void)
>>>>> {
>>>>> int c;
>>>>> @@ -659,7 +765,7 @@ struct perf_config_set *perf_config_set__new(void)
>>>>>
>>>>> static void perf_config_item__delete(struct perf_config_item *item)
>>>>> {
>>>>> - zfree(&item->name);
>>>>> + zfree((char **)&item->name);
>>>>> zfree(&item->value);
>>>>> free(item);
>>>>> }
>>>>> @@ -677,7 +783,7 @@ static void perf_config_section__purge(struct
>>>>> perf_config_section *section)
>>>>> static void perf_config_section__delete(struct perf_config_section
>>>>> *section)
>>>>> {
>>>>> perf_config_section__purge(section);
>>>>> - zfree(§ion->name);
>>>>> + zfree((char **)§ion->name);
>>>>> free(section);
>>>>> }
>>>>>
>>>>> diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
>>>>> index 22ec626..84dcc1d 100644
>>>>> --- a/tools/perf/util/config.h
>>>>> +++ b/tools/perf/util/config.h
>>>>> @@ -4,14 +4,34 @@
>>>>> #include <stdbool.h>
>>>>> #include <linux/list.h>
>>>>>
>>>>> +enum perf_config_type {
>>>>> + CONFIG_TYPE_BOOL,
>>>>> + CONFIG_TYPE_INT,
>>>>> + CONFIG_TYPE_LONG,
>>>>> + CONFIG_TYPE_U64,
>>>>> + CONFIG_TYPE_FLOAT,
>>>>> + CONFIG_TYPE_DOUBLE,
>>>>> + CONFIG_TYPE_STRING
>>>>> +};
>>>>> +
>>>>> struct perf_config_item {
>>>>> - char *name;
>>>>> + const char *name;
>>>>> char *value;
>>>>> + union {
>>>>> + bool b;
>>>>> + int i;
>>>>> + u32 l;
>>>>> + u64 ll;
>>>>> + float f;
>>>>> + double d;
>>>>> + const char *s;
>>>>> + } default_value;
>>>>> + enum perf_config_type type;
>>>>> struct list_head node;
>>>>> };
>>>>>
>>>>> struct perf_config_section {
>>>>> - char *name;
>>>>> + const char *name;
>>>>> struct list_head items;
>>>>> struct list_head node;
>>>>> };
>>>>> @@ -20,6 +40,44 @@ struct perf_config_set {
>>>>> struct list_head sections;
>>>>> };
>>>>>
>>>>> +enum perf_config_secion_idx {
>>>>> + CONFIG_COLORS,
>>>>> + CONFIG_TUI,
>>>>> + CONFIG_BUILDID,
>>>>> + CONFIG_ANNOTATE,
>>>>> + CONFIG_GTK,
>>>>> + CONFIG_PAGER,
>>>>> + CONFIG_HELP,
>>>>> + CONFIG_HIST,
>>>>> + CONFIG_UI,
>>>>> + CONFIG_CALL_GRAPH,
>>>>> + CONFIG_REPORT,
>>>>> + CONFIG_TOP,
>>>>> + CONFIG_MAN,
>>>>> + CONFIG_KMEM,
>>>>> + CONFIG_END
>>>>> +};
>>>>> +
>>>>> +#define CONF_VAR(_name, _field, _val, _type) \
>>>>> + { .name = _name, .default_value._field = _val, .type = _type }
>>>>> +
>>>>> +#define CONF_BOOL_VAR(_name, _val) \
>>>>> + CONF_VAR(_name, b, _val, CONFIG_TYPE_BOOL)
>>>>> +#define CONF_INT_VAR(_name, _val) \
>>>>> + CONF_VAR(_name, i, _val, CONFIG_TYPE_INT)
>>>>> +#define CONF_LONG_VAR(_name, _val) \
>>>>> + CONF_VAR(_name, l, _val, CONFIG_TYPE_LONG)
>>>>> +#define CONF_U64_VAR(_name, _val) \
>>>>> + CONF_VAR(_name, ll, _val, CONFIG_TYPE_U64)
>>>>> +#define CONF_FLOAT_VAR(_name, _val) \
>>>>> + CONF_VAR(_name, f, _val, CONFIG_TYPE_FLOAT)
>>>>> +#define CONF_DOUBLE_VAR(_name, _val) \
>>>>> + CONF_VAR(_name, d, _val, CONFIG_TYPE_DOUBLE)
>>>>> +#define CONF_STR_VAR(_name, _val) \
>>>>> + CONF_VAR(_name, s, _val, CONFIG_TYPE_STRING)
>>>>> +#define CONF_END() \
>>>>> + { .name = NULL }
>>>>> +
>>>>> struct perf_config_set *perf_config_set__new(void);
>>>>> void perf_config_set__delete(struct perf_config_set *set);
>>>>>
>>>>> --
>>>>> 2.5.0
Powered by blists - more mailing lists