Messages in this thread Patches in this message | | | Subject | Re: [PATCH 1/4] perf config: Introduce default_config_item for all default config key-value pairs | From | Taeung Song <> | Date | Tue, 10 May 2016 20:49:16 +0900 |
| |
Hi, Arnaldo, Namhyung and jirka :)
On 05/10/2016 02:17 AM, Arnaldo Carvalho de Melo wrote: > Em Mon, May 09, 2016 at 08:41:46PM +0900, Taeung Song escreveu: >> We currently use values of actual type(int, bool, char *, etc.) >> when initializing default perf config values. >> >> But I suggest using new config arrays(default_config_items) >> that have all default perf config key-value pairs. >> >> Because if we do, we can manage default perf config values at one spot (like util/config.c) >> and it can be easy and simple to modify default config values or add new configs. >> >> In the near future, this could be used when >> showing all configs with default values, >> initializing default values of each actual config variable >> with the default_config_item arrays and etc. > > looks ok
Thank you for your review !! :-)
We can use this patch as it is, but what about this change ?
diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h index 84414af..8e143fb 100644 --- a/tools/perf/util/config.h +++ b/tools/perf/util/config.h @@ -5,6 +5,9 @@ #include <linux/list.h>
enum perf_config_type { + /* special type */ + CONFIG_END, + CONFIG_TYPE_BOOL, CONFIG_TYPE_INT, CONFIG_TYPE_LONG, @@ -14,8 +17,10 @@ enum perf_config_type { CONFIG_TYPE_STRING };
+#define MAX_CONFIG_NAME 100 + struct default_config_item { - const char *name; + const char name[MAX_CONFIG_NAME]; union { bool b; int i; @@ -62,7 +67,7 @@ struct perf_config_set { #define CONF_STR_VAR(_name, _val) \ CONF_VAR(_name, s, _val, CONFIG_TYPE_STRING) #define CONF_END() \ - { .name = NULL } + { .type = CONFIG_END }
Because if we do, we can use name of default_config_item as a constant to replace values of actual type(int, char *, etc) with colors_config_items[].name more easy e.g. (Contrastively if we use 'name' member variable as 'const char *' type, we can't use it as a constant like below. It is a limitation of C language.)
diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c index 385d0de..207c62d 100644 --- a/tools/perf/ui/browser.c +++ b/tools/perf/ui/browser.c @@ -508,31 +508,31 @@ static struct ui_browser_colorset { } ui_browser__colorsets[] = { { .colorset = HE_COLORSET_TOP, - .name = "top", + .name = colors_config_items[CONFIG_COLORS_TOP].name, }, { .colorset = HE_COLORSET_MEDIUM, - .name = "medium", + .name = colors_config_items[CONFIG_COLORS_MEDIUM].name, }, { .colorset = HE_COLORSET_NORMAL, - .name = "normal", + .name = colors_config_items[CONFIG_COLORS_NORMAL].name, }, { .colorset = HE_COLORSET_SELECTED, - .name = "selected", + .name = colors_config_items[CONFIG_COLORS_SELECTED].name, }, { .colorset = HE_COLORSET_JUMP_ARROWS, - .name = "jump_arrows", + .name = colors_config_items[CONFIG_COLORS_JUMP_ARROWS].name, }, { .colorset = HE_COLORSET_ADDR, - .name = "addr", + .name = colors_config_items[CONFIG_COLORS_ADDR].name, }, { .colorset = HE_COLORSET_ROOT, - .name = "root", + .name = colors_config_items[CONFIG_COLORS_ROOT].name, }, { .name = NULL,
If you give me your opinion or other way, I'd appreciate it. :)
Thanks, Taeung
> >> Cc: Namhyung Kim <namhyung@kernel.org> >> Cc: Masami Hiramatsu <mhiramat@kernel.org> >> Cc: Jiri Olsa <jolsa@kernel.org> >> Signed-off-by: Taeung Song <treeze.taeung@gmail.com> >> --- >> tools/perf/util/config.c | 150 ++++++++++++++++++++++++++++++++++++++++++++++- >> tools/perf/util/config.h | 46 ++++++++++++++- >> 2 files changed, 194 insertions(+), 2 deletions(-) >> >> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c >> index dad7d82..3a72ed7 100644 >> --- a/tools/perf/util/config.c >> +++ b/tools/perf/util/config.c >> @@ -29,6 +29,154 @@ static int config_file_eof; >> >> const char *config_exclusive_filename; >> >> +const 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" }, >> + { .name = "intel-pt" }, >> + { .name = "convert" }, >> +}; >> + >> +const struct default_config_item colors_config_items[] = { >> + CONF_STR_VAR("top", "red, default"), >> + CONF_STR_VAR("medium", "green, default"), >> + CONF_STR_VAR("normal", "default, default"), >> + CONF_STR_VAR("selected", "black, yellow"), >> + CONF_STR_VAR("jump_arrows", "blue, default"), >> + CONF_STR_VAR("addr", "magenta, default"), >> + CONF_STR_VAR("root", "white, blue"), >> + CONF_END() >> +}; >> + >> +const struct default_config_item tui_config_items[] = { >> + CONF_BOOL_VAR("report", true), >> + CONF_BOOL_VAR("annotate", true), >> + CONF_BOOL_VAR("top", true), >> + CONF_END() >> +}; >> + >> +const struct default_config_item buildid_config_items[] = { >> + CONF_STR_VAR("dir", "~/.debug"), >> + CONF_END() >> +}; >> + >> +const struct default_config_item annotate_config_items[] = { >> + 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() >> +}; >> + >> +const struct default_config_item gtk_config_items[] = { >> + CONF_BOOL_VAR("annotate", false), >> + CONF_BOOL_VAR("report", false), >> + CONF_BOOL_VAR("top", false), >> + CONF_END() >> +}; >> + >> +const struct default_config_item pager_config_items[] = { >> + 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() >> +}; >> + >> +const struct default_config_item help_config_items[] = { >> + CONF_STR_VAR("format", "man"), >> + CONF_INT_VAR("autocorrect", 0), >> + CONF_END() >> +}; >> + >> +const struct default_config_item hist_config_items[] = { >> + CONF_STR_VAR("percentage", "absolute"), >> + CONF_END() >> +}; >> + >> +const struct default_config_item ui_config_items[] = { >> + CONF_BOOL_VAR("show-headers", true), >> + CONF_END() >> +}; >> + >> +const struct default_config_item call_graph_config_items[] = { >> + 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() >> +}; >> + >> +const struct default_config_item report_config_items[] = { >> + CONF_BOOL_VAR("group", true), >> + CONF_BOOL_VAR("children", true), >> + CONF_FLOAT_VAR("percent-limit", 0), >> + CONF_U64_VAR("queue-size", 0), >> + CONF_END() >> +}; >> + >> +const struct default_config_item top_config_items[] = { >> + CONF_BOOL_VAR("children", true), >> + CONF_END() >> +}; >> + >> +const struct default_config_item man_config_items[] = { >> + CONF_STR_VAR("viewer", "man"), >> + CONF_END() >> +}; >> + >> +const struct default_config_item kmem_config_items[] = { >> + CONF_STR_VAR("default", "slab"), >> + CONF_END() >> +}; >> + >> +const struct default_config_item intel_pt_config_items[] = { >> + CONF_INT_VAR("cache-divisor", 64), >> + CONF_BOOL_VAR("mispred-all", false), >> + CONF_END() >> +}; >> + >> +const struct default_config_item convert_config_items[] = { >> + CONF_U64_VAR("queue-size", 0), >> + CONF_END() >> +}; >> + >> +const struct default_config_item *default_config_items[] = { >> + colors_config_items, >> + tui_config_items, >> + buildid_config_items, >> + annotate_config_items, >> + gtk_config_items, >> + pager_config_items, >> + help_config_items, >> + hist_config_items, >> + ui_config_items, >> + call_graph_config_items, >> + report_config_items, >> + top_config_items, >> + man_config_items, >> + kmem_config_items, >> + intel_pt_config_items, >> + convert_config_items, >> +}; >> + >> static int get_next_char(void) >> { >> int c; >> @@ -677,7 +825,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..84414af 100644 >> --- a/tools/perf/util/config.h >> +++ b/tools/perf/util/config.h >> @@ -4,6 +4,30 @@ >> #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 default_config_item { >> + const char *name; >> + union { >> + bool b; >> + int i; >> + u32 l; >> + u64 ll; >> + float f; >> + double d; >> + const char *s; >> + } value; >> + enum perf_config_type type; >> +}; >> + >> struct perf_config_item { >> char *name; >> char *value; >> @@ -11,7 +35,7 @@ struct perf_config_item { >> }; >> >> struct perf_config_section { >> - char *name; >> + const char *name; >> struct list_head items; >> struct list_head node; >> }; >> @@ -20,6 +44,26 @@ struct perf_config_set { >> struct list_head sections; >> }; >> >> +#define CONF_VAR(_name, _field, _val, _type) \ >> + { .name = _name, .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
| |