[RFC][PATCH v10 0/2] Infrastructure code for perf-config

[RFC][PATCH v10 0/2] Infrastructure code for perf-config

[Taeung Song]

Hi,

We can use the config files (i.e user wide ~/.perfconfig
and system wide $(sysconfdir)/perfconfig)
to configure perf tools. perf-config help user
manage the config files, not manually look into or edit them.

Introduce new infrastructure code for config
management features of perf-config subcommand.

This pathset contains basic code for various purposes of configuration management
showing current configs, in the near future,
showing all configs with default value,
getting current configs from the config files
or writing configs that user type on the config files, etc.

IMHO, I think this infrastructure code is needed
to add new funcationalities for config management of perf-config.

If anyone reviews this, I'd appreciate it.

Thanks,
Taeung

v10:
- fix default values of colors.normal and colors.selected
  according ui_browser__colorsets[] at ui/browser.c
- recheck all default config values looking into relevant codes
- v9 1/2, 2/2 acked by Namhyung Kim (see https://lkml.org/lkml/2016/4/20/834)

v9:
- don't use the arbitrary maximum 'MAX_CONFIGS' (Arnaldo, Namhyung)
- change two-dimensinal arrays 'default_config_items' to array of pointers (Namhyung)
- remove needless 'enum perf_config_secion_idx'
- add sections 'intel-pt','convert' and their items
- modify perf_config_set__init() in accordance with new default config sections and items
- (applied two previous patches 860b8d4 and 20105ca from this patchset)

v8:
- rebased onto the current acme/perf/core

v7:
- rename 'is_custom' to 'is_allocated' to be proper (Masami)
- fix the code about free() or zfree() in perf_config_*__delete() (Masami)
- check set == NULL or not in show_config() (Masami)

v6:
- don't use goto in add_config_item() (Masami)

v5:
- departmentalize perf_config_set__delete() (Arnaldo)
- remove confusing find_config() (Arnaldo)
- use pr_debug() instead of pr_err() (Arnaldo)
- use zfree() instead of free() (Arnaldo)
- more compact in perf_config_set__new() (Arnaldo)
- rename variables 'perf_configs', 'config_items', etc. (Arnaldo)

v4:
- fill perf_config_set__delete() in collect_config() for state of error
- fill the code setting is_custom value in add_config_item() (Namhyung)

v3:
- use the section list that contains configs each section
  instead of the single config list (Namhyung)
- exclude a patch for '--list-all' option from this patchset

v2:
- remove perf_config_kind (user, system or both config files)
  and needless at this time, etc. (Namhyung)
- separate this patch as several patches (Namhyung)
- fix typing errors, etc.

Taeung Song (2):
  perf config: Prepare all default configs
  perf config: Initialize perf_config_set with all default configs

 tools/perf/builtin-config.c |  11 ++-
 tools/perf/util/config.c    | 187 ++++++++++++++++++++++++++++++++++++++++++--
 tools/perf/util/config.h    |  51 +++++++++++-
 3 files changed, 238 insertions(+), 11 deletions(-)

-- 
2.5.0

[PATCH v10 1/2] perf config: Prepare all default configs

[Taeung Song]

To precisely manage configs,
prepare all default perf's configs that contain
default section name, variable name, value
and correct type, not string type.

In the near future, this will be used when
checking type of config variable or showing
all configs with default values, etc.

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 | 152 ++++++++++++++++++++++++++++++++++++++++++++++-
 tools/perf/util/config.h |  44 +++++++++++++-
 2 files changed, 192 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index dad7d82..e09ecbb 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;
 
+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" },
+};
+
+struct perf_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()
+};
+
+struct perf_config_item tui_config_items[] = {
+	CONF_BOOL_VAR("report", true),
+	CONF_BOOL_VAR("annotate", true),
+	CONF_BOOL_VAR("top", true),
+	CONF_END()
+};
+
+struct perf_config_item buildid_config_items[] = {
+	CONF_STR_VAR("dir", "~/.debug"),
+	CONF_END()
+};
+
+struct perf_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()
+};
+
+struct perf_config_item gtk_config_items[] = {
+	CONF_BOOL_VAR("annotate", false),
+	CONF_BOOL_VAR("report", false),
+	CONF_BOOL_VAR("top", false),
+	CONF_END()
+};
+
+struct perf_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()
+};
+
+struct perf_config_item help_config_items[] = {
+	CONF_STR_VAR("format", "man"),
+	CONF_INT_VAR("autocorrect", 0),
+	CONF_END()
+};
+
+struct perf_config_item hist_config_items[] = {
+	CONF_STR_VAR("percentage", "absolute"),
+	CONF_END()
+};
+
+struct perf_config_item ui_config_items[] = {
+	CONF_BOOL_VAR("show-headers", true),
+	CONF_END()
+};
+
+struct perf_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()
+};
+
+struct perf_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()
+};
+
+struct perf_config_item top_config_items[] = {
+	CONF_BOOL_VAR("children", true),
+	CONF_END()
+};
+
+struct perf_config_item man_config_items[] = {
+	CONF_STR_VAR("viewer", "man"),
+	CONF_END()
+};
+
+struct perf_config_item kmem_config_items[] = {
+	CONF_STR_VAR("default", "slab"),
+	CONF_END()
+};
+
+struct perf_config_item intel_pt_config_items[] = {
+	CONF_INT_VAR("cache-divisor", 64),
+	CONF_BOOL_VAR("mispred-all", false),
+	CONF_END()
+};
+
+struct perf_config_item convert_config_items[] = {
+	CONF_U64_VAR("queue-size", 0),
+	CONF_END()
+};
+
+struct perf_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;
@@ -659,7 +807,7 @@ struct perf_config_set *perf_config_set__new(void)
 
 static void perf_config_item__delete(struct perf_config_item *item)
 {
-	zfree(&item->name);
+	zfree((char **)&item->name);
 	zfree(&item->value);
 	free(item);
 }
