Re: [PATCH v2 1/2] perf: Suggest inbuilt commands for unknown command

From: Arnaldo Carvalho de Melo
Date: Wed Mar 20 2024 - 10:54:24 EST

Next message: Kuan-Wei Chiu: "[PATCH v2 00/15] treewide: Refactor heap related implementation"
Previous message: Daniel Bristot de Oliveira: "[GIT PULL V2] tracing/tools: Updates for 6.9"
Next in thread: Arnaldo Carvalho de Melo: "Re: [PATCH v2 1/2] perf: Suggest inbuilt commands for unknown command"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Fri, Mar 01, 2024 at 12:13:05PM -0800, Ian Rogers wrote:
> The existing unknown command code looks for perf scripts like
> perf-archive.sh and perf-iostat.sh, however, inbuilt commands aren't
> suggested. Add the inbuilt commands so they may be suggested too.
>
> Before:
> ```
> $ perf reccord
> perf: 'reccord' is not a perf-command. See 'perf --help'.
> ```
>
> After:
> ```
> $ perf reccord
> perf: 'reccord' is not a perf-command. See 'perf --help'.
>
> Did you mean this?
> record
> ```
>
> Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>
> ---
> v2. Drops a merged patch and rebases. No functional change. Arnaldo
> reported the patch not working for him, but I've not found a

Not working:

root@number:~# perf reccord
Failed to run command 'reccord': No such file or directory
root@number:~#

⬢[acme@toolbox perf-tools-next]$ git log --oneline -1
a65ef8052854ba75 (HEAD) perf: Suggest inbuilt commands for unknown command
⬢[acme@toolbox perf-tools-next]$

I use O= with install-bin, trying:

⬢[acme@toolbox perf-tools-next]$ make -C tools/perf install-bin
⬢[acme@toolbox perf-tools-next]$ perf raccord
Failed to run command 'raccord': No such file or directory
⬢[acme@toolbox perf-tools-next]$

Also didn't work

Trying to figure this out...

- Arnaldo

