Hi Taeung, On Sun, Apr 12, 2015 at 11:44:48PM +0900, Taeung Song wrote: > The perf configuration file contains many variables which can make > the perf command's action more effective and more skilful. > But looking through state of configuration is difficult and > there's no knowing what kind of other variables except variables in > perfconfig.example exist. > So This patch adds 'perf-config' command with '--all' option and a document > for it. > > Signed-off-by: Taeung Song <treeze.tae...@gmail.com> > ---
Thanks for your work! I think the documentation updates need to be shared with tools manpages, or at least they can point to this document for details. This can be further work though. ;-) [SNIP] > diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c > new file mode 100644 > index 0000000..7cf6d03 > --- /dev/null > +++ b/tools/perf/builtin-config.c > @@ -0,0 +1,71 @@ > +/* > + * builtin-config.c > + * > + * Copyright (C) 2015, Taeung Song <treeze.tae...@gmail.com> > + * > + */ > +#include "builtin.h" > + > +#include "perf.h" > + > +#include "util/cache.h" > +#include "util/parse-options.h" > +#include "util/util.h" > +#include "util/debug.h" > + > +static struct { > + bool all_action; > +} params; > + > +static const char * const config_usage[] = { > + "perf config [options]", > + NULL > +}; > +static const struct option config_options[] = { > + OPT_GROUP("Action"), > + OPT_BOOLEAN('a', "all", ¶ms.all_action, "print all configurations"), > + OPT_END() > +}; > + > +static void check_argc(int argc, int limit) > +{ > + if (argc >= limit && argc <= limit) > + return; > + error("wrong number of arguments"); > + usage_with_options(config_usage, config_options); > +} I don't know why this is needed. The -a option won't need this and we can support to get/set any number of config items IMHO. > + > +static int show_config(const char *key, const char *value, > + void *cb __maybe_unused) > +{ > + if (value) > + printf("%s=%s\n", key, value); > + else > + printf("%s\n", key); > + > + return 0; > +} > + > +int cmd_config(int argc, const char **argv, const char *prefix > __maybe_unused) > +{ > + int ret = 0; > + > + argc = parse_options(argc, argv, config_options, config_usage, > + PARSE_OPT_STOP_AT_NON_OPTION); > + if (argc > 0) { > + if (strcmp(argv[0], "-") == 0) { > + pr_warning(" Error: '-' is not supported.\n"); I don't understand this. Why is the "-" so special? And I think current behavior don't allow any argument. So what about this? argc = parse_options(...); if (argc) usage_with_options(...); if (params.all_action) ... > + usage_with_options(config_usage, config_options); > + } > + } > + > + if (argc == 0 || params.all_action) { > + check_argc(argc, 0); > + ret = perf_config(show_config, NULL); But doesn't it just print currently set items in a config file? I guess -a/--all option should show this *AND* all possible config items with default values. Current behavior can be done by -l/--list option like git-config.. Thanks, Namhyung > + } else { > + pr_warning("Error: Unknown argument.\n"); > + usage_with_options(config_usage, config_options); > + } > + > + return ret; > +} > diff --git a/tools/perf/builtin.h b/tools/perf/builtin.h > index 3688ad2..3f871b5 100644 > --- a/tools/perf/builtin.h > +++ b/tools/perf/builtin.h > @@ -17,6 +17,7 @@ extern int cmd_annotate(int argc, const char **argv, const > char *prefix); > extern int cmd_bench(int argc, const char **argv, const char *prefix); > extern int cmd_buildid_cache(int argc, const char **argv, const char > *prefix); > extern int cmd_buildid_list(int argc, const char **argv, const char *prefix); > +extern int cmd_config(int argc, const char **argv, const char *prefix); > extern int cmd_diff(int argc, const char **argv, const char *prefix); > extern int cmd_evlist(int argc, const char **argv, const char *prefix); > extern int cmd_help(int argc, const char **argv, const char *prefix); > diff --git a/tools/perf/command-list.txt b/tools/perf/command-list.txt > index 00fcaf8..acc3ea7 100644 > --- a/tools/perf/command-list.txt > +++ b/tools/perf/command-list.txt > @@ -9,6 +9,7 @@ perf-buildid-cache mainporcelain common > perf-buildid-list mainporcelain common > perf-data mainporcelain common > perf-diff mainporcelain common > +perf-config mainporcelain common > perf-evlist mainporcelain common > perf-inject mainporcelain common > perf-kmem mainporcelain common > diff --git a/tools/perf/perf.c b/tools/perf/perf.c > index b857fcb..604fa4a 100644 > --- a/tools/perf/perf.c > +++ b/tools/perf/perf.c > @@ -37,6 +37,7 @@ struct cmd_struct { > static struct cmd_struct commands[] = { > { "buildid-cache", cmd_buildid_cache, 0 }, > { "buildid-list", cmd_buildid_list, 0 }, > + { "config", cmd_config, 0 }, > { "diff", cmd_diff, 0 }, > { "evlist", cmd_evlist, 0 }, > { "help", cmd_help, 0 }, > -- > 1.9.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/