@@ -677,7 +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..297b33e 100644
--- a/tools/perf/util/config.h
+++ b/tools/perf/util/config.h
@@ -4,14 +4,34 @@
 #include <stdbool.h>
 #include <linux/list.h>
 
+enum perf_config_type {
+	CONFIG_TYPE_BOOL,
+	CONFIG_TYPE_INT,
+	CONFIG_TYPE_LONG,
+	CONFIG_TYPE_U64,
+	CONFIG_TYPE_FLOAT,
+	CONFIG_TYPE_DOUBLE,
+	CONFIG_TYPE_STRING
+};
+
 struct perf_config_item {
-	char *name;
+	const char *name;
 	char *value;
+	union {
+		bool b;
+		int i;
+		u32 l;
+		u64 ll;
+		float f;
+		double d;
+		const char *s;
+	} default_value;
+	enum perf_config_type type;
 	struct list_head node;
 };
 
 struct perf_config_section {
-	char *name;
+	const char *name;
 	struct list_head items;
 	struct list_head node;
 };
@@ -20,6 +40,26 @@ struct perf_config_set {
 	struct list_head sections;
 };
 
+#define CONF_VAR(_name, _field, _val, _type)				\
+	{ .name = _name, .default_value._field = _val, .type = _type }
+
+#define CONF_BOOL_VAR(_name, _val)			\
+	CONF_VAR(_name, b, _val, CONFIG_TYPE_BOOL)
+#define CONF_INT_VAR(_name, _val)			\
+	CONF_VAR(_name, i, _val, CONFIG_TYPE_INT)
+#define CONF_LONG_VAR(_name, _val)			\
+	CONF_VAR(_name, l, _val, CONFIG_TYPE_LONG)
+#define CONF_U64_VAR(_name, _val)			\
+	CONF_VAR(_name, ll, _val, CONFIG_TYPE_U64)
+#define CONF_FLOAT_VAR(_name, _val)			\
+	CONF_VAR(_name, f, _val, CONFIG_TYPE_FLOAT)
+#define CONF_DOUBLE_VAR(_name, _val)			\
+	CONF_VAR(_name, d, _val, CONFIG_TYPE_DOUBLE)
+#define CONF_STR_VAR(_name, _val)			\
+	CONF_VAR(_name, s, _val, CONFIG_TYPE_STRING)
+#define CONF_END()					\
+	{ .name = NULL }
+
 struct perf_config_set *perf_config_set__new(void);
 void perf_config_set__delete(struct perf_config_set *set);
 
-- 
2.5.0

[PATCH v10 2/2] perf config: Initialize perf_config_set with all default configs

[Taeung Song]

