Re: [PATCH v7 3/7] perf config: Add a option 'list-all' to perf-config

[Namhyung Kim <namhyung@kernel.org>]

On Sun, Oct 04, 2015 at 04:35:06PM +0900, Taeung Song wrote:
> A option 'list-all' is to display both current config variables and
> all possible config variables with default values.
> The syntax examples are like below
> 
>     perf config [<file-option>] [options]
> 
>     display all perf config with default values.
>     # perf config -a | --list-all
> 
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
> ---
>  tools/perf/Documentation/perf-config.txt |   6 +
>  tools/perf/builtin-config.c              | 339 ++++++++++++++++++++++++++++++-
>  2 files changed, 343 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt
> index a42d409..15fbbd9 100644
> --- a/tools/perf/Documentation/perf-config.txt
> +++ b/tools/perf/Documentation/perf-config.txt
> @@ -9,6 +9,8 @@ SYNOPSIS
>  --------
>  [verse]
>  'perf config' [<file-option>] -l | --list
> +or
> +'perf config' [<file-option>] -a | --list-all
>  
>  DESCRIPTION
>  -----------
> @@ -29,6 +31,10 @@ OPTIONS
>  	For writing and reading options: write to system-wide
>  	'$(sysconfdir)/perfconfig' or read it.
>  
> +-a::
> +--list-all::
> +	Show current and all possible config variables with default values.
> +
>  CONFIGURATION FILE
>  ------------------
>  
> diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
> index 92be3eb..8e16e18 100644
> --- a/tools/perf/builtin-config.c
> +++ b/tools/perf/builtin-config.c
> @@ -21,7 +21,8 @@ static const char * const config_usage[] = {
>  };
>  
>  enum actions {
> -	ACTION_LIST = 1
> +	ACTION_LIST = 1,
> +	ACTION_LIST_ALL
>  } actions;
>  
>  static struct option config_options[] = {
> @@ -31,8 +32,292 @@ static struct option config_options[] = {
>  	OPT_GROUP("Action"),
>  	OPT_SET_UINT('l', "list", &actions,
>  		     "show current config variables", ACTION_LIST),
> +	OPT_SET_UINT('a', "list-all", &actions,
> +		     "show current and all possible config variables with default values",
> +		     ACTION_LIST_ALL),
>  	OPT_END()
>  };
> +
> +/* section names */
> +#define COLORS "colors"
> +#define TUI "tui"
> +#define BUILDID "buildid"
> +#define ANNOTATE "annotate"
> +#define GTK "gtk"
> +#define PAGER "pager"
> +#define HELP "help"
> +#define HIST "hist"
> +#define UI "ui"
> +#define CALL_GRAPH "call-graph"
> +#define REPORT "report"
> +#define TOP "top"
> +#define MAN "man"
> +#define KMEM "kmem"
> +
> +/* config variable types */
> +#define TYPE_INT "int"
> +#define TYPE_LONG "long"
> +#define TYPE_DIRNAME "dirname"
> +#define TYPE_BOOL "bool"
> +
> +static struct default_configset {
> +	const char *section_name;
> +	const char *name, *value, *type;
> +
> +} default_configsets[] = {

I think it should in sync with the actual value.  So how about
changing the code to use this table instead of hard-coded value?

Maybe something like this?  (untested!!)

  enum config_idx {
    CONFIG_COLORS_TOP,
    CONFIG_COLORS_MEDIUM,
    CONFIG_COLORS_NORMAL,
    ...
    CONFIG_ITEM_MAX,
  };

  enum config_type {
    CONFIG_TYPE_BOOL,
    CONFIG_TYPE_INT,
    CONFIG_TYPE_STRING,
    ...
  };

  struct config_item {
    const char *section;
    const char *name;
    union {
      bool   b;
      int    i;
      char   *s;
      ...
    } value;
    enum config_type type;
  }

  #define CONF_VAR(_sec, _name, _field, _val, _type) \
  { .section = _sec, .name = name, .value._field = _val, .type = _type }

  #define CONF_BOOL_VAR(_idx, _sec, _name, _val) \
    [CONFIG_##_idx] = CONF_VAR(_sec, _name, b, _val, CONFIG_TYPE_BOOL)
  #define CONF_INT_VAR(_idx, _sec, _name, _val) \
    [CONFIG_##_idx] = CONF_VAR(_sec, _name, i, _val, CONFIG_TYPE_INT)
  #define CONF_STR_VAR(_idx, _sec, _name, _val) \
    [CONFIG_##_idx] = CONF_VAR(_sec, _name, s, _val, CONFIG_TYPE_STRING)


  struct config_item default_configs[] = {
    CONF_STR_VAR(COLORS_TOP, "colors", "top", "red, default"),
    CONF_STR_VAR(COLORS_TOP, "colors", "medium", "green, default"),
    ...
    CONF_BOOL_VAR(UI_SHOW_HEADERS, "ui", "show-headers", true),
    ...
  };


And then we can use it, for example, like below..

util/symbol.c:

struct symbol_conf symbol_conf = {
  ...
  .show_hist_headers = default_configs[CONFIG_UI_SHOW_HEADERS].value.b,
  ...
};

or

#define CONFIG_DEFAULT_BOOL(_sec, _name) \
  default_configs[CONFIG_##_sec##_##_name].value.b
...

struct symbol_conf symbol_conf = {
  ...
  .show_hist_headers = CONFIG_DEFAULT_BOOL(UI, SHOW_HEADERS),
  ...
};


Thanks,
Namhyung


> +	{
> +		.section_name = COLORS,
> +		.name = "top",
> +		.value = "red, default",
> +		.type = NULL,
> +	},
> +	{
> +		.section_name = COLORS,
> +		.name = "medium",
> +		.value = "green, default",
> +		.type = NULL,
> +	},
> +	{
> +		.section_name = COLORS,
> +		.name = "normal",
> +		.value = "lightgray, default",
> +		.type = NULL,
> +	},
> +	{
> +		.section_name = COLORS,
> +		.name = "selected",
> +		.value = "white, lightgray",
> +		.type = NULL,
> +	},
> +	{
> +		.section_name = COLORS,
> +		.name = "code",
> +		.value = "blue, default",
> +		.type = NULL,
> +	},
> +	{
> +		.section_name = COLORS,
> +		.name = "addr",
> +		.value = "magenta, default",
> +		.type = NULL,
> +	},
> +	{
> +		.section_name = COLORS,
> +		.name = "root",
> +		.value = "white, blue",
> +		.type = NULL,
> +	},
> +	{
> +		.section_name = TUI,
> +		.name = "report",
> +		.value = "true",
> +		.type = TYPE_BOOL,
> +	},
> +	{
> +		.section_name = TUI,
> +		.name = "annotate",
> +		.value = "true",
> +		.type = TYPE_BOOL,
> +	},
> +	{
> +		.section_name = TUI,
> +		.name = "top",
> +		.value = "true",
> +		.type = TYPE_BOOL,
> +	},
> +	{
> +		.section_name = BUILDID,
> +		.name = "dir",
> +		.value = "~/.debug",
> +		.type = TYPE_DIRNAME,
> +	},
> +	{
> +		.section_name = ANNOTATE,
> +		.name = "hide_src_code",
> +		.value = "false",
> +		.type = TYPE_BOOL,
> +	},
> +	{
> +		.section_name = ANNOTATE,
> +		.name = "use_offset",
> +		.value = "true",
> +		.type = TYPE_BOOL,
> +	},
> +	{
> +		.section_name = ANNOTATE,
> +		.name = "jump_arrows",
> +		.value = "true",
> +		.type = TYPE_BOOL,
> +	},
> +	{
> +		.section_name = ANNOTATE,
> +		.name = "show_nr_jumps",
> +		.value = "false",
> +		.type = TYPE_BOOL,
> +	},
> +	{
> +		.section_name = GTK,
> +		.name = "annotate",
> +		.value = "false",
> +		.type = TYPE_BOOL,
> +	},
> +	{
> +		.section_name = GTK,
> +		.name = "report",
> +		.value = "false",
> +		.type = TYPE_BOOL,
> +	},
> +	{
> +		.section_name = GTK,
> +		.name = "top",
> +		.value = "false",
> +		.type = TYPE_BOOL,
> +	},
> +	{
> +		.section_name = PAGER,
> +		.name = "cmd",
> +		.value = "true",
> +		.type = TYPE_BOOL,
> +	},
> +	{
> +		.section_name = PAGER,
> +		.name = "report",
> +		.value = "true",
> +		.type = TYPE_BOOL,
> +	},
> +	{
> +		.section_name = PAGER,
> +		.name = "annotate",
> +		.value = "true",
> +		.type = TYPE_BOOL,
> +	},
> +	{
> +		.section_name = PAGER,
> +		.name = "record",
> +		.value = "true",
> +		.type = TYPE_BOOL,
> +	},
> +	{
> +		.section_name = PAGER,
> +		.name = "top",
> +		.value = "true",
> +		.type = TYPE_BOOL,
> +	},
> +	{
> +		.section_name = PAGER,
> +		.name = "diff",
> +		.value = "true",
> +		.type = TYPE_BOOL,
> +	},
> +	{
> +		.section_name = HELP,
> +		.name = "format",
> +		.value = "man",
> +		.type = NULL,
> +	},
> +	{
> +		.section_name = HELP,
> +		.name = "autocorrect",
> +		.value = "0",
> +		.type = NULL,
> +	},
> +	{
> +		.section_name = HIST,
> +		.name = "percentage",
> +		.value = "absolute",
> +		.type = NULL,
> +	},
> +	{
> +		.section_name = UI,
> +		.name = "show-headers",
> +		.value = "true",
> +		.type = TYPE_BOOL,
> +	},
> +	{
> +		.section_name = CALL_GRAPH,
> +		.name = "record-mode",
> +		.value = "fp",
> +	},
> +	{
> +		.section_name = CALL_GRAPH,
> +		.name = "dump-size",
> +		.value = "8192",
> +		.type = TYPE_INT,
> +	},
> +	{
> +		.section_name = CALL_GRAPH,
> +		.name = "print-type",
> +		.value = "fractal",
> +		.type = NULL,
> +	},
> +	{
> +		.section_name = CALL_GRAPH,
> +		.name = "order",
> +		.value = "caller",
> +		.type = NULL,
> +	},
> +	{
> +		.section_name = CALL_GRAPH,
> +		.name = "sort-key",
> +		.value = "function",
> +		.type = NULL,
> +	},
> +	{
> +		.section_name = CALL_GRAPH,
> +		.name = "threshold",
> +		.value = "0.5",
> +		.type = TYPE_LONG,
> +	},
> +	{
> +		.section_name = CALL_GRAPH,
> +		.name = "print-limit",
> +		.value = "0",
> +		.type = TYPE_INT,
> +	},
> +	{
> +		.section_name = REPORT,
> +		.name = "children",
> +		.value = "true",
> +		.type = TYPE_BOOL,
> +	},
> +	{
> +		.section_name = REPORT,
> +		.name = "percent-limit",
> +		.value = "0",
> +		.type = TYPE_INT,
> +	},
> +	{
> +		.section_name = REPORT,
> +		.name = "queue-size",
> +		.value = "0",
> +		.type = TYPE_INT,
> +	},
> +	{
> +		.section_name = TOP,
> +		.name = "children",
> +		.value = "false",
> +		.type = TYPE_BOOL,
> +	},
> +	{
> +		.section_name = MAN,
> +		.name = "viewer",
> +		.value = "man",
> +		.type = NULL,
> +	},
> +	{
> +		.section_name = KMEM,
> +		.name = "default",
> +		.value = "slab",
> +		.type = NULL,
> +	},
> +	{
> +		.section_name = NULL,
> +		.name = NULL,
> +		.value = NULL,
> +		.type = NULL,
> +	},
> +};
> +
>  static int show_config(struct list_head *sections)
>  {
>  	struct config_section *section;
> @@ -128,6 +413,45 @@ static int add_element(struct list_head *head,
>  	return 0;
>  }
>  
> +static int show_all_config(struct list_head *sections)
> +{
> +	int i;
> +	bool has_config;
> +	struct config_section *section;
> +	struct config_element *element;
> +
> +	for (i = 0; default_configsets[i].section_name != NULL; i++) {
> +		find_config(sections, &section, &element,
> +			    default_configsets[i].section_name, default_configsets[i].name);
> +
> +		if (!element)
> +			printf("%s.%s=%s\n", default_configsets[i].section_name,
> +			       default_configsets[i].name, default_configsets[i].value);
> +		else
> +			printf("%s.%s=%s\n", section->name,
> +			       element->name, element->value);
> +	}
> +
> +	/* Print config variables the default configsets haven't */
> +	list_for_each_entry(section, sections, list) {
> +		list_for_each_entry(element, &section->element_head, list) {
> +			has_config = false;
> +			for (i = 0; default_configsets[i].section_name != NULL; i++) {
> +				if (!strcmp(default_configsets[i].section_name, section->name)
> +				    && !strcmp(default_configsets[i].name, element->name)) {
> +					has_config = true;
> +					break;
> +				}
> +			}
> +			if (!has_config)
> +				printf("%s.%s=%s\n", section->name,
> +				       element->name, element->value);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  static int collect_current_config(const char *var, const char *value,
>  				  void *spec_sections __maybe_unused)
>  {
> @@ -180,6 +504,9 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
>  	const char *system_config = perf_etc_perfconfig();
>  	char *user_config = mkpath("%s/.perfconfig", getenv("HOME"));
>  
> +	set_option_flag(config_options, 'l', "list", PARSE_OPT_EXCLUSIVE);
> +	set_option_flag(config_options, 'a', "list-all", PARSE_OPT_EXCLUSIVE);
> +
>  	argc = parse_options(argc, argv, config_options, config_usage,
>  			     PARSE_OPT_STOP_AT_NON_OPTION);
>  
> @@ -199,11 +526,19 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
>  		perf_config_from_file(collect_current_config, user_config, &sections);
>  
>  	switch (actions) {
> +	case ACTION_LIST_ALL:
> +		if (argc == 0) {
> +			ret = show_all_config(&sections);
> +			break;
> +		}
>  	case ACTION_LIST:
>  	default:
>  		if (argc) {
>  			pr_err("Error: takes no arguments\n");
> -			parse_options_usage(config_usage, config_options, "l", 1);
> +			if (actions == ACTION_LIST_ALL)
> +				parse_options_usage(config_usage, config_options, "a", 1);
> +			else
> +				parse_options_usage(config_usage, config_options, "l", 1);
>  			return -1;
>  		} else
>  			ret = show_config(&sections);
> -- 
> 1.9.1
>