All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] perf config: Introduce default config key-value pairs arrays
@ 2016-05-09 11:41 Taeung Song
  2016-05-09 11:41 ` [PATCH 1/4] perf config: Introduce default_config_item for all default config key-value pairs Taeung Song
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Taeung Song @ 2016-05-09 11:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar,
	Peter Zijlstra, Alexander Shishkin, Taeung Song

We currently use values of actual type(int, bool, char *, etc.)
when initializing default perf config values.

For example,
If there isn't user config value at ~/.perfconfig for 'annotate.use_offset' config variable,
default value for it is 'true' bool type value in perf like below.

At ui/browsers/annoate.c

static struct annotate_browser_opt {
       bool hide_src_code,
            use_offset,
	    jump_arrows,
	    show_linenr,
	    show_nr_jumps,
	    show_total_period;
} annotate_browser__opts = {
       .use_offset      = true,
       .jump_arrows     = true,
};

But I suggest using new config arrays that have all default config key-value pairs
and then initializing default config values with them.
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.

For example,
If we use new default config arrays and there isn't user config value for 'annoate.use_offset'
default value for it will be set as annotate_config_items[CONFIG_ANNOATE_USE_OFFSET].value
instead of actual boolean type value 'true'.

IMHO, I think it should be needed to use new default config arrays
to manage default perf config values more effectively.
And this pathset contains patchs for only 'colors' and 'annoate' section
because waiting for other opinions.

If you review this patchset, I'd appreciate it :-)

Thanks,
Taeung

Taeung Song (4):
  perf config: Introduce default_config_item for all default config
    key-value pairs
  perf tools: Separate out code setting ground colors from
    ui_browser__color_config
  perf config: Initialize ui_browser__colorsets with default config
    items
  perf config: Initialize annotate_browser__opts with default config
    items

 tools/perf/ui/browser.c           |  89 ++++++++++++++--------
 tools/perf/ui/browser.h           |   1 +
 tools/perf/ui/browsers/annotate.c |  12 ++-
 tools/perf/ui/tui/setup.c         |   1 +
 tools/perf/util/cache.h           |   1 +
 tools/perf/util/config.c          | 150 +++++++++++++++++++++++++++++++++++++-
 tools/perf/util/config.h          |  74 ++++++++++++++++++-
 7 files changed, 291 insertions(+), 37 deletions(-)

-- 
2.5.0

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/4] perf config: Introduce default_config_item for all default config key-value pairs
  2016-05-09 11:41 [PATCH 0/4] perf config: Introduce default config key-value pairs arrays Taeung Song
@ 2016-05-09 11:41 ` Taeung Song
  2016-05-09 17:17   ` Arnaldo Carvalho de Melo
  2016-05-09 11:41 ` [PATCH 2/4] perf tools: Separate out code setting ground colors from ui_browser__color_config Taeung Song
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Taeung Song @ 2016-05-09 11:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar,
	Peter Zijlstra, Alexander Shishkin, Taeung Song,
	Masami Hiramatsu

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.

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(&section->name);
+	zfree((char **)&section->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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/4] perf tools: Separate out code setting ground colors from ui_browser__color_config
  2016-05-09 11:41 [PATCH 0/4] perf config: Introduce default config key-value pairs arrays Taeung Song
  2016-05-09 11:41 ` [PATCH 1/4] perf config: Introduce default_config_item for all default config key-value pairs Taeung Song
@ 2016-05-09 11:41 ` Taeung Song
  2016-05-09 17:17   ` Arnaldo Carvalho de Melo
  2016-05-09 11:41 ` [PATCH 3/4] perf config: Initialize ui_browser__colorsets with default config items Taeung Song
  2016-05-09 11:41 ` [PATCH 4/4] perf config: Initialize annotate_browser__opts " Taeung Song
  3 siblings, 1 reply; 13+ messages in thread
From: Taeung Song @ 2016-05-09 11:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar,
	Peter Zijlstra, Alexander Shishkin, Taeung Song

ui_browser__color_config() set foreground and background
colors values in ui_browser__colorsets.
But it can be reused by other functions so make ui_browser__config_gcolors()
bringing it from ui_browser__color_config().

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/ui/browser.c | 43 ++++++++++++++++++++++++++-----------------
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c
index af68a9d..a477867 100644
--- a/tools/perf/ui/browser.c
+++ b/tools/perf/ui/browser.c
@@ -553,12 +553,33 @@ static struct ui_browser_colorset {
 	}
 };
 
+static int ui_browser__config_gcolors(struct ui_browser_colorset *ui_browser_color,
+				      const char *value)
+{
+	char *fg = NULL, *bg;
+
+	fg = strdup(value);
+	if (fg == NULL)
+		return -1;
+
+	bg = strchr(fg, ',');
+	if (bg == NULL) {
+		free(fg);
+		return -1;
+	}
+
+	*bg = '\0';
+	while (isspace(*++bg));
+
+	ui_browser_color->fg = fg;
+	ui_browser_color->bg = bg;
+	return 0;
+}
 
 static int ui_browser__color_config(const char *var, const char *value,
 				    void *data __maybe_unused)
 {
-	char *fg = NULL, *bg;
-	int i;
+	int i, ret;
 
 	/* same dir for all commands */
 	if (prefixcmp(var, "colors.") != 0)
@@ -570,23 +591,11 @@ static int ui_browser__color_config(const char *var, const char *value,
 		if (strcmp(ui_browser__colorsets[i].name, name) != 0)
 			continue;
 
-		fg = strdup(value);
-		if (fg == NULL)
-			break;
-
-		bg = strchr(fg, ',');
-		if (bg == NULL)
-			break;
-
-		*bg = '\0';
-		while (isspace(*++bg));
-		ui_browser__colorsets[i].bg = bg;
-		ui_browser__colorsets[i].fg = fg;
-		return 0;
+		ret = ui_browser__config_gcolors(&ui_browser__colorsets[i], value);
+		break;
 	}
 
-	free(fg);
-	return -1;
+	return ret;
 }
 
 void ui_browser__argv_seek(struct ui_browser *browser, off_t offset, int whence)
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/4] perf config: Initialize ui_browser__colorsets with default config items
  2016-05-09 11:41 [PATCH 0/4] perf config: Introduce default config key-value pairs arrays Taeung Song
  2016-05-09 11:41 ` [PATCH 1/4] perf config: Introduce default_config_item for all default config key-value pairs Taeung Song
  2016-05-09 11:41 ` [PATCH 2/4] perf tools: Separate out code setting ground colors from ui_browser__color_config Taeung Song
@ 2016-05-09 11:41 ` Taeung Song
  2016-05-09 17:19   ` Arnaldo Carvalho de Melo
  2016-05-09 11:41 ` [PATCH 4/4] perf config: Initialize annotate_browser__opts " Taeung Song
  3 siblings, 1 reply; 13+ messages in thread
