Messages in this thread | | | Date | Sun, 9 Aug 2015 12:01:35 +0900 | From | Namhyung Kim <> | Subject | Re: [PATCH v4 4/5] perf config: Add a option 'list-all' to perf-config |
| |
On Fri, Aug 07, 2015 at 10:12:02AM +0900, taeung wrote: > Hi, Namhyung > > On 07/27/2015 05:48 PM, Namhyung Kim wrote: > >On Mon, Jul 27, 2015 at 12:58:30AM +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 [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 | 48 ++++++++++++++++++++++++++++++++ > >> 2 files changed, 54 insertions(+) > >> > >>diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt > >>index cd4b1a6..d8b3acc 100644 > >>--- a/tools/perf/Documentation/perf-config.txt > >>+++ b/tools/perf/Documentation/perf-config.txt > >>@@ -11,6 +11,8 @@ SYNOPSIS > >> 'perf config' [<file-option>] [section.name[=value] ...] > >> or > >> 'perf config' [<file-option>] -l | --list > >>+or > >>+'perf config' [<file-option>] -a | --list-all > >> DESCRIPTION > >> ----------- > >>@@ -31,6 +33,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 6d9f28c..f4a1569 100644 > >>--- a/tools/perf/builtin-config.c > >>+++ b/tools/perf/builtin-config.c > >>@@ -23,6 +23,7 @@ static const char * const config_usage[] = { > >> }; > >> #define ACTION_LIST (1<<0) > >>+#define ACTION_LIST_ALL (1<<1) > >> static const struct option config_options[] = { > >> OPT_GROUP("Config file location"), > >>@@ -31,6 +32,8 @@ static const struct option config_options[] = { > >> OPT_GROUP("Action"), > >> OPT_BIT('l', "list", &actions, > >> "show current config variables", ACTION_LIST), > >>+ OPT_BIT('a', "list-all", &actions, > >>+ "show current and all possible config variables with default values", ACTION_LIST_ALL), > >Why did you use OPT_BIT? Do you want to support multiple 'actions' at > >the same time? I'd rather support just one action, but I won't insist > >it strongly.. Anyway, setting bits will confuse the switch statement > >in the cmd_config(). > > > >Thanks, > >Namhyung > > > I don't understand why setting bits will confuse the switch statement. > Is the reason about readability of source code ?
Supposed you set ADD as 1 and DEL as 2. If you want do both action, it'll have value of 3. But switch statement only have case 1 or 2 (unless you give all possible combinations - but I don't think we want it).
> > But I searched for other parse-option which can be replaced. > Is it better to use OPT_SET_INT instead of OPT_BIT ?
Please just use OPT_INTEGER.
Thanks, Namhyung
| |