> On Aug 9, 2015, at 12:01 PM, Namhyung Kim <namhy...@kernel.org> wrote:
>
> 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.tae...@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).
>
I understood what you said. Combinations of actions isn’t needed.
So we don’t need to use OPT_BIT in perf-config.
But I thought that if more than two separate or same actions is used
error messages should be printed like “error: only one action at a time” like
this.
# perf config -l -l
error: only one action at a time
# perf config -l -a -r test.test
error: only one action at a time
Then using more than two actions should be blocked.
I thought about 2 solutions
1) comparing original ‘argc' and ‘argc' after parse_options() work like this.
int origin_argc = arc -1;
argc = parse_options(argc, argv, config_options, config_usage,
PARSE_OPT_STOP_AT_NON_OPTION);
if (origin_argc != argc -1)
pr_err(“error: only one action at a time\n");
2) using OPT_BIT and HAS_MULTI_BIT()
HAS_MULTI_BIT() can check whether more than two actions is used or not.
Is needed this exception handing ?
Or it isn’t needed ?
>
>>
>> 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.
I modified source code to use OPT_INTEGER.
But OPT_INTEGER require entering integer value like this.
# perf config —list=1
or
# perf config -l 1
So, I just use OPT_SET_UINT because it can have default value
in contrast with OPT_INTEGER like this.
static const struct option config_options[] = {
OPT_GROUP(“Action”),
OPT_SET_UINT(‘l’, “list”, &actions,
“show current config variables”,
ACTION_LIST),
And I declared enum variable to distinguish between default value 0
and value of ACTION_LIST like this.
enum actions {
ACTION_LIST = 1,
ACTION_LIST_ALL,
ACTION_REMOVE
} actions;
Aren’t there problems if using OPT_SET_UINT instead of OPT_INTEGER ?
Thanks,
Taeung
>
> Thanks,
> Namhyung
--
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/
