Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1202188 > unrolled thread

Re: [PATCH v4 4/5] perf config: Add a option 'list-all' to perf-config

Started bytaeung <treeze.taeung@gmail.com>
First post2015-08-07 03:20 +0200
Last post2015-08-09 10:00 +0200
Articles 5 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v4 4/5] perf config: Add a option 'list-all' to perf-config taeung <treeze.taeung@gmail.com> - 2015-08-07 03:20 +0200
    Re: [PATCH v4 4/5] perf config: Add a option 'list-all' to perf-config Taewoong Song <treeze.taeung@gmail.com> - 2015-08-08 12:00 +0200
      Re: [PATCH v4 4/5] perf config: Add a option 'list-all' to  perf-config Namhyung Kim <namhyung@kernel.org> - 2015-08-09 05:20 +0200
    Re: [PATCH v4 4/5] perf config: Add a option 'list-all' to  perf-config Namhyung Kim <namhyung@kernel.org> - 2015-08-09 05:10 +0200
      Re: [PATCH v4 4/5] perf config: Add a option 'list-all' to perf-config Taewoong Song <treeze.taeung@gmail.com> - 2015-08-09 10:00 +0200

#1202188 — Re: [PATCH v4 4/5] perf config: Add a option 'list-all' to perf-config

Fromtaeung <treeze.taeung@gmail.com>
Date2015-08-07 03:20 +0200
SubjectRe: [PATCH v4 4/5] perf config: Add a option 'list-all' to perf-config
Message-ID<pUEue-Y2-11@gated-at.bofh.it>
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 ?

But I searched for other parse-option which can be replaced.
Is it better to use OPT_SET_INT instead of OPT_BIT ?

Thanks,
Taeung

