lkml.org 
[lkml]   [2015]   [Aug]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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


\
 
 \ /
  Last update: 2015-08-09 05:21    [W:0.067 / U:0.740 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site