To avoid duplicated config variables and
use perf_config_set classifying between standard
perf config variables and unknown or new config
variables other than them, initialize perf_config_set
with all default configs.

And this will be needed when showing all configs with
default value or checking correct type of a config
variable in the near future.

Acked-by: 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/builtin-config.c | 11 +++++++----
 tools/perf/util/config.c    | 39 ++++++++++++++++++++++++++++++++++-----
 tools/perf/util/config.h    |  7 +++++++
 3 files changed, 48 insertions(+), 9 deletions(-)

diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
index fe1b77f..0fe9bc5 100644
--- a/tools/perf/builtin-config.c
+++ b/tools/perf/builtin-config.c
@@ -35,6 +35,7 @@ static struct option config_options[] = {
 
 static int show_config(struct perf_config_set *set)
 {
+	bool has_value = false;
 	struct perf_config_section *section;
 	struct perf_config_item *item;
 	struct list_head *sections;
@@ -43,19 +44,21 @@ static int show_config(struct perf_config_set *set)
 		return -1;
 
 	sections = &set->sections;
-	if (list_empty(sections))
-		return -1;
-
 	list_for_each_entry(section, sections, node) {
 		list_for_each_entry(item, &section->items, node) {
 			char *value = item->value;
 
-			if (value)
+			if (value) {
 				printf("%s.%s=%s\n", section->name,
 				       item->name, value);
+				has_value = true;
+			}
 		}
 	}
 
+	if (!has_value)
+		return -1;
+
 	return 0;
 }
 
diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index e09ecbb..a19a15c 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -705,6 +705,7 @@ static struct perf_config_section *add_section(struct list_head *sections,
 	if (!section)
 		return NULL;
 
+	section->is_allocated = true;
 	INIT_LIST_HEAD(&section->items);
 	section->name = strdup(section_name);
 	if (!section->name) {
@@ -725,6 +726,7 @@ static struct perf_config_item *add_config_item(struct perf_config_section *sect
 	if (!item)
 		return NULL;
 
+	item->is_allocated = true;
 	item->name = strdup(name);
 	if (!item->name) {
 		pr_debug("%s: strdup failed\n", __func__);
@@ -793,12 +795,35 @@ out_free:
 	return -1;
 }
 
+static struct perf_config_set *perf_config_set__init(struct perf_config_set *set)
+{
+	unsigned int i, j;
+	struct perf_config_section *section;
+	struct perf_config_item *items;
+	struct list_head *sections = &set->sections;
+
+	INIT_LIST_HEAD(&set->sections);
+
+	for (i = 0; i < ARRAY_SIZE(default_sections); i++) {
+		section = &default_sections[i];
+		INIT_LIST_HEAD(&section->items);
+
+		items = default_config_items[i];
+		for (j = 0; items[j].name != NULL; j++)
+			list_add_tail(&items[j].node, &section->items);
+
+		list_add_tail(&section->node, sections);
+	}
+
+	return set;
+}
+
 struct perf_config_set *perf_config_set__new(void)
 {
 	struct perf_config_set *set = zalloc(sizeof(*set));
 
 	if (set) {
-		INIT_LIST_HEAD(&set->sections);
+		perf_config_set__init(set);
 		perf_config(collect_config, set);
 	}
 
@@ -807,9 +832,11 @@ struct perf_config_set *perf_config_set__new(void)
 
 static void perf_config_item__delete(struct perf_config_item *item)
 {
-	zfree((char **)&item->name);
 	zfree(&item->value);
-	free(item);
+	if (item->is_allocated) {
+		zfree((char **)&item->name);
+		free(item);
+	}
 }
 
 static void perf_config_section__purge(struct perf_config_section *section)
@@ -825,8 +852,10 @@ 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((char **)&section->name);
-	free(section);
+	if (section->is_allocated) {
+		zfree((char **)&section->name);
+		free(section);
+	}
 }
 
 static void perf_config_set__purge(struct perf_config_set *set)
diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
index 297b33e..e093143 100644
--- a/tools/perf/util/config.h
+++ b/tools/perf/util/config.h
@@ -14,6 +14,11 @@ enum perf_config_type {
 	CONFIG_TYPE_STRING
 };
 
+/**
+ * struct perf_config_item - element of perf's configs
+ *
+ * @is_allocated: unknown or new config other than default config
+ */
 struct perf_config_item {
 	const char *name;
 	char *value;
@@ -27,11 +32,13 @@ struct perf_config_item {
 		const char *s;
 	} default_value;
 	enum perf_config_type type;
+	bool is_allocated;
 	struct list_head node;
 };
 
 struct perf_config_section {
 	const char *name;
+	bool is_allocated;
 	struct list_head items;
 	struct list_head node;
 };
-- 
2.5.0

Re: [PATCH v10 1/2] perf config: Prepare all default configs

[Taeung Song]

Hi,

On 04/22/2016 07:21 PM, Taeung Song wrote:
> To precisely manage configs,
> prepare all default perf's configs that contain
> default section name, variable name, value
> and correct type, not string type.
>
> In the near future, this will be used when
> checking type of config variable or showing
> all configs with default values, etc.
>
> 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 | 152 ++++++++++++++++++++++++++++++++++++++++++++++-
>   tools/perf/util/config.h |  44 +++++++++++++-
>   2 files changed, 192 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
> index dad7d82..e09ecbb 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;
>
> +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" },
> +};
> +
> +struct perf_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()
> +};
> +
> +struct perf_config_item tui_config_items[] = {
> +	CONF_BOOL_VAR("report", true),
> +	CONF_BOOL_VAR("annotate", true),
> +	CONF_BOOL_VAR("top", true),
> +	CONF_END()
> +};
> +
> +struct perf_config_item buildid_config_items[] = {
> +	CONF_STR_VAR("dir", "~/.debug"),
> +	CONF_END()
> +};
> +
> +struct perf_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()
> +};
> +
> +struct perf_config_item gtk_config_items[] = {
> +	CONF_BOOL_VAR("annotate", false),
> +	CONF_BOOL_VAR("report", false),
> +	CONF_BOOL_VAR("top", false),
> +	CONF_END()
> +};
> +
> +struct perf_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()
> +};
> +
> +struct perf_config_item help_config_items[] = {
> +	CONF_STR_VAR("format", "man"),
> +	CONF_INT_VAR("autocorrect", 0),
> +	CONF_END()
> +};
> +
> +struct perf_config_item hist_config_items[] = {
> +	CONF_STR_VAR("percentage", "absolute"),
> +	CONF_END()
> +};
> +
> +struct perf_config_item ui_config_items[] = {
> +	CONF_BOOL_VAR("show-headers", true),
> +	CONF_END()
> +};
> +
> +struct perf_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()
> +};
> +
> +struct perf_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()
> +};
> +
> +struct perf_config_item top_config_items[] = {
> +	CONF_BOOL_VAR("children", true),
> +	CONF_END()
> +};
> +
> +struct perf_config_item man_config_items[] = {
> +	CONF_STR_VAR("viewer", "man"),
> +	CONF_END()
> +};
> +
> +struct perf_config_item kmem_config_items[] = {
> +	CONF_STR_VAR("default", "slab"),
> +	CONF_END()
> +};
> +
> +struct perf_config_item intel_pt_config_items[] = {
> +	CONF_INT_VAR("cache-divisor", 64),
> +	CONF_BOOL_VAR("mispred-all", false),
> +	CONF_END()
> +};
> +
> +struct perf_config_item convert_config_items[] = {
> +	CONF_U64_VAR("queue-size", 0),
> +	CONF_END()
> +};
> +
> +struct perf_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;
> @@ -659,7 +807,7 @@ struct perf_config_set *perf_config_set__new(void)
>
>   static void perf_config_item__delete(struct perf_config_item *item)
>   {
> -	zfree(&item->name);
> +	zfree((char **)&item->name);
>   	zfree(&item->value);
>   	free(item);
>   }
> @@ -677,7 +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..297b33e 100644
> --- a/tools/perf/util/config.h
> +++ b/tools/perf/util/config.h
> @@ -4,14 +4,34 @@
>   #include <stdbool.h>
>   #include <linux/list.h>
>
> +enum perf_config_type {
> +	CONFIG_TYPE_BOOL,
> +	CONFIG_TYPE_INT,
> +	CONFIG_TYPE_LONG,
> +	CONFIG_TYPE_U64,
> +	CONFIG_TYPE_FLOAT,
> +	CONFIG_TYPE_DOUBLE,
> +	CONFIG_TYPE_STRING
> +};
> +
>   struct perf_config_item {
> -	char *name;
> +	const char *name;
>   	char *value;
> +	union {
> +		bool b;
> +		int i;
> +		u32 l;
> +		u64 ll;
> +		float f;
> +		double d;
> +		const char *s;
> +	} default_value;
> +	enum perf_config_type type;

I have worries about mismatch between types of actual variables
which are related each config value
and types of 'default_value' in struct perf_config_item.

I rechecked all config default values looking into relevant source codes.

After the checking,
I thought that the reason why we use actual type in perf_config_item
is because of checking correct type of config values and
a possibility to replace raw values of actual variables with particular 
values of default_config_item[] e.g.


diff --git a/tools/perf/ui/browsers/annotate.c 
b/tools/perf/ui/browsers/annotate.c
index 4fc208e..a0d27ca 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -38,8 +38,8 @@ static struct annotate_browser_opt {
               show_nr_jumps,
               show_total_period;
  } annotate_browser__opts = {
-        .use_offset     = true,
-        .jump_arrows    = true,
+        .use_offset     = 
(bool)annotate_config_items[CONFIG_ANNOTATE_USE_OFFSET].default_value,
+        .jump_arrows    = 
(bool)annotate_config_items[CONFIG_ANNOTATE_JUMP_ARROWS].default_value,
  };


But I found actual variables related each config value can be
enum chain_mode, enum perf_call_graph_mode or etc.

and config 'hist.percentage' can be a string 'absolute' or 'relative'
but actual type of the variable(symbol_conf.filter_relative)
is bool type i.e.

     struct symbol_conf {
             unsigned short  priv_size;
             unsigned short  nr_events;
             bool            try_vmlinux_path,
                             force,
     ..(omitted)...
                             filter_relative,
     ...(omitted)...


Isn't the mismatch a problem ?

If we just use type casting or each function to convert type
(e.g. parse_filter_percentage(), etc.), the mismatch couldn't be
a problem ? or

Is it better to add each enum types into union type of struct 
perf_config_item ?

or is it better to fix default values according types of the actual 
variables e.g.


diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index a19a15c..231a2bc 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -104,7 +104,7 @@ struct perf_config_item help_config_items[] = {
  };

  struct perf_config_item hist_config_items[] = {
-        CONF_STR_VAR("percentage", "absolute"),
+        CONF_STR_VAR("percentage", false),
          CONF_END()
  };


I want to know your opinions.
If anyone have other idea and tell me,
I'd appreciate it. :-)


Thanks,
Taeung

>   	struct list_head node;
>   };
>
>   struct perf_config_section {
> -	char *name;
> +	const char *name;
>   	struct list_head items;
>   	struct list_head node;
>   };
> @@ -20,6 +40,26 @@ struct perf_config_set {
>   	struct list_head sections;
>   };
>
> +#define CONF_VAR(_name, _field, _val, _type)				\
> +	{ .name = _name, .default_value._field = _val, .type = _type }
> +
> +#define CONF_BOOL_VAR(_name, _val)			\
> +	CONF_VAR(_name, b, _val, CONFIG_TYPE_BOOL)
> +#define CONF_INT_VAR(_name, _val)			\
> +	CONF_VAR(_name, i, _val, CONFIG_TYPE_INT)
> +#define CONF_LONG_VAR(_name, _val)			\
> +	CONF_VAR(_name, l, _val, CONFIG_TYPE_LONG)
> +#define CONF_U64_VAR(_name, _val)			\
> +	CONF_VAR(_name, ll, _val, CONFIG_TYPE_U64)
> +#define CONF_FLOAT_VAR(_name, _val)			\
> +	CONF_VAR(_name, f, _val, CONFIG_TYPE_FLOAT)
> +#define CONF_DOUBLE_VAR(_name, _val)			\
> +	CONF_VAR(_name, d, _val, CONFIG_TYPE_DOUBLE)
> +#define CONF_STR_VAR(_name, _val)			\
> +	CONF_VAR(_name, s, _val, CONFIG_TYPE_STRING)
> +#define CONF_END()					\
> +	{ .name = NULL }
> +
>   struct perf_config_set *perf_config_set__new(void);
>   void perf_config_set__delete(struct perf_config_set *set);
>
>