From: Taeung Song @ 2016-05-09 11:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar,
	Peter Zijlstra, Alexander Shishkin, Taeung Song

Set default config values for 'colors' section with 'colors_config_items[]'
instead of actual const char * type values.
(e.g. using colors_config_item[CONFIG_COLORS_TOP].value
instead of "red, default" string value for 'colors.top')

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/ui/browser.c   | 46 ++++++++++++++++++++++++++++++++--------------
 tools/perf/ui/browser.h   |  1 +
 tools/perf/ui/tui/setup.c |  1 +
 tools/perf/util/cache.h   |  1 +
 tools/perf/util/config.h  | 12 ++++++++++++
 5 files changed, 47 insertions(+), 14 deletions(-)

diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c
index a477867..385d0de 100644
--- a/tools/perf/ui/browser.c
+++ b/tools/perf/ui/browser.c
@@ -509,44 +509,30 @@ static struct ui_browser_colorset {
 	{
 		.colorset = HE_COLORSET_TOP,
 		.name	  = "top",
-		.fg	  = "red",
-		.bg	  = "default",
 	},
 	{
 		.colorset = HE_COLORSET_MEDIUM,
 		.name	  = "medium",
-		.fg	  = "green",
-		.bg	  = "default",
 	},
 	{
 		.colorset = HE_COLORSET_NORMAL,
 		.name	  = "normal",
-		.fg	  = "default",
-		.bg	  = "default",
 	},
 	{
 		.colorset = HE_COLORSET_SELECTED,
 		.name	  = "selected",
-		.fg	  = "black",
-		.bg	  = "yellow",
 	},
 	{
 		.colorset = HE_COLORSET_JUMP_ARROWS,
 		.name	  = "jump_arrows",
-		.fg	  = "blue",
-		.bg	  = "default",
 	},
 	{
 		.colorset = HE_COLORSET_ADDR,
 		.name	  = "addr",
-		.fg	  = "magenta",
-		.bg	  = "default",
 	},
 	{
 		.colorset = HE_COLORSET_ROOT,
 		.name	  = "root",
-		.fg	  = "white",
-		.bg	  = "blue",
 	},
 	{
 		.name = NULL,
@@ -591,6 +577,7 @@ static int ui_browser__color_config(const char *var, const char *value,
 		if (strcmp(ui_browser__colorsets[i].name, name) != 0)
 			continue;
 
+		zfree((char **)&ui_browser__colorsets[i].fg);
 		ret = ui_browser__config_gcolors(&ui_browser__colorsets[i], value);
 		break;
 	}
@@ -745,10 +732,41 @@ void __ui_browser__line_arrow(struct ui_browser *browser, unsigned int column,
 		__ui_browser__line_arrow_down(browser, column, start, end);
 }
 
+static void ui_browser__free_color_configs(void)
+{
+	int i;
+
+	for (i = 0; ui_browser__colorsets[i].name != NULL; ++i)
+		zfree((char **)&ui_browser__colorsets[i].fg);
+}
+
+void ui_browser__free(void)
+{
+	ui_browser__free_color_configs();
+}
+
+static void ui_browser__init_colorsets(void)
+{
+	int i, j;
+
+	for (i = 0; ui_browser__colorsets[i].name != NULL; ++i) {
+		const char *name = ui_browser__colorsets[i].name;
+
+		for (j = 0; colors_config_items[j].name != NULL; j++) {
+			if (!strcmp(name, colors_config_items[j].name)) {
+				ui_browser__config_gcolors(&ui_browser__colorsets[i],
+							   colors_config_items[j].value.s);
+				break;
+			}
+		}
+	}
+}
+
 void ui_browser__init(void)
 {
 	int i = 0;
 
+	ui_browser__init_colorsets();
 	perf_config(ui_browser__color_config, NULL);
 
 	while (ui_browser__colorsets[i].name) {
diff --git a/tools/perf/ui/browser.h b/tools/perf/ui/browser.h
index be3b70e..7a83a1a 100644
--- a/tools/perf/ui/browser.h
+++ b/tools/perf/ui/browser.h
@@ -74,5 +74,6 @@ void ui_browser__list_head_seek(struct ui_browser *browser, off_t offset, int wh
 unsigned int ui_browser__list_head_refresh(struct ui_browser *browser);
 
 void ui_browser__init(void);
+void ui_browser__free(void);
 void annotate_browser__init(void);
 #endif /* _PERF_UI_BROWSER_H_ */
diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
index 7dfeba0..7999a57 100644
--- a/tools/perf/ui/tui/setup.c
+++ b/tools/perf/ui/tui/setup.c
@@ -171,4 +171,5 @@ void ui__exit(bool wait_for_ok)
 	SLang_reset_tty();
 
 	perf_error__unregister(&perf_tui_eops);
+	ui_browser__free();
 }
diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h
index 1f5a93c..8eab653 100644
--- a/tools/perf/util/cache.h
+++ b/tools/perf/util/cache.h
@@ -7,6 +7,7 @@
 #include <subcmd/pager.h>
 #include "../perf.h"
 #include "../ui/ui.h"
+#include "config.h"
 
 #include <linux/string.h>
 
diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
index 84414af..e0c8392 100644
--- a/tools/perf/util/config.h
+++ b/tools/perf/util/config.h
@@ -44,6 +44,16 @@ struct perf_config_set {
 	struct list_head sections;
 };
 
+enum colors_config_items_idx {
+	CONFIG_COLORS_TOP,
+	CONFIG_COLORS_MEDIUM,
+	CONFIG_COLORS_NORMAL,
+	CONFIG_COLORS_SELECTED,
+	CONFIG_COLORS_JUMP_ARROWS,
+	CONFIG_COLORS_ADDR,
+	CONFIG_COLORS_ROOT,
+};
+
 #define CONF_VAR(_name, _field, _val, _type)			\
 	{ .name = _name, .value._field = _val, .type = _type }
 
@@ -64,6 +74,8 @@ struct perf_config_set {
 #define CONF_END()					\
 	{ .name = NULL }
 
+extern const struct default_config_item colors_config_items[];
+
 struct perf_config_set *perf_config_set__new(void);
 void perf_config_set__delete(struct perf_config_set *set);
 
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/4] perf config: Initialize annotate_browser__opts with default config items
  2016-05-09 11:41 [PATCH 0/4] perf config: Introduce default config key-value pairs arrays Taeung Song
                   ` (2 preceding siblings ...)
  2016-05-09 11:41 ` [PATCH 3/4] perf config: Initialize ui_browser__colorsets with default config items Taeung Song
@ 2016-05-09 11:41 ` Taeung Song
  3 siblings, 0 replies; 13+ messages in thread
From: Taeung Song @ 2016-05-09 11:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar,
	Peter Zijlstra, Alexander Shishkin, Taeung Song

Set default config values for 'annotate' section with 'annotate_config_items[]'
instead of actual bool type values.
(e.g. using annotate_config_items[CONFIG_ANNOTATE_USE_OFFSET].value
instead of 'true' bool type value for 'annotate.use_offset'.)

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/ui/browsers/annotate.c | 12 ++++++++----
 tools/perf/util/config.h          | 16 ++++++++++++++++
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 4fc208e..f52e1ea 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -37,10 +37,7 @@ static struct annotate_browser_opt {
 	     show_linenr,
 	     show_nr_jumps,
 	     show_total_period;
-} annotate_browser__opts = {
-	.use_offset	= true,
-	.jump_arrows	= true,
-};
+} annotate_browser__opts;
 
 struct annotate_browser {
 	struct ui_browser b;
@@ -1160,5 +1157,12 @@ static int annotate__config(const char *var, const char *value,
 
 void annotate_browser__init(void)
 {
+	annotate_browser__opts.hide_src_code = CONF_ANNOTATE_DEFAULT_VAL(HIDE_SRC_CODE, b);
+	annotate_browser__opts.use_offset = CONF_ANNOTATE_DEFAULT_VAL(USE_OFFSET, b);
+	annotate_browser__opts.jump_arrows = CONF_ANNOTATE_DEFAULT_VAL(JUMP_ARROWS, b);
+	annotate_browser__opts.show_linenr = CONF_ANNOTATE_DEFAULT_VAL(SHOW_LINENR, b);
+	annotate_browser__opts.show_nr_jumps = CONF_ANNOTATE_DEFAULT_VAL(SHOW_NR_JUMPS, b);
+	annotate_browser__opts.show_total_period = CONF_ANNOTATE_DEFAULT_VAL(SHOW_TOTAL_PERIOD, b);
+
 	perf_config(annotate__config, NULL);
 }
diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
index e0c8392..344b344 100644
--- a/tools/perf/util/config.h
+++ b/tools/perf/util/config.h
@@ -54,6 +54,15 @@ enum colors_config_items_idx {
 	CONFIG_COLORS_ROOT,
 };
 
+enum annotate_config_items_idx {
+	CONFIG_ANNOTATE_HIDE_SRC_CODE,
+	CONFIG_ANNOTATE_USE_OFFSET,
+	CONFIG_ANNOTATE_JUMP_ARROWS,
+	CONFIG_ANNOTATE_SHOW_NR_JUMPS,
+	CONFIG_ANNOTATE_SHOW_LINENR,
+	CONFIG_ANNOTATE_SHOW_TOTAL_PERIOD,
+};
+
 #define CONF_VAR(_name, _field, _val, _type)			\
 	{ .name = _name, .value._field = _val, .type = _type }
 
@@ -74,7 +83,14 @@ enum colors_config_items_idx {
 #define CONF_END()					\
 	{ .name = NULL }
 
+#define CONF_DEFAULT_VAL(section, name, field)			\
+	section##_config_items[CONFIG_##name].value.field
+
+#define CONF_ANNOTATE_DEFAULT_VAL(name, field)			\
+	CONF_DEFAULT_VAL(annotate, ANNOTATE_##name, field)
+
 extern const struct default_config_item colors_config_items[];
+extern const struct default_config_item annotate_config_items[];
 
 struct perf_config_set *perf_config_set__new(void);
 void perf_config_set__delete(struct perf_config_set *set);
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] perf tools: Separate out code setting ground colors from ui_browser__color_config
  2016-05-09 11:41 ` [PATCH 2/4] perf tools: Separate out code setting ground colors from ui_browser__color_config Taeung Song
@ 2016-05-09 17:17   ` Arnaldo Carvalho de Melo
  2016-05-10 11:33     ` Taeung Song
  0 siblings, 1 reply; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-05-09 17:17 UTC (permalink / raw)
  To: Taeung Song
  Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar,
	Peter Zijlstra, Alexander Shishkin

Em Mon, May 09, 2016 at 08:41:47PM +0900, Taeung Song escreveu:
> ui_browser__color_config() set foreground and background
> colors values in ui_browser__colorsets.

"ground colors" sounds strange, I guess referreing to them as "*colors"
or "{back,fore}ground colors" is more appropriate, replace "gcolors"
with "colors" too, please.

> But it can be reused by other functions so make ui_browser__config_gcolors()
> bringing it from ui_browser__color_config().
> 
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
> ---
>  tools/perf/ui/browser.c | 43 ++++++++++++++++++++++++++-----------------
>  1 file changed, 26 insertions(+), 17 deletions(-)
> 
> diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c
> index af68a9d..a477867 100644
> --- a/tools/perf/ui/browser.c
> +++ b/tools/perf/ui/browser.c
> @@ -553,12 +553,33 @@ static struct ui_browser_colorset {
>  	}
>  };
>  
> +static int ui_browser__config_gcolors(struct ui_browser_colorset *ui_browser_color,
> +				      const char *value)
> +{
> +	char *fg = NULL, *bg;
> +
> +	fg = strdup(value);
> +	if (fg == NULL)
> +		return -1;
> +
> +	bg = strchr(fg, ',');
> +	if (bg == NULL) {
> +		free(fg);
> +		return -1;
> +	}
> +
> +	*bg = '\0';

Isn't the above strtok()?

> +	while (isspace(*++bg));

Isn't this ltrim()?

> +
> +	ui_browser_color->fg = fg;
> +	ui_browser_color->bg = bg;
> +	return 0;
> +}
>  
>  static int ui_browser__color_config(const char *var, const char *value,
>  				    void *data __maybe_unused)
>  {
> -	char *fg = NULL, *bg;
> -	int i;
> +	int i, ret;
>  
>  	/* same dir for all commands */
>  	if (prefixcmp(var, "colors.") != 0)
> @@ -570,23 +591,11 @@ static int ui_browser__color_config(const char *var, const char *value,
>  		if (strcmp(ui_browser__colorsets[i].name, name) != 0)
>  			continue;
>  
> -		fg = strdup(value);
> -		if (fg == NULL)
> -			break;
> -
> -		bg = strchr(fg, ',');
> -		if (bg == NULL)
> -			break;
> -
> -		*bg = '\0';
> -		while (isspace(*++bg));

strtok + ltrim

> -		ui_browser__colorsets[i].bg = bg;
> -		ui_browser__colorsets[i].fg = fg;
> -		return 0;
> +		ret = ui_browser__config_gcolors(&ui_browser__colorsets[i], value);
> +		break;
>  	}
>  
> -	free(fg);
> -	return -1;
> +	return ret;
>  }
>  
>  void ui_browser__argv_seek(struct ui_browser *browser, off_t offset, int whence)
> -- 
> 2.5.0

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] perf config: Introduce default_config_item for all default config key-value pairs
  2016-05-09 11:41 ` [PATCH 1/4] perf config: Introduce default_config_item for all default config key-value pairs Taeung Song
@ 2016-05-09 17:17   ` Arnaldo Carvalho de Melo
  2016-05-10 11:49     ` Taeung Song
  0 siblings, 1 reply; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-05-09 17:17 UTC (permalink / raw)
  To: Taeung Song
  Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar,
	Peter Zijlstra, Alexander Shishkin, Masami Hiramatsu

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
 
> 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(&section->name);
> +	zfree((char **)&section->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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/4] perf config: Initialize ui_browser__colorsets with default config items
  2016-05-09 11:41 ` [PATCH 3/4] perf config: Initialize ui_browser__colorsets with default config items Taeung Song
@ 2016-05-09 17:19   ` Arnaldo Carvalho de Melo
  2016-05-10 11:57     ` Taeung Song
  0 siblings, 1 reply; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-05-09 17:19 UTC (permalink / raw)
  To: Taeung Song
  Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar,
	Peter Zijlstra, Alexander Shishkin

Em Mon, May 09, 2016 at 08:41:48PM +0900, Taeung Song escreveu:
> Set default config values for 'colors' section with 'colors_config_items[]'
> instead of actual const char * type values.
> (e.g. using colors_config_item[CONFIG_COLORS_TOP].value
> instead of "red, default" string value for 'colors.top')

looks ok
 
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
> ---
>  tools/perf/ui/browser.c   | 46 ++++++++++++++++++++++++++++++++--------------
>  tools/perf/ui/browser.h   |  1 +
>  tools/perf/ui/tui/setup.c |  1 +
>  tools/perf/util/cache.h   |  1 +
>  tools/perf/util/config.h  | 12 ++++++++++++
>  5 files changed, 47 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c
> index a477867..385d0de 100644
> --- a/tools/perf/ui/browser.c
> +++ b/tools/perf/ui/browser.c
> @@ -509,44 +509,30 @@ static struct ui_browser_colorset {
>  	{
>  		.colorset = HE_COLORSET_TOP,
>  		.name	  = "top",
> -		.fg	  = "red",
> -		.bg	  = "default",
>  	},
>  	{
>  		.colorset = HE_COLORSET_MEDIUM,
>  		.name	  = "medium",
> -		.fg	  = "green",
> -		.bg	  = "default",
>  	},
>  	{
>  		.colorset = HE_COLORSET_NORMAL,
>  		.name	  = "normal",
> -		.fg	  = "default",
> -		.bg	  = "default",
>  	},
>  	{
>  		.colorset = HE_COLORSET_SELECTED,
>  		.name	  = "selected",
> -		.fg	  = "black",
> -		.bg	  = "yellow",
>  	},
>  	{
>  		.colorset = HE_COLORSET_JUMP_ARROWS,
>  		.name	  = "jump_arrows",
> -		.fg	  = "blue",
> -		.bg	  = "default",
>  	},
>  	{
>  		.colorset = HE_COLORSET_ADDR,
>  		.name	  = "addr",
> -		.fg	  = "magenta",
> -		.bg	  = "default",
>  	},
>  	{
>  		.colorset = HE_COLORSET_ROOT,
>  		.name	  = "root",
> -		.fg	  = "white",
> -		.bg	  = "blue",
>  	},
>  	{
>  		.name = NULL,
> @@ -591,6 +577,7 @@ static int ui_browser__color_config(const char *var, const char *value,
>  		if (strcmp(ui_browser__colorsets[i].name, name) != 0)
>  			continue;
>  
> +		zfree((char **)&ui_browser__colorsets[i].fg);
>  		ret = ui_browser__config_gcolors(&ui_browser__colorsets[i], value);
>  		break;
>  	}
> @@ -745,10 +732,41 @@ void __ui_browser__line_arrow(struct ui_browser *browser, unsigned int column,
>  		__ui_browser__line_arrow_down(browser, column, start, end);
>  }
>  
> +static void ui_browser__free_color_configs(void)
> +{
> +	int i;
> +
> +	for (i = 0; ui_browser__colorsets[i].name != NULL; ++i)
> +		zfree((char **)&ui_browser__colorsets[i].fg);
> +}
> +
> +void ui_browser__free(void)
> +{
> +	ui_browser__free_color_configs();
> +}
> +
> +static void ui_browser__init_colorsets(void)
> +{
> +	int i, j;
> +
> +	for (i = 0; ui_browser__colorsets[i].name != NULL; ++i) {
> +		const char *name = ui_browser__colorsets[i].name;
> +
> +		for (j = 0; colors_config_items[j].name != NULL; j++) {
> +			if (!strcmp(name, colors_config_items[j].name)) {
> +				ui_browser__config_gcolors(&ui_browser__colorsets[i],
> +							   colors_config_items[j].value.s);
> +				break;
> +			}
> +		}
> +	}
> +}
> +
>  void ui_browser__init(void)
>  {
>  	int i = 0;
>  
> +	ui_browser__init_colorsets();
>  	perf_config(ui_browser__color_config, NULL);
>  
>  	while (ui_browser__colorsets[i].name) {
> diff --git a/tools/perf/ui/browser.h b/tools/perf/ui/browser.h
> index be3b70e..7a83a1a 100644
> --- a/tools/perf/ui/browser.h
> +++ b/tools/perf/ui/browser.h
> @@ -74,5 +74,6 @@ void ui_browser__list_head_seek(struct ui_browser *browser, off_t offset, int wh
>  unsigned int ui_browser__list_head_refresh(struct ui_browser *browser);
>  
>  void ui_browser__init(void);
> +void ui_browser__free(void);
>  void annotate_browser__init(void);
>  #endif /* _PERF_UI_BROWSER_H_ */
> diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
> index 7dfeba0..7999a57 100644
> --- a/tools/perf/ui/tui/setup.c
> +++ b/tools/perf/ui/tui/setup.c
> @@ -171,4 +171,5 @@ void ui__exit(bool wait_for_ok)
>  	SLang_reset_tty();
>  
>  	perf_error__unregister(&perf_tui_eops);
> +	ui_browser__free();
>  }
> diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h
> index 1f5a93c..8eab653 100644
> --- a/tools/perf/util/cache.h
> +++ b/tools/perf/util/cache.h
> @@ -7,6 +7,7 @@
>  #include <subcmd/pager.h>
>  #include "../perf.h"
>  #include "../ui/ui.h"
> +#include "config.h"
>  
>  #include <linux/string.h>
>  
> diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
> index 84414af..e0c8392 100644
> --- a/tools/perf/util/config.h
> +++ b/tools/perf/util/config.h
> @@ -44,6 +44,16 @@ struct perf_config_set {
>  	struct list_head sections;
>  };
>  
> +enum colors_config_items_idx {
> +	CONFIG_COLORS_TOP,
> +	CONFIG_COLORS_MEDIUM,
> +	CONFIG_COLORS_NORMAL,
> +	CONFIG_COLORS_SELECTED,
> +	CONFIG_COLORS_JUMP_ARROWS,
> +	CONFIG_COLORS_ADDR,
> +	CONFIG_COLORS_ROOT,
> +};
> +
>  #define CONF_VAR(_name, _field, _val, _type)			\
>  	{ .name = _name, .value._field = _val, .type = _type }
>  
> @@ -64,6 +74,8 @@ struct perf_config_set {
>  #define CONF_END()					\
>  	{ .name = NULL }
>  
> +extern const struct default_config_item colors_config_items[];
> +
>  struct perf_config_set *perf_config_set__new(void);
>  void perf_config_set__delete(struct perf_config_set *set);
>  
> -- 
> 2.5.0

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] perf tools: Separate out code setting ground colors from ui_browser__color_config
  2016-05-09 17:17   ` Arnaldo Carvalho de Melo
@ 2016-05-10 11:33     ` Taeung Song
  0 siblings, 0 replies; 13+ messages in thread
From: Taeung Song @ 2016-05-10 11:33 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar,
	Peter Zijlstra, Alexander Shishkin

Hi, Arnaldo :)

On 05/10/2016 02:17 AM, Arnaldo Carvalho de Melo wrote:
> Em Mon, May 09, 2016 at 08:41:47PM +0900, Taeung Song escreveu:
>> ui_browser__color_config() set foreground and background
>> colors values in ui_browser__colorsets.
>
> "ground colors" sounds strange, I guess referreing to them as "*colors"
> or "{back,fore}ground colors" is more appropriate, replace "gcolors"
> with "colors" too, please.

I got it.

>> But it can be reused by other functions so make ui_browser__config_gcolors()
>> bringing it from ui_browser__color_config().
>>
>> Cc: Namhyung Kim <namhyung@kernel.org>
>> Cc: Jiri Olsa <jolsa@kernel.org>
>> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
>> ---
>>   tools/perf/ui/browser.c | 43 ++++++++++++++++++++++++++-----------------
>>   1 file changed, 26 insertions(+), 17 deletions(-)
>>
>> diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c
>> index af68a9d..a477867 100644
>> --- a/tools/perf/ui/browser.c
>> +++ b/tools/perf/ui/browser.c
>> @@ -553,12 +553,33 @@ static struct ui_browser_colorset {
>>   	}
>>   };
>>
>> +static int ui_browser__config_gcolors(struct ui_browser_colorset *ui_browser_color,
>> +				      const char *value)
>> +{
>> +	char *fg = NULL, *bg;
>> +
>> +	fg = strdup(value);
>> +	if (fg == NULL)
>> +		return -1;
>> +
>> +	bg = strchr(fg, ',');
>> +	if (bg == NULL) {
>> +		free(fg);
>> +		return -1;
>> +	}
>> +
>> +	*bg = '\0';
>
> Isn't the above strtok()?
>
>> +	while (isspace(*++bg));
>
> Isn't this ltrim()?

I modified it like below and retested it !


diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c
index a477867..c905445 100644
--- a/tools/perf/ui/browser.c
+++ b/tools/perf/ui/browser.c
@@ -553,8 +553,8 @@ static struct ui_browser_colorset {
          }
  };

-static int ui_browser__config_gcolors(struct ui_browser_colorset 
*ui_browser_color,
-                                      const char *value)
+static int ui_browser__config_colors(struct ui_browser_colorset 
*ui_browser_color,
+                                     const char *value)
  {
          char *fg = NULL, *bg;

@@ -562,14 +562,12 @@ static int ui_browser__config_gcolors(struct 
ui_browser_colorset *ui_browser_col
          if (fg == NULL)
                  return -1;

-        bg = strchr(fg, ',');
+        bg = strtok(fg, ",");
          if (bg == NULL) {
                  free(fg);
                  return -1;
          }
-
-        *bg = '\0';
-        while (isspace(*++bg));
+        ltrim(bg);

          ui_browser_color->fg = fg;
          ui_browser_color->bg = bg;
@@ -591,7 +589,7 @@ static int ui_browser__color_config(const char *var, 
const char *value,
                  if (strcmp(ui_browser__colorsets[i].name, name) != 0)
                          continue;

-                ret = 
ui_browser__config_gcolors(&ui_browser__colorsets[i], value);
+                ret = 
ui_browser__config_colors(&ui_browser__colorsets[i], value);
                  break;
          }


I'll send this modified patch with v2


Thanks,
Taeung

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] perf config: Introduce default_config_item for all default config key-value pairs
  2016-05-09 17:17   ` Arnaldo Carvalho de Melo
@ 2016-05-10 11:49     ` Taeung Song
  2016-05-10 15:05       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 13+ messages in thread
From: Taeung Song @ 2016-05-10 11:49 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa, Namhyung Kim
  Cc: linux-kernel, Ingo Molnar, Peter Zijlstra, Alexander Shishkin,
	Masami Hiramatsu

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(&section->name);
>> +	zfree((char **)&section->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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/4] perf config: Initialize ui_browser__colorsets with default config items
  2016-05-09 17:19   ` Arnaldo Carvalho de Melo
@ 2016-05-10 11:57     ` Taeung Song
  0 siblings, 0 replies; 13+ messages in thread
From: Taeung Song @ 2016-05-10 11:57 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar,
	Peter Zijlstra, Alexander Shishkin



On 05/10/2016 02:19 AM, Arnaldo Carvalho de Melo wrote:
> Em Mon, May 09, 2016 at 08:41:48PM +0900, Taeung Song escreveu:
>> Set default config values for 'colors' section with 'colors_config_items[]'
>> instead of actual const char * type values.
>> (e.g. using colors_config_item[CONFIG_COLORS_TOP].value
>> instead of "red, default" string value for 'colors.top')
>
> looks ok

Thank you for your review !!
But I'm waiting for your opinion because of previous mail
if there's any change, I'll resend this patch after modifying it. :)

Thanks,
Taeung

>
>> Cc: Namhyung Kim <namhyung@kernel.org>
>> Cc: Jiri Olsa <jolsa@kernel.org>
>> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
>> ---
>>   tools/perf/ui/browser.c   | 46 ++++++++++++++++++++++++++++++++--------------
>>   tools/perf/ui/browser.h   |  1 +
>>   tools/perf/ui/tui/setup.c |  1 +
>>   tools/perf/util/cache.h   |  1 +
>>   tools/perf/util/config.h  | 12 ++++++++++++
>>   5 files changed, 47 insertions(+), 14 deletions(-)
>>
>> diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c
>> index a477867..385d0de 100644
>> --- a/tools/perf/ui/browser.c
>> +++ b/tools/perf/ui/browser.c
>> @@ -509,44 +509,30 @@ static struct ui_browser_colorset {
>>   	{
>>   		.colorset = HE_COLORSET_TOP,
>>   		.name	  = "top",
>> -		.fg	  = "red",
>> -		.bg	  = "default",
>>   	},
>>   	{
>>   		.colorset = HE_COLORSET_MEDIUM,
>>   		.name	  = "medium",
>> -		.fg	  = "green",
>> -		.bg	  = "default",
>>   	},
>>   	{
>>   		.colorset = HE_COLORSET_NORMAL,
>>   		.name	  = "normal",
>> -		.fg	  = "default",
>> -		.bg	  = "default",
>>   	},
>>   	{
>>   		.colorset = HE_COLORSET_SELECTED,
>>   		.name	  = "selected",
>> -		.fg	  = "black",
>> -		.bg	  = "yellow",
>>   	},
>>   	{
>>   		.colorset = HE_COLORSET_JUMP_ARROWS,
>>   		.name	  = "jump_arrows",
>> -		.fg	  = "blue",
>> -		.bg	  = "default",
>>   	},
>>   	{
>>   		.colorset = HE_COLORSET_ADDR,
>>   		.name	  = "addr",
>> -		.fg	  = "magenta",
>> -		.bg	  = "default",
>>   	},
>>   	{
>>   		.colorset = HE_COLORSET_ROOT,
>>   		.name	  = "root",
>> -		.fg	  = "white",
>> -		.bg	  = "blue",
>>   	},
>>   	{
>>   		.name = NULL,
>> @@ -591,6 +577,7 @@ static int ui_browser__color_config(const char *var, const char *value,
>>   		if (strcmp(ui_browser__colorsets[i].name, name) != 0)
>>   			continue;
>>
>> +		zfree((char **)&ui_browser__colorsets[i].fg);
>>   		ret = ui_browser__config_gcolors(&ui_browser__colorsets[i], value);
>>   		break;
>>   	}
>> @@ -745,10 +732,41 @@ void __ui_browser__line_arrow(struct ui_browser *browser, unsigned int column,
>>   		__ui_browser__line_arrow_down(browser, column, start, end);
>>   }
>>
>> +static void ui_browser__free_color_configs(void)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; ui_browser__colorsets[i].name != NULL; ++i)
>> +		zfree((char **)&ui_browser__colorsets[i].fg);
>> +}
>> +
>> +void ui_browser__free(void)
>> +{
>> +	ui_browser__free_color_configs();
>> +}
>> +
>> +static void ui_browser__init_colorsets(void)
>> +{
>> +	int i, j;
>> +
>> +	for (i = 0; ui_browser__colorsets[i].name != NULL; ++i) {
>> +		const char *name = ui_browser__colorsets[i].name;
>> +
>> +		for (j = 0; colors_config_items[j].name != NULL; j++) {
>> +			if (!strcmp(name, colors_config_items[j].name)) {
>> +				ui_browser__config_gcolors(&ui_browser__colorsets[i],
>> +							   colors_config_items[j].value.s);
>> +				break;
>> +			}
>> +		}
>> +	}
>> +}
>> +
>>   void ui_browser__init(void)
>>   {
>>   	int i = 0;
>>
>> +	ui_browser__init_colorsets();
>>   	perf_config(ui_browser__color_config, NULL);
>>
>>   	while (ui_browser__colorsets[i].name) {
>> diff --git a/tools/perf/ui/browser.h b/tools/perf/ui/browser.h
>> index be3b70e..7a83a1a 100644
>> --- a/tools/perf/ui/browser.h
>> +++ b/tools/perf/ui/browser.h
>> @@ -74,5 +74,6 @@ void ui_browser__list_head_seek(struct ui_browser *browser, off_t offset, int wh
>>   unsigned int ui_browser__list_head_refresh(struct ui_browser *browser);
>>
>>   void ui_browser__init(void);
>> +void ui_browser__free(void);
>>   void annotate_browser__init(void);
>>   #endif /* _PERF_UI_BROWSER_H_ */
>> diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
>> index 7dfeba0..7999a57 100644
>> --- a/tools/perf/ui/tui/setup.c
>> +++ b/tools/perf/ui/tui/setup.c
>> @@ -171,4 +171,5 @@ void ui__exit(bool wait_for_ok)
>>   	SLang_reset_tty();
>>
>>   	perf_error__unregister(&perf_tui_eops);
>> +	ui_browser__free();
>>   }
>> diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h
>> index 1f5a93c..8eab653 100644
>> --- a/tools/perf/util/cache.h
>> +++ b/tools/perf/util/cache.h
>> @@ -7,6 +7,7 @@
>>   #include <subcmd/pager.h>
>>   #include "../perf.h"
>>   #include "../ui/ui.h"
>> +#include "config.h"
>>
>>   #include <linux/string.h>
>>
>> diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
>> index 84414af..e0c8392 100644
>> --- a/tools/perf/util/config.h
>> +++ b/tools/perf/util/config.h
>> @@ -44,6 +44,16 @@ struct perf_config_set {
>>   	struct list_head sections;
>>   };
>>
>> +enum colors_config_items_idx {
>> +	CONFIG_COLORS_TOP,
>> +	CONFIG_COLORS_MEDIUM,
>> +	CONFIG_COLORS_NORMAL,
>> +	CONFIG_COLORS_SELECTED,
>> +	CONFIG_COLORS_JUMP_ARROWS,
>> +	CONFIG_COLORS_ADDR,
>> +	CONFIG_COLORS_ROOT,
>> +};
>> +
>>   #define CONF_VAR(_name, _field, _val, _type)			\
>>   	{ .name = _name, .value._field = _val, .type = _type }
>>
>> @@ -64,6 +74,8 @@ struct perf_config_set {
>>   #define CONF_END()					\
>>   	{ .name = NULL }
>>
>> +extern const struct default_config_item colors_config_items[];
>> +
>>   struct perf_config_set *perf_config_set__new(void);
>>   void perf_config_set__delete(struct perf_config_set *set);
>>
>> --
>> 2.5.0

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] perf config: Introduce default_config_item for all default config key-value pairs
  2016-05-10 11:49     ` Taeung Song
@ 2016-05-10 15:05       ` Arnaldo Carvalho de Melo
  2016-05-11 10:30         ` Taeung Song
  0 siblings, 1 reply; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-05-10 15:05 UTC (permalink / raw)
  To: Taeung Song
  Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Namhyung Kim, linux-kernel,
	Ingo Molnar, Peter Zijlstra, Alexander Shishkin,
	Masami Hiramatsu

Em Tue, May 10, 2016 at 08:49:16PM +0900, Taeung Song escreveu:
> 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
> +

No hard coded magical maximum values :-\

>  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(&section->name);
> > > +	zfree((char **)&section->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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] perf config: Introduce default_config_item for all default config key-value pairs
  2016-05-10 15:05       ` Arnaldo Carvalho de Melo
@ 2016-05-11 10:30         ` Taeung Song
  0 siblings, 0 replies; 13+ messages in thread
From: Taeung Song @ 2016-05-11 10:30 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, linux-kernel, Ingo Molnar,
	Peter Zijlstra, Alexander Shishkin, Masami Hiramatsu

Hi, Arnaldo

On 05/11/2016 12:05 AM, Arnaldo Carvalho de Melo wrote:
> Em Tue, May 10, 2016 at 08:49:16PM +0900, Taeung Song escreveu:
>> 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
>> +
>
> No hard coded magical maximum values :-\
>

I got it.
I'll send v2 without the above change. :)

Thanks,
Taeung

>>   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(&section->name);
>>>> +	zfree((char **)&section->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

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2016-05-11 10:30 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-09 11:41 [PATCH 0/4] perf config: Introduce default config key-value pairs arrays Taeung Song
2016-05-09 11:41 ` [PATCH 1/4] perf config: Introduce default_config_item for all default config key-value pairs Taeung Song
2016-05-09 17:17   ` Arnaldo Carvalho de Melo
2016-05-10 11:49     ` Taeung Song
2016-05-10 15:05       ` Arnaldo Carvalho de Melo
2016-05-11 10:30         ` Taeung Song
2016-05-09 11:41 ` [PATCH 2/4] perf tools: Separate out code setting ground colors from ui_browser__color_config Taeung Song
2016-05-09 17:17   ` Arnaldo Carvalho de Melo
2016-05-10 11:33     ` Taeung Song
2016-05-09 11:41 ` [PATCH 3/4] perf config: Initialize ui_browser__colorsets with default config items Taeung Song
2016-05-09 17:19   ` Arnaldo Carvalho de Melo
2016-05-10 11:57     ` Taeung Song
2016-05-09 11:41 ` [PATCH 4/4] perf config: Initialize annotate_browser__opts " Taeung Song

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.