* [PATCH] perf-script : improves option passing mechansim
@ 2014-03-18 16:09 Adrien BAK
2014-03-18 18:15 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 4+ messages in thread
From: Adrien BAK @ 2014-03-18 16:09 UTC (permalink / raw)
To: Peter Zijlstra, Paul Mackerras, Ingo Molnar,
Arnaldo Carvalho de Melo
Cc: linux-kernel, Mathias Gaunard
This pull request fixes the following issues :
* when passing custom arguments to a script, if those arguments are
not declared within perf, they prevent the execution of perf.
e.g.
perf script -i path/to/perf.data -s my_script -arg1 arg_value
perf will issue an error message and no processing will occur
* when passing custom arguments to a script, if those arguments are
declared by perf, they are consumed and not passed to the script.
e.g.
perf script -i path/to/perf.data -s my_script -h
perf will display its own help message, instead of the expected help
message from my_script.
These issues are addressed as follows :
* The parse option flag is changed from PARSE_OPT_STOP_AT_NON_OPTION to
PARSE_OPT_KEEP_UNKNOWN
* A new option type is introduce OPT_CALLBACK_FINAL_OPTION, which
effectively
prevents the parsing of all options located after the script.
Signed-off-by: Adrien Bak <adrien.bak@metascale.org>
---
tools/perf/builtin-script.c | 8 ++++----
tools/perf/util/parse-options.c | 4 ++++
tools/perf/util/parse-options.h | 5 +++++
3 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 9e9c91f..3cd6a46 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1523,9 +1523,9 @@ int cmd_script(int argc, const char **argv, const
char *prefix __maybe_unused)
"show latency attributes (irqs/preemption disabled, etc)"),
OPT_CALLBACK_NOOPT('l', "list", NULL, NULL, "list available scripts",
list_available_scripts),
- OPT_CALLBACK('s', "script", NULL, "name",
- "script file name (lang:script name, script name, or *)",
- parse_scriptname),
+ OPT_CALLBACK_FINAL_OPTION('s', "script", NULL, "name",
+ "script file name (lang:script name, script name, or *)",
+ parse_scriptname),
OPT_STRING('g', "gen-script", &generate_script_lang, "lang",
"generate perf-script.xx script in specified language"),
OPT_STRING('i', "input", &input_name, "file", "input file name"),
@@ -1578,7 +1578,7 @@ int cmd_script(int argc, const char **argv, const
char *prefix __maybe_unused)
setup_scripting();
argc = parse_options(argc, argv, options, script_usage,
- PARSE_OPT_STOP_AT_NON_OPTION);
+ PARSE_OPT_KEEP_UNKNOWN);
file.path = input_name;
diff --git a/tools/perf/util/parse-options.c
b/tools/perf/util/parse-options.c
index d22e3f8..77f566d 100644
--- a/tools/perf/util/parse-options.c
+++ b/tools/perf/util/parse-options.c
@@ -112,6 +112,8 @@ static int get_value(struct parse_opt_ctx_t *p,
return (*opt->callback)(opt, NULL, 0) ? (-1) : 0;
if (get_arg(p, opt, flags, &arg))
return -1;
+ if (opt->flags & PARSE_OPT_FINAL_OPTION)
+ return (*opt->callback)(opt, arg, 0)?(-1) : (-3);
return (*opt->callback)(opt, arg, 0) ? (-1) : 0;
case OPTION_INTEGER:
@@ -366,6 +368,8 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
return parse_options_usage(usagestr, options, arg + 1, 1);
case -2:
goto unknown;
+ case -3:
+ return PARSE_OPT_DONE;
default:
break;
}
diff --git a/tools/perf/util/parse-options.h
b/tools/perf/util/parse-options.h
index cbf0149..b1fa6b6 100644
--- a/tools/perf/util/parse-options.h
+++ b/tools/perf/util/parse-options.h
@@ -38,6 +38,7 @@ enum parse_opt_option_flags {
PARSE_OPT_NONEG = 4,
PARSE_OPT_HIDDEN = 8,
PARSE_OPT_LASTARG_DEFAULT = 16,
+ PARSE_OPT_FINAL_OPTION = 32,
};
struct option;
@@ -123,6 +124,10 @@ struct option {
{ .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l),
.value = (v), .argh = "time", .help = (h), .callback =
parse_opt_approxidate_cb }
#define OPT_CALLBACK(s, l, v, a, h, f) \
{ .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l),
.value = (v), (a), .help = (h), .callback = (f) }
+#define OPT_CALLBACK_FINAL_OPTION(s, l, v, a, h, f) \
+ { .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l),\
+ .value = (v), (a), .help = (h), .callback = (f),\
+ .flags = PARSE_OPT_FINAL_OPTION}
#define OPT_CALLBACK_NOOPT(s, l, v, a, h, f) \
{ .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l),
.value = (v), (a), .help = (h), .callback = (f), .flags = PARSE_OPT_NOARG }
#define OPT_CALLBACK_DEFAULT(s, l, v, a, h, f, d) \
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] perf-script : improves option passing mechansim
2014-03-18 16:09 [PATCH] perf-script : improves option passing mechansim Adrien BAK
@ 2014-03-18 18:15 ` Arnaldo Carvalho de Melo
2014-03-20 13:52 ` Adrien BAK
0 siblings, 1 reply; 4+ messages in thread
From: Arnaldo Carvalho de Melo @ 2014-03-18 18:15 UTC (permalink / raw)
To: Adrien BAK
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, linux-kernel,
Mathias Gaunard
Em Tue, Mar 18, 2014 at 05:09:33PM +0100, Adrien BAK escreveu:
> This pull request fixes the following issues :
> * when passing custom arguments to a script, if those arguments are
> not declared within perf, they prevent the execution of perf.
> e.g.
>
> perf script -i path/to/perf.data -s my_script -arg1 arg_value
>
> perf will issue an error message and no processing will occur
I haven't tested this, but what comes to mind is the use of -- to
separate what options should be processed by perf and which ones should
be left to the script, is that what you're fixing?
- Arnaldo
>
> * when passing custom arguments to a script, if those arguments are
> declared by perf, they are consumed and not passed to the script.
> e.g.
> perf script -i path/to/perf.data -s my_script -h
>
> perf will display its own help message, instead of the expected help
> message from my_script.
>
> These issues are addressed as follows :
> * The parse option flag is changed from PARSE_OPT_STOP_AT_NON_OPTION to
> PARSE_OPT_KEEP_UNKNOWN
> * A new option type is introduce OPT_CALLBACK_FINAL_OPTION, which
> effectively
> prevents the parsing of all options located after the script.
>
> Signed-off-by: Adrien Bak <adrien.bak@metascale.org>
>
> ---
>
> tools/perf/builtin-script.c | 8 ++++----
> tools/perf/util/parse-options.c | 4 ++++
> tools/perf/util/parse-options.h | 5 +++++
> 3 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 9e9c91f..3cd6a46 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -1523,9 +1523,9 @@ int cmd_script(int argc, const char **argv,
> const char *prefix __maybe_unused)
> "show latency attributes (irqs/preemption disabled, etc)"),
> OPT_CALLBACK_NOOPT('l', "list", NULL, NULL, "list available scripts",
> list_available_scripts),
> - OPT_CALLBACK('s', "script", NULL, "name",
> - "script file name (lang:script name, script name, or *)",
> - parse_scriptname),
> + OPT_CALLBACK_FINAL_OPTION('s', "script", NULL, "name",
> + "script file name (lang:script name, script name, or *)",
> + parse_scriptname),
> OPT_STRING('g', "gen-script", &generate_script_lang, "lang",
> "generate perf-script.xx script in specified language"),
> OPT_STRING('i', "input", &input_name, "file", "input file name"),
> @@ -1578,7 +1578,7 @@ int cmd_script(int argc, const char **argv,
> const char *prefix __maybe_unused)
> setup_scripting();
>
> argc = parse_options(argc, argv, options, script_usage,
> - PARSE_OPT_STOP_AT_NON_OPTION);
> + PARSE_OPT_KEEP_UNKNOWN);
>
> file.path = input_name;
>
> diff --git a/tools/perf/util/parse-options.c
> b/tools/perf/util/parse-options.c
> index d22e3f8..77f566d 100644
> --- a/tools/perf/util/parse-options.c
> +++ b/tools/perf/util/parse-options.c
> @@ -112,6 +112,8 @@ static int get_value(struct parse_opt_ctx_t *p,
> return (*opt->callback)(opt, NULL, 0) ? (-1) : 0;
> if (get_arg(p, opt, flags, &arg))
> return -1;
> + if (opt->flags & PARSE_OPT_FINAL_OPTION)
> + return (*opt->callback)(opt, arg, 0)?(-1) : (-3);
> return (*opt->callback)(opt, arg, 0) ? (-1) : 0;
>
> case OPTION_INTEGER:
> @@ -366,6 +368,8 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
> return parse_options_usage(usagestr, options, arg + 1, 1);
> case -2:
> goto unknown;
> + case -3:
> + return PARSE_OPT_DONE;
> default:
> break;
> }
> diff --git a/tools/perf/util/parse-options.h
> b/tools/perf/util/parse-options.h
> index cbf0149..b1fa6b6 100644
> --- a/tools/perf/util/parse-options.h
> +++ b/tools/perf/util/parse-options.h
> @@ -38,6 +38,7 @@ enum parse_opt_option_flags {
> PARSE_OPT_NONEG = 4,
> PARSE_OPT_HIDDEN = 8,
> PARSE_OPT_LASTARG_DEFAULT = 16,
> + PARSE_OPT_FINAL_OPTION = 32,
> };
>
> struct option;
> @@ -123,6 +124,10 @@ struct option {
> { .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l),
> .value = (v), .argh = "time", .help = (h), .callback =
> parse_opt_approxidate_cb }
> #define OPT_CALLBACK(s, l, v, a, h, f) \
> { .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l),
> .value = (v), (a), .help = (h), .callback = (f) }
> +#define OPT_CALLBACK_FINAL_OPTION(s, l, v, a, h, f) \
> + { .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l),\
> + .value = (v), (a), .help = (h), .callback = (f),\
> + .flags = PARSE_OPT_FINAL_OPTION}
> #define OPT_CALLBACK_NOOPT(s, l, v, a, h, f) \
> { .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l),
> .value = (v), (a), .help = (h), .callback = (f), .flags =
> PARSE_OPT_NOARG }
> #define OPT_CALLBACK_DEFAULT(s, l, v, a, h, f, d) \
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] perf-script : improves option passing mechansim
2014-03-18 18:15 ` Arnaldo Carvalho de Melo
@ 2014-03-20 13:52 ` Adrien BAK
2014-03-20 19:32 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 4+ messages in thread
From: Adrien BAK @ 2014-03-20 13:52 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, linux-kernel,
Mathias Gaunard
Indeed. I wasn't aware of the '--' behaviour and the documentation
doesn't seem to mention it. My patch essentially implements the same
behaviour, except that the script name itself is used to separate the
options. I guess there is not much need the proposed patch then.
Sorry for the wasted time.
Adrien
On 03/18/2014 07:15 PM, Arnaldo Carvalho de Melo wrote:
> Em Tue, Mar 18, 2014 at 05:09:33PM +0100, Adrien BAK escreveu:
>> This pull request fixes the following issues :
>> * when passing custom arguments to a script, if those arguments are
>> not declared within perf, they prevent the execution of perf.
>> e.g.
>>
>> perf script -i path/to/perf.data -s my_script -arg1 arg_value
>>
>> perf will issue an error message and no processing will occur
> I haven't tested this, but what comes to mind is the use of -- to
> separate what options should be processed by perf and which ones should
> be left to the script, is that what you're fixing?
>
> - Arnaldo
>
>> * when passing custom arguments to a script, if those arguments are
>> declared by perf, they are consumed and not passed to the script.
>> e.g.
>> perf script -i path/to/perf.data -s my_script -h
>>
>> perf will display its own help message, instead of the expected help
>> message from my_script.
>>
>> These issues are addressed as follows :
>> * The parse option flag is changed from PARSE_OPT_STOP_AT_NON_OPTION to
>> PARSE_OPT_KEEP_UNKNOWN
>> * A new option type is introduce OPT_CALLBACK_FINAL_OPTION, which
>> effectively
>> prevents the parsing of all options located after the script.
>>
>> Signed-off-by: Adrien Bak <adrien.bak@metascale.org>
>>
>> ---
>>
>> tools/perf/builtin-script.c | 8 ++++----
>> tools/perf/util/parse-options.c | 4 ++++
>> tools/perf/util/parse-options.h | 5 +++++
>> 3 files changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
>> index 9e9c91f..3cd6a46 100644
>> --- a/tools/perf/builtin-script.c
>> +++ b/tools/perf/builtin-script.c
>> @@ -1523,9 +1523,9 @@ int cmd_script(int argc, const char **argv,
>> const char *prefix __maybe_unused)
>> "show latency attributes (irqs/preemption disabled, etc)"),
>> OPT_CALLBACK_NOOPT('l', "list", NULL, NULL, "list available scripts",
>> list_available_scripts),
>> - OPT_CALLBACK('s', "script", NULL, "name",
>> - "script file name (lang:script name, script name, or *)",
>> - parse_scriptname),
>> + OPT_CALLBACK_FINAL_OPTION('s', "script", NULL, "name",
>> + "script file name (lang:script name, script name, or *)",
>> + parse_scriptname),
>> OPT_STRING('g', "gen-script", &generate_script_lang, "lang",
>> "generate perf-script.xx script in specified language"),
>> OPT_STRING('i', "input", &input_name, "file", "input file name"),
>> @@ -1578,7 +1578,7 @@ int cmd_script(int argc, const char **argv,
>> const char *prefix __maybe_unused)
>> setup_scripting();
>>
>> argc = parse_options(argc, argv, options, script_usage,
>> - PARSE_OPT_STOP_AT_NON_OPTION);
>> + PARSE_OPT_KEEP_UNKNOWN);
>>
>> file.path = input_name;
>>
>> diff --git a/tools/perf/util/parse-options.c
>> b/tools/perf/util/parse-options.c
>> index d22e3f8..77f566d 100644
>> --- a/tools/perf/util/parse-options.c
>> +++ b/tools/perf/util/parse-options.c
>> @@ -112,6 +112,8 @@ static int get_value(struct parse_opt_ctx_t *p,
>> return (*opt->callback)(opt, NULL, 0) ? (-1) : 0;
>> if (get_arg(p, opt, flags, &arg))
>> return -1;
>> + if (opt->flags & PARSE_OPT_FINAL_OPTION)
>> + return (*opt->callback)(opt, arg, 0)?(-1) : (-3);
>> return (*opt->callback)(opt, arg, 0) ? (-1) : 0;
>>
>> case OPTION_INTEGER:
>> @@ -366,6 +368,8 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
>> return parse_options_usage(usagestr, options, arg + 1, 1);
>> case -2:
>> goto unknown;
>> + case -3:
>> + return PARSE_OPT_DONE;
>> default:
>> break;
>> }
>> diff --git a/tools/perf/util/parse-options.h
>> b/tools/perf/util/parse-options.h
>> index cbf0149..b1fa6b6 100644
>> --- a/tools/perf/util/parse-options.h
>> +++ b/tools/perf/util/parse-options.h
>> @@ -38,6 +38,7 @@ enum parse_opt_option_flags {
>> PARSE_OPT_NONEG = 4,
>> PARSE_OPT_HIDDEN = 8,
>> PARSE_OPT_LASTARG_DEFAULT = 16,
>> + PARSE_OPT_FINAL_OPTION = 32,
>> };
>>
>> struct option;
>> @@ -123,6 +124,10 @@ struct option {
>> { .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l),
>> .value = (v), .argh = "time", .help = (h), .callback =
>> parse_opt_approxidate_cb }
>> #define OPT_CALLBACK(s, l, v, a, h, f) \
>> { .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l),
>> .value = (v), (a), .help = (h), .callback = (f) }
>> +#define OPT_CALLBACK_FINAL_OPTION(s, l, v, a, h, f) \
>> + { .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l),\
>> + .value = (v), (a), .help = (h), .callback = (f),\
>> + .flags = PARSE_OPT_FINAL_OPTION}
>> #define OPT_CALLBACK_NOOPT(s, l, v, a, h, f) \
>> { .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l),
>> .value = (v), (a), .help = (h), .callback = (f), .flags =
>> PARSE_OPT_NOARG }
>> #define OPT_CALLBACK_DEFAULT(s, l, v, a, h, f, d) \
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] perf-script : improves option passing mechansim
2014-03-20 13:52 ` Adrien BAK
@ 2014-03-20 19:32 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 4+ messages in thread
From: Arnaldo Carvalho de Melo @ 2014-03-20 19:32 UTC (permalink / raw)
To: Adrien BAK
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, linux-kernel,
Mathias Gaunard
Em Thu, Mar 20, 2014 at 02:52:30PM +0100, Adrien BAK escreveu:
> Indeed. I wasn't aware of the '--' behaviour and the documentation
> doesn't seem to mention it. My patch essentially implements the same
> behaviour, except that the script name itself is used to separate
> the options. I guess there is not much need the proposed patch then.
>
> Sorry for the wasted time.
No need to be sorry, I appreciate _you_ taking the time to help improve
the tools, maybe you can send a patch to update the documentation
instead, with examples where -- is used so that people don't fall into
this trap again?
Thanks!
- Arnaldo
> Adrien
>
> On 03/18/2014 07:15 PM, Arnaldo Carvalho de Melo wrote:
> >Em Tue, Mar 18, 2014 at 05:09:33PM +0100, Adrien BAK escreveu:
> >>This pull request fixes the following issues :
> >>* when passing custom arguments to a script, if those arguments are
> >> not declared within perf, they prevent the execution of perf.
> >>e.g.
> >>
> >>perf script -i path/to/perf.data -s my_script -arg1 arg_value
> >>
> >>perf will issue an error message and no processing will occur
> >I haven't tested this, but what comes to mind is the use of -- to
> >separate what options should be processed by perf and which ones should
> >be left to the script, is that what you're fixing?
> >
> >- Arnaldo
> >>* when passing custom arguments to a script, if those arguments are
> >> declared by perf, they are consumed and not passed to the script.
> >>e.g.
> >>perf script -i path/to/perf.data -s my_script -h
> >>
> >>perf will display its own help message, instead of the expected help
> >>message from my_script.
> >>
> >>These issues are addressed as follows :
> >>* The parse option flag is changed from PARSE_OPT_STOP_AT_NON_OPTION to
> >>PARSE_OPT_KEEP_UNKNOWN
> >>* A new option type is introduce OPT_CALLBACK_FINAL_OPTION, which
> >>effectively
> >> prevents the parsing of all options located after the script.
> >>
> >>Signed-off-by: Adrien Bak <adrien.bak@metascale.org>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-03-20 19:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-18 16:09 [PATCH] perf-script : improves option passing mechansim Adrien BAK
2014-03-18 18:15 ` Arnaldo Carvalho de Melo
2014-03-20 13:52 ` Adrien BAK
2014-03-20 19:32 ` Arnaldo Carvalho de Melo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.