2023-10-31 08:10:51

by Yang Jihong

Subject: [PATCH] perf debug: List available options when no variable is specified

Minor help message improvement for `perf --debug`

Before:

# perf --debug
No variable specified for --debug.

Usage: perf [--version] [--help] [OPTIONS] COMMAND [ARGS]

After:

# perf --debug
No variable specified for --debug, available options: verbose,ordered-events,stderr,data-convert,perf-event-open.

Signed-off-by: Yang Jihong <[email protected]>
---
tools/perf/perf.c | 4 +++-
tools/perf/util/debug.c | 3 +++
tools/perf/util/debug.h | 1 +
3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index d3fc8090413c..0fc4719e6825 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -279,7 +279,9 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
exit(0);
} else if (!strcmp(cmd, "--debug")) {
if (*argc < 2) {
- fprintf(stderr, "No variable specified for --debug.\n");
+ fprintf(stderr,
+ "No variable specified for --debug, available options: %s.\n",
+ perf_debug_options_string);
usage(perf_usage_string);
}
if (perf_debug_option((*argv)[1]))
diff --git a/tools/perf/util/debug.c b/tools/perf/util/debug.c
index 88378c4c5dd9..e02f6d97b826 100644
--- a/tools/perf/util/debug.c
+++ b/tools/perf/util/debug.c
@@ -215,6 +215,9 @@ void trace_event(union perf_event *event)
trace_event_printer, event);
}

+const char perf_debug_options_string[] =
+ "verbose,ordered-events,stderr,data-convert,perf-event-open";
+
static struct sublevel_option debug_opts[] = {
{ .name = "verbose", .value_ptr = &verbose },
{ .name = "ordered-events", .value_ptr = &debug_ordered_events},
diff --git a/tools/perf/util/debug.h b/tools/perf/util/debug.h
index f99468a7f681..b9b4d17a1845 100644
--- a/tools/perf/util/debug.h
+++ b/tools/perf/util/debug.h
@@ -13,6 +13,7 @@ extern int debug_peo_args;
extern bool quiet, dump_trace;
extern int debug_ordered_events;
extern int debug_data_convert;
+extern const char perf_debug_options_string[];

#ifndef pr_fmt
#define pr_fmt(fmt) fmt
--
2.34.1

2023-10-31 15:42:31

by Arnaldo Carvalho de Melo

Subject: Re: [PATCH] perf debug: List available options when no variable is specified

Em Tue, Oct 31, 2023 at 08:08:23AM +0000, Yang Jihong escreveu:
> Before:

> # perf --debug
> No variable specified for --debug.

> After:

> # perf --debug
> No variable specified for --debug, available options: verbose,ordered-events,stderr,data-convert,perf-event-open.

Looks useful, but the implementation can be different to reduce
maintainership costs, see below:

> +++ b/tools/perf/perf.c
> @@ -279,7 +279,9 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
> } else if (!strcmp(cmd, "--debug")) {
> if (*argc < 2) {
> - fprintf(stderr, "No variable specified for --debug.\n");
> + fprintf(stderr,
> + "No variable specified for --debug, available options: %s.\n",
> + perf_debug_options_string);
> usage(perf_usage_string);

> +++ b/tools/perf/util/debug.c
> @@ -215,6 +215,9 @@ void trace_event(union perf_event *event)
> trace_event_printer, event);
> }
>
> +const char perf_debug_options_string[] =
> + "verbose,ordered-events,stderr,data-convert,perf-event-open";

Instead of adding a new variable that has to be kept in sync with
debug_opts[], you could provide a function that iterates debug_opts,
printing its options names, then use that function on perf.c handle_options.

- Arnaldo

> static struct sublevel_option debug_opts[] = {
> { .name = "verbose", .value_ptr = &verbose },
> { .name = "ordered-events", .value_ptr = &debug_ordered_events},

2023-11-01 06:36:55

by Yang Jihong

Subject: Re: [PATCH] perf debug: List available options when no variable is specified

Hello,

On 2023/10/31 23:42, Arnaldo Carvalho de Melo wrote:
> Em Tue, Oct 31, 2023 at 08:08:23AM +0000, Yang Jihong escreveu:
>> Before:
>
>> # perf --debug
>> No variable specified for --debug.
>
>> After:
>
>> # perf --debug
>> No variable specified for --debug, available options: verbose,ordered-events,stderr,data-convert,perf-event-open.
>
> Looks useful, but the implementation can be different to reduce
> maintainership costs, see below:
>
>> +++ b/tools/perf/perf.c
>> @@ -279,7 +279,9 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
>> } else if (!strcmp(cmd, "--debug")) {
>> if (*argc < 2) {
>> - fprintf(stderr, "No variable specified for --debug.\n");
>> + fprintf(stderr,
>> + "No variable specified for --debug, available options: %s.\n",
>> + perf_debug_options_string);
>> usage(perf_usage_string);
>
>> +++ b/tools/perf/util/debug.c
>> @@ -215,6 +215,9 @@ void trace_event(union perf_event *event)
>> trace_event_printer, event);
>> }
>>
>> +const char perf_debug_options_string[] =
>> + "verbose,ordered-events,stderr,data-convert,perf-event-open";
>
> Instead of adding a new variable that has to be kept in sync with
> debug_opts[], you could provide a function that iterates debug_opts,
> printing its options names, then use that function on perf.c handle_options.

Thanks for the advice.
OK, will send a v2 patch according to this scheme.

Thanks,
Yang