> reproduction and it works for me:
> https://lore.kernel.org/lkml/ZZcdDyyADG8dP8LM@xxxxxxxxxx/
> ---
> tools/perf/builtin.h | 4 ++-
> tools/perf/perf.c | 21 +++++++++++---
> tools/perf/util/help-unknown-cmd.c | 45 ++++++++++++++----------------
> 3 files changed, 41 insertions(+), 29 deletions(-)
>
> diff --git a/tools/perf/builtin.h b/tools/perf/builtin.h
> index f2ab5bae2150..f4375deabfa3 100644
> --- a/tools/perf/builtin.h
> +++ b/tools/perf/builtin.h
> @@ -2,8 +2,10 @@
> #ifndef BUILTIN_H
> #define BUILTIN_H
>
> +struct cmdnames;
> +
> void list_common_cmds_help(void);
> -const char *help_unknown_cmd(const char *cmd);
> +const char *help_unknown_cmd(const char *cmd, struct cmdnames *main_cmds);
>
> int cmd_annotate(int argc, const char **argv);
> int cmd_bench(int argc, const char **argv);
> diff --git a/tools/perf/perf.c b/tools/perf/perf.c
> index 921bee0a6437..c719e6ccd9e2 100644
> --- a/tools/perf/perf.c
> +++ b/tools/perf/perf.c
> @@ -18,6 +18,7 @@
> #include <subcmd/run-command.h>
> #include "util/parse-events.h"
> #include <subcmd/parse-options.h>
> +#include <subcmd/help.h>
> #include "util/debug.h"
> #include "util/event.h"
> #include "util/util.h" // usage()
> @@ -557,7 +558,7 @@ int main(int argc, const char **argv)
> pthread__block_sigwinch();
>
> while (1) {
> - static int done_help;
> + int done_help;
>
> run_argv(&argc, &argv);
>
> @@ -565,14 +566,26 @@ int main(int argc, const char **argv)
> break;
>
> if (!done_help) {
> - cmd = argv[0] = help_unknown_cmd(cmd);
> + struct cmdnames main_cmds;
> +
> + for (unsigned int i = 0; i < ARRAY_SIZE(commands); i++) {
> + add_cmdname(&main_cmds,
> + commands[i].cmd,
> + strlen(commands[i].cmd));
> + }
> + cmd = argv[0] = help_unknown_cmd(cmd, &main_cmds);
> + clean_cmdnames(&main_cmds);
> done_help = 1;
> + if (!cmd)
> + break;
> } else
> break;
> }
>
> - fprintf(stderr, "Failed to run command '%s': %s\n",
> - cmd, str_error_r(errno, sbuf, sizeof(sbuf)));
> + if (cmd) {
> + fprintf(stderr, "Failed to run command '%s': %s\n",
> + cmd, str_error_r(errno, sbuf, sizeof(sbuf)));
> + }
> out:
> if (debug_fp)
> fclose(debug_fp);
> diff --git a/tools/perf/util/help-unknown-cmd.c b/tools/perf/util/help-unknown-cmd.c
> index eab99ea6ac01..2ba3369f1620 100644
> --- a/tools/perf/util/help-unknown-cmd.c
> +++ b/tools/perf/util/help-unknown-cmd.c
> @@ -52,46 +52,44 @@ static int add_cmd_list(struct cmdnames *cmds, struct cmdnames *old)
> return 0;
> }
>
> -const char *help_unknown_cmd(const char *cmd)
> +const char *help_unknown_cmd(const char *cmd, struct cmdnames *main_cmds)
> {
> unsigned int i, n = 0, best_similarity = 0;
> - struct cmdnames main_cmds, other_cmds;
> + struct cmdnames other_cmds;
>
> - memset(&main_cmds, 0, sizeof(main_cmds));
> - memset(&other_cmds, 0, sizeof(main_cmds));
> + memset(&other_cmds, 0, sizeof(other_cmds));
>
> perf_config(perf_unknown_cmd_config, NULL);
>
> - load_command_list("perf-", &main_cmds, &other_cmds);
> + load_command_list("perf-", main_cmds, &other_cmds);
>
> - if (add_cmd_list(&main_cmds, &other_cmds) < 0) {
> + if (add_cmd_list(main_cmds, &other_cmds) < 0) {
> fprintf(stderr, "ERROR: Failed to allocate command list for unknown command.\n");
> goto end;
> }
> - qsort(main_cmds.names, main_cmds.cnt,
> - sizeof(main_cmds.names), cmdname_compare);
> - uniq(&main_cmds);
> + qsort(main_cmds->names, main_cmds->cnt,
> + sizeof(main_cmds->names), cmdname_compare);
> + uniq(main_cmds);
>
> - if (main_cmds.cnt) {
> + if (main_cmds->cnt) {
> /* This reuses cmdname->len for similarity index */
> - for (i = 0; i < main_cmds.cnt; ++i)
> - main_cmds.names[i]->len =
> - levenshtein(cmd, main_cmds.names[i]->name, 0, 2, 1, 4);
> + for (i = 0; i < main_cmds->cnt; ++i)
> + main_cmds->names[i]->len =
> + levenshtein(cmd, main_cmds->names[i]->name, 0, 2, 1, 4);
>
> - qsort(main_cmds.names, main_cmds.cnt,
> - sizeof(*main_cmds.names), levenshtein_compare);
> + qsort(main_cmds->names, main_cmds->cnt,
> + sizeof(*main_cmds->names), levenshtein_compare);
>
> - best_similarity = main_cmds.names[0]->len;
> + best_similarity = main_cmds->names[0]->len;
> n = 1;
> - while (n < main_cmds.cnt && best_similarity == main_cmds.names[n]->len)
> + while (n < main_cmds->cnt && best_similarity == main_cmds->names[n]->len)
> ++n;
> }
>
> if (autocorrect && n == 1) {
> - const char *assumed = main_cmds.names[0]->name;
> + const char *assumed = main_cmds->names[0]->name;
>
> - main_cmds.names[0] = NULL;
> - clean_cmdnames(&main_cmds);
> + main_cmds->names[0] = NULL;
> clean_cmdnames(&other_cmds);
> fprintf(stderr, "WARNING: You called a perf program named '%s', "
> "which does not exist.\n"
> @@ -107,15 +105,14 @@ const char *help_unknown_cmd(const char *cmd)
>
> fprintf(stderr, "perf: '%s' is not a perf-command. See 'perf --help'.\n", cmd);
>
> - if (main_cmds.cnt && best_similarity < 6) {
> + if (main_cmds->cnt && best_similarity < 6) {
> fprintf(stderr, "\nDid you mean %s?\n",
> n < 2 ? "this": "one of these");
>
> for (i = 0; i < n; i++)
> - fprintf(stderr, "\t%s\n", main_cmds.names[i]->name);
> + fprintf(stderr, "\t%s\n", main_cmds->names[i]->name);
> }
> end:
> - clean_cmdnames(&main_cmds);
> clean_cmdnames(&other_cmds);
> - exit(1);
> + return NULL;
> }
> --
> 2.44.0.278.ge034bb2e1d-goog
>

Next message: Kuan-Wei Chiu: "[PATCH v2 00/15] treewide: Refactor heap related implementation"
Previous message: Daniel Bristot de Oliveira: "[GIT PULL V2] tracing/tools: Updates for 6.9"
Next in thread: Arnaldo Carvalho de Melo: "Re: [PATCH v2 1/2] perf: Suggest inbuilt commands for unknown command"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]