>>   	OPT_END()
>>   };
>>   
>> @@ -539,6 +542,45 @@ static int collect_current_config(const char *var, const char *value,
>>   			   normalize_value(section_name, name, value));
>>   }
>>   
>> +static int show_all_config(void)
>> +{
>> +	int i;
>> +	bool has_config;
>> +	struct config_section *section_node;
>> +	struct config_element *element_node;
>> +
>> +	for (i = 0; default_configsets[i].section_name != NULL; i++) {
>> +		find_config(&section_node, &element_node,
>> +			    default_configsets[i].section_name, default_configsets[i].name);
>> +
>> +		if (!element_node)
>> +			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_node->name,
>> +			       element_node->name, element_node->value);
>> +	}
>> +
>> +	/* Print config variables the default configsets haven't */
>> +	list_for_each_entry(section_node, &sections, list) {
>> +		list_for_each_entry(element_node, &section_node->element_head, list) {
>> +			has_config = false;
>> +			for (i = 0; default_configsets[i].section_name != NULL; i++) {
>> +				if (!strcmp(default_configsets[i].section_name, section_node->name)
>> +				    && !strcmp(default_configsets[i].name, element_node->name)) {
>> +					has_config = true;
>> +					break;
>> +				}
>> +			}
>> +			if (!has_config)
>> +				printf("%s.%s=%s\n", section_node->name,
>> +				       element_node->name, element_node->value);
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>   static int perf_configset_with_option(configset_fn_t fn, const char *var, char *value)
>>   {
>>   	char *section_name;
>> @@ -617,6 +659,12 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
>>   		else
>>   			goto out_err;
>>   		goto out;
>> +	case ACTION_LIST_ALL:
>> +		if (argc == 0)
>> +			ret = show_all_config();
>> +		else
>> +			goto out_err;
>> +		goto out;
>>   	default:
>>   		if ((!has_option || use_global_config || use_system_config)
>>   		    && argc == 0) {
>> -- 
>> 1.9.1
>>

-- 
Thanks,
Taeung

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1203240

FromTaewoong Song <treeze.taeung@gmail.com>
Date2015-08-08 12:00 +0200
Message-ID<pV950-3zD-15@gated-at.bofh.it>
In reply to#1202188
Hi, Namhyung


> On Aug 7, 2015, at 10:12 AM, taeung <treeze.taeung@gmail.com> 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 ?
> 
> But I searched for other parse-option which can be replaced.
> Is it better to use OPT_SET_INT instead of OPT_BIT ?
> 

I modified source code to use OPT_SET_UINT
but the problem is happened. I declared "enum actions” to use OPT_SET_UINT like this.

enum actions {
    ACTION_LIST,
    ACTION_LIST_ALL,
    ACTION_REMOVE
} actions;

But default value of actions” variable is 0 and ACTION_LIST is also 0
so when ‘get’ or ’set’ feature is used, ‘default:' of the switch statement in cmd_config() can’t reached
because of default value of ‘actions’ variable is 0.

Use other parse-option function instead of OPT_SET_UINT ?


Thanks,
Taeung

> Thanks,
> Taeung
> 
>>>  	OPT_END()
>>>  };
>>>  @@ -539,6 +542,45 @@ static int collect_current_config(const char *var, const char *value,
>>>  			   normalize_value(section_name, name, value));
>>>  }
>>>  +static int show_all_config(void)
>>> +{
>>> +	int i;
>>> +	bool has_config;
>>> +	struct config_section *section_node;
>>> +	struct config_element *element_node;
>>> +
>>> +	for (i = 0; default_configsets[i].section_name != NULL; i++) {
>>> +		find_config(&section_node, &element_node,
>>> +			    default_configsets[i].section_name, default_configsets[i].name);
>>> +
>>> +		if (!element_node)
>>> +			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_node->name,
>>> +			       element_node->name, element_node->value);
>>> +	}
>>> +
>>> +	/* Print config variables the default configsets haven't */
>>> +	list_for_each_entry(section_node, &sections, list) {
>>> +		list_for_each_entry(element_node, &section_node->element_head, list) {
>>> +			has_config = false;
>>> +			for (i = 0; default_configsets[i].section_name != NULL; i++) {
>>> +				if (!strcmp(default_configsets[i].section_name, section_node->name)
>>> +				    && !strcmp(default_configsets[i].name, element_node->name)) {
>>> +					has_config = true;
>>> +					break;
>>> +				}
>>> +			}
>>> +			if (!has_config)
>>> +				printf("%s.%s=%s\n", section_node->name,
>>> +				       element_node->name, element_node->value);
>>> +		}
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> +
>>>  static int perf_configset_with_option(configset_fn_t fn, const char *var, char *value)
>>>  {
>>>  	char *section_name;
>>> @@ -617,6 +659,12 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
>>>  		else
>>>  			goto out_err;
>>>  		goto out;
>>> +	case ACTION_LIST_ALL:
>>> +		if (argc == 0)
>>> +			ret = show_all_config();
>>> +		else
>>> +			goto out_err;
>>> +		goto out;
>>>  	default:
>>>  		if ((!has_option || use_global_config || use_system_config)
>>>  		    && argc == 0) {
>>> -- 
>>> 1.9.1
>>> 
> 
> -- 
> Thanks,
> Taeung
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1203566 — Re: [PATCH v4 4/5] perf config: Add a option 'list-all' to perf-config

FromNamhyung Kim <namhyung@kernel.org>
Date2015-08-09 05:20 +0200
SubjectRe: [PATCH v4 4/5] perf config: Add a option 'list-all' to perf-config
Message-ID<pVpjr-1S7-1@gated-at.bofh.it>
In reply to#1203240
On Sat, Aug 08, 2015 at 06:50:59PM +0900, Taewoong Song wrote:
> >>>  	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 ?
> > 
> > But I searched for other parse-option which can be replaced.
> > Is it better to use OPT_SET_INT instead of OPT_BIT ?
> > 
> 
> I modified source code to use OPT_SET_UINT
> but the problem is happened. I declared "enum actions” to use OPT_SET_UINT like this.
> 
> enum actions {
>     ACTION_LIST,
>     ACTION_LIST_ALL,
>     ACTION_REMOVE
> } actions;
> 
> But default value of actions” variable is 0 and ACTION_LIST is also 0
> so when ‘get’ or ’set’ feature is used, ‘default:' of the switch statement in cmd_config() can’t reached
> because of default value of ‘actions’ variable is 0.

I don't know what's the problem.  You could either set default value
to -1 or make ACTION_LIST start from 1.

Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1203565 — Re: [PATCH v4 4/5] perf config: Add a option 'list-all' to perf-config

FromNamhyung Kim <namhyung@kernel.org>
Date2015-08-09 05:10 +0200
SubjectRe: [PATCH v4 4/5] perf config: Add a option 'list-all' to perf-config
Message-ID<pVp9L-1GQ-3@gated-at.bofh.it>
In reply to#1202188
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1203601

FromTaewoong Song <treeze.taeung@gmail.com>
Date2015-08-09 10:00 +0200
Message-ID<pVtGq-86D-3@gated-at.bofh.it>
In reply to#1203565
> On Aug 9, 2015, at 12:01 PM, Namhyung Kim <namhyung@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.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).
> 

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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web