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

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

[Ian Rogers]

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@google.com>
---
v2. Drops a merged patch and rebases. No functional change. Arnaldo
    reported the patch not working for him, but I've not found a
    reproduction and it works for me:
    https://lore.kernel.org/lkml/ZZcdDyyADG8dP8LM@kernel.org/
---
 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

[PATCH v2 2/2] perf help: Lower levenshtein penality for deleting character

[Ian Rogers]

The levenshtein penalty for deleting a character was far higher than
subsituting or inserting a character. Lower the penalty to match that
of inserting a character.

Before:
```
$ perf recccord
perf: 'recccord' is not a perf-command. See 'perf --help'.
```

After:
```
$ perf recccord
perf: 'recccord' is not a perf-command. See 'perf --help'.

Did you mean this?
        record
```

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/help-unknown-cmd.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/help-unknown-cmd.c b/tools/perf/util/help-unknown-cmd.c
index 2ba3369f1620..a0a46e34f8d1 100644
--- a/tools/perf/util/help-unknown-cmd.c
+++ b/tools/perf/util/help-unknown-cmd.c
@@ -73,10 +73,14 @@ const char *help_unknown_cmd(const char *cmd, struct cmdnames *main_cmds)
 
 	if (main_cmds->cnt) {
 		/* This reuses cmdname->len for similarity index */
-		for (i = 0; i < main_cmds->cnt; ++i)
+		for (i = 0; i < main_cmds->cnt; ++i) {
 			main_cmds->names[i]->len =
-				levenshtein(cmd, main_cmds->names[i]->name, 0, 2, 1, 4);
-
+				levenshtein(cmd, main_cmds->names[i]->name,
+					/*swap_penalty=*/0,
+					/*substition_penality=*/2,
+					/*insertion_penality=*/1,
+					/*deletion_penalty=*/1);
+		}
 		qsort(main_cmds->names, main_cmds->cnt,
 		      sizeof(*main_cmds->names), levenshtein_compare);
 
-- 
2.44.0.278.ge034bb2e1d-goog

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

[Arnaldo Carvalho de Melo]

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@google.com>
> ---
> 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@kernel.org/
> ---
>  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
> 

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

[Arnaldo Carvalho de Melo]

On Wed, Mar 20, 2024 at 11:54:09AM -0300, Arnaldo Carvalho de Melo wrote:
> 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@google.com>
> > ---
> > 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...

It somehow gets done_help set to 32767, and this will not run help_unknown_cmd(), continuing after a conf call...

(gdb) p *argv
$7 = 0x7fffffffe4c5 "raccord"
(gdb) s
run_argv (argcp=0x7fffffffdfbc, argv=0x7fffffffdfb0) at perf.c:445
445	{
(gdb) n
447		handle_internal_command(*argcp, *argv);
(gdb) n
450		execv_dashed_external(*argv);
(gdb) p *argv
$8 = (const char **) 0x7fffffffe1d0
(gdb) p **argv
$9 = 0x7fffffffe4c5 "raccord"
(gdb) n
[Detaching after fork from child process 3245070]
451		return 0;
(gdb) n
452	}
(gdb) n
main (argc=1, argv=0x7fffffffe1d0) at perf.c:565
565			if (errno != ENOENT)
(gdb) p; errno
Invalid character ';' in expression.
(gdb) p errno
$10 = 2
(gdb) n
568			if (!done_help) {
(gdb) p done_help
$11 = 32767
(gdb) list
563			run_argv(&argc, &argv);
564	
565			if (errno != ENOENT)
566				break;
567	
568			if (!done_help) {
569				struct cmdnames main_cmds;
570	
571				for (unsigned int i = 0; i < ARRAY_SIZE(commands); i++) {
572					add_cmdname(&main_cmds,
(gdb) 
573						    commands[i].cmd,
574						    strlen(commands[i].cmd));
575				}
576				cmd = argv[0] = help_unknown_cmd(cmd, &main_cmds);
577				clean_cmdnames(&main_cmds);
578				done_help = 1;
579				if (!cmd)
580					break;
581			} else
582				break;
(gdb)

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

[Ian Rogers]

On Wed, Mar 20, 2024 at 8:00 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> On Wed, Mar 20, 2024 at 11:54:09AM -0300, Arnaldo Carvalho de Melo wrote:
> > 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@google.com>
> > > ---
> > > 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...
>
> It somehow gets done_help set to 32767, and this will not run help_unknown_cmd(), continuing after a conf call...
>
> (gdb) p *argv
> $7 = 0x7fffffffe4c5 "raccord"
> (gdb) s
> run_argv (argcp=0x7fffffffdfbc, argv=0x7fffffffdfb0) at perf.c:445
> 445     {
> (gdb) n
> 447             handle_internal_command(*argcp, *argv);
> (gdb) n
> 450             execv_dashed_external(*argv);
> (gdb) p *argv
> $8 = (const char **) 0x7fffffffe1d0
> (gdb) p **argv
> $9 = 0x7fffffffe4c5 "raccord"
> (gdb) n
> [Detaching after fork from child process 3245070]
> 451             return 0;
> (gdb) n
> 452     }
> (gdb) n
> main (argc=1, argv=0x7fffffffe1d0) at perf.c:565
> 565                     if (errno != ENOENT)
> (gdb) p; errno
> Invalid character ';' in expression.
> (gdb) p errno
> $10 = 2
> (gdb) n
> 568                     if (!done_help) {
> (gdb) p done_help
> $11 = 32767
> (gdb) list
> 563                     run_argv(&argc, &argv);
> 564
> 565                     if (errno != ENOENT)
> 566                             break;
> 567
> 568                     if (!done_help) {
> 569                             struct cmdnames main_cmds;
> 570
> 571                             for (unsigned int i = 0; i < ARRAY_SIZE(commands); i++) {
> 572                                     add_cmdname(&main_cmds,
> (gdb)
> 573                                                 commands[i].cmd,
> 574                                                 strlen(commands[i].cmd));
> 575                             }
> 576                             cmd = argv[0] = help_unknown_cmd(cmd, &main_cmds);
> 577                             clean_cmdnames(&main_cmds);
> 578                             done_help = 1;
> 579                             if (!cmd)
> 580                                     break;
> 581                     } else
> 582                             break;
> (gdb)

Ah, the change:

-               static int done_help;
+               int done_help;

created an uninitialized use. Compiler warning/sanitizers? Anyway,
done_help needs hoisting out of the loop and initializing to zero, or
being made static again (ugh).

Thanks,
Ian

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

[Arnaldo Carvalho de Melo]

On Wed, Mar 20, 2024 at 08:12:56AM -0700, Ian Rogers wrote:
> Ah, the change:
> 
> -               static int done_help;
> +               int done_help;
> 
> created an uninitialized use. Compiler warning/sanitizers? Anyway,
> done_help needs hoisting out of the loop and initializing to zero, or
> being made static again (ugh).

⬢[acme@toolbox perf-tools-next]$ git diff
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index c719e6ccd9e27778..f9532b20e87cbf05 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -459,7 +459,7 @@ static int libperf_print(enum libperf_print_level level,
 
 int main(int argc, const char **argv)
 {
-       int err;
+       int err, done_help = 0;
        const char *cmd;
        char sbuf[STRERR_BUFSIZE];
 
@@ -558,8 +558,6 @@ int main(int argc, const char **argv)
        pthread__block_sigwinch();
 
        while (1) {
-               int done_help;
-
                run_argv(&argc, &argv);
 
                if (errno != ENOENT)
⬢[acme@toolbox perf-tools-next]$ perf raccord
  Fatal: Out of memory, realloc failed
⬢[acme@toolbox perf-tools-next]$

Then:

diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index c719e6ccd9e27778..54f62aa6612bc290 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -558,7 +558,7 @@ int main(int argc, const char **argv)
        pthread__block_sigwinch();
 
        while (1) {
-               int done_help;
+               static int done_help;
 
                run_argv(&argc, &argv);
 
⬢[acme@toolbox perf-tools-next]$ perf raccord
  Fatal: Out of memory, realloc failed
⬢[acme@toolbox perf-tools-next]$

- Arnaldo

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

[Ian Rogers]

On Wed, Mar 20, 2024 at 8:20 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> On Wed, Mar 20, 2024 at 08:12:56AM -0700, Ian Rogers wrote:
> > Ah, the change:
> >
> > -               static int done_help;
> > +               int done_help;
> >
> > created an uninitialized use. Compiler warning/sanitizers? Anyway,
> > done_help needs hoisting out of the loop and initializing to zero, or
> > being made static again (ugh).
>
> ⬢[acme@toolbox perf-tools-next]$ git diff
> diff --git a/tools/perf/perf.c b/tools/perf/perf.c
> index c719e6ccd9e27778..f9532b20e87cbf05 100644
> --- a/tools/perf/perf.c
> +++ b/tools/perf/perf.c
> @@ -459,7 +459,7 @@ static int libperf_print(enum libperf_print_level level,
>
>  int main(int argc, const char **argv)
>  {
> -       int err;
> +       int err, done_help = 0;
>         const char *cmd;
>         char sbuf[STRERR_BUFSIZE];
>
> @@ -558,8 +558,6 @@ int main(int argc, const char **argv)
>         pthread__block_sigwinch();
>
>         while (1) {
> -               int done_help;
> -
>                 run_argv(&argc, &argv);
>
>                 if (errno != ENOENT)
> ⬢[acme@toolbox perf-tools-next]$ perf raccord
>   Fatal: Out of memory, realloc failed
> ⬢[acme@toolbox perf-tools-next]$
>
> Then:
>
> diff --git a/tools/perf/perf.c b/tools/perf/perf.c
> index c719e6ccd9e27778..54f62aa6612bc290 100644
> --- a/tools/perf/perf.c
> +++ b/tools/perf/perf.c
> @@ -558,7 +558,7 @@ int main(int argc, const char **argv)
>         pthread__block_sigwinch();
>
>         while (1) {
> -               int done_help;
> +               static int done_help;
>
>                 run_argv(&argc, &argv);
>
> ⬢[acme@toolbox perf-tools-next]$ perf raccord
>   Fatal: Out of memory, realloc failed
> ⬢[acme@toolbox perf-tools-next]$


Similar issue on main_cmds, caught by memory sanitizer. I think the
following fixes it:

```
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index c719e6ccd9e2..bd3f80b5bb46 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -459,7 +459,7 @@ static int libperf_print(enum libperf_print_level level,

int main(int argc, const char **argv)
{
-       int err;
+       int err, done_help = 0;
       const char *cmd;
       char sbuf[STRERR_BUFSIZE];

@@ -558,15 +558,13 @@ int main(int argc, const char **argv)
       pthread__block_sigwinch();

       while (1) {
-               int done_help;
-
               run_argv(&argc, &argv);

               if (errno != ENOENT)
                       break;

               if (!done_help) {
-                       struct cmdnames main_cmds;
+                       struct cmdnames main_cmds = {};

                       for (unsigned int i = 0; i < ARRAY_SIZE(commands); i++) {
                               add_cmdname(&main_cmds,
```

Thanks,
Ian

> - Arnaldo

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

[Arnaldo Carvalho de Melo]

On Wed, Mar 20, 2024 at 12:20:20PM -0300, Arnaldo Carvalho de Melo wrote:
> On Wed, Mar 20, 2024 at 08:12:56AM -0700, Ian Rogers wrote:
> > Ah, the change:
> > 
> > -               static int done_help;
> > +               int done_help;
> > 
> > created an uninitialized use. Compiler warning/sanitizers? Anyway,
> > done_help needs hoisting out of the loop and initializing to zero, or
> > being made static again (ugh).
> 
> ⬢[acme@toolbox perf-tools-next]$ git diff
> diff --git a/tools/perf/perf.c b/tools/perf/perf.c
> index c719e6ccd9e27778..f9532b20e87cbf05 100644
> --- a/tools/perf/perf.c
> +++ b/tools/perf/perf.c
> @@ -459,7 +459,7 @@ static int libperf_print(enum libperf_print_level level,
>  
>  int main(int argc, const char **argv)
>  {
> -       int err;
> +       int err, done_help = 0;
>         const char *cmd;
>         char sbuf[STRERR_BUFSIZE];
>  
> @@ -558,8 +558,6 @@ int main(int argc, const char **argv)
>         pthread__block_sigwinch();
>  
>         while (1) {
> -               int done_help;
> -
>                 run_argv(&argc, &argv);
>  
>                 if (errno != ENOENT)
> ⬢[acme@toolbox perf-tools-next]$ perf raccord
>   Fatal: Out of memory, realloc failed
> ⬢[acme@toolbox perf-tools-next]$
> 
> Then:
> 
> diff --git a/tools/perf/perf.c b/tools/perf/perf.c
> index c719e6ccd9e27778..54f62aa6612bc290 100644
> --- a/tools/perf/perf.c
> +++ b/tools/perf/perf.c
> @@ -558,7 +558,7 @@ int main(int argc, const char **argv)
>         pthread__block_sigwinch();
>  
>         while (1) {
> -               int done_help;
> +               static int done_help;
>  
>                 run_argv(&argc, &argv);
>  
> ⬢[acme@toolbox perf-tools-next]$ perf raccord
>   Fatal: Out of memory, realloc failed
> ⬢[acme@toolbox perf-tools-next]$

That main_cmds variable is uninitialized, which ends up making
add_cmdname() to explode, are you sure this is working on your side?

Further clarifying, this is without considering the second patch, I
haven't got to it yet, from what I recall from the description it
shouldn't matter tho.

- Arnaldo

⬢[acme@toolbox perf-tools-next]$ git diff
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index c719e6ccd9e27778..164b3c78baff4204 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -558,7 +558,7 @@ int main(int argc, const char **argv)
        pthread__block_sigwinch();
 
        while (1) {
-               int done_help;
+               static int done_help;
 
                run_argv(&argc, &argv);
 
@@ -566,7 +566,7 @@ int main(int argc, const char **argv)
                        break;
 
                if (!done_help) {
-                       struct cmdnames main_cmds;
+                       struct cmdnames main_cmds = { 0, };
 
                        for (unsigned int i = 0; i < ARRAY_SIZE(commands); i++) {
                                add_cmdname(&main_cmds,
⬢[acme@toolbox perf-tools-next]$ perf raccord
perf: 'raccord' is not a perf-command. See 'perf --help'.
⬢[acme@toolbox perf-tools-next]$ 

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

[Ian Rogers]

On Wed, Mar 20, 2024 at 8:32 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> On Wed, Mar 20, 2024 at 12:20:20PM -0300, Arnaldo Carvalho de Melo wrote:
> > On Wed, Mar 20, 2024 at 08:12:56AM -0700, Ian Rogers wrote:
> > > Ah, the change:
> > >
> > > -               static int done_help;
> > > +               int done_help;
> > >
> > > created an uninitialized use. Compiler warning/sanitizers? Anyway,
> > > done_help needs hoisting out of the loop and initializing to zero, or
> > > being made static again (ugh).
> >
> > ⬢[acme@toolbox perf-tools-next]$ git diff
> > diff --git a/tools/perf/perf.c b/tools/perf/perf.c
> > index c719e6ccd9e27778..f9532b20e87cbf05 100644
> > --- a/tools/perf/perf.c
> > +++ b/tools/perf/perf.c
> > @@ -459,7 +459,7 @@ static int libperf_print(enum libperf_print_level level,
> >
> >  int main(int argc, const char **argv)
> >  {
> > -       int err;
> > +       int err, done_help = 0;
> >         const char *cmd;
> >         char sbuf[STRERR_BUFSIZE];
> >
> > @@ -558,8 +558,6 @@ int main(int argc, const char **argv)
> >         pthread__block_sigwinch();
> >
> >         while (1) {
> > -               int done_help;
> > -
> >                 run_argv(&argc, &argv);
> >
> >                 if (errno != ENOENT)
> > ⬢[acme@toolbox perf-tools-next]$ perf raccord
> >   Fatal: Out of memory, realloc failed
> > ⬢[acme@toolbox perf-tools-next]$
> >
> > Then:
> >
> > diff --git a/tools/perf/perf.c b/tools/perf/perf.c
> > index c719e6ccd9e27778..54f62aa6612bc290 100644
> > --- a/tools/perf/perf.c
> > +++ b/tools/perf/perf.c
> > @@ -558,7 +558,7 @@ int main(int argc, const char **argv)
> >         pthread__block_sigwinch();
> >
> >         while (1) {
> > -               int done_help;
> > +               static int done_help;
> >
> >                 run_argv(&argc, &argv);
> >
> > ⬢[acme@toolbox perf-tools-next]$ perf raccord
> >   Fatal: Out of memory, realloc failed
> > ⬢[acme@toolbox perf-tools-next]$
>
> That main_cmds variable is uninitialized, which ends up making
> add_cmdname() to explode, are you sure this is working on your side?
>
> Further clarifying, this is without considering the second patch, I
> haven't got to it yet, from what I recall from the description it
> shouldn't matter tho.
>
> - Arnaldo
>
> ⬢[acme@toolbox perf-tools-next]$ git diff
> diff --git a/tools/perf/perf.c b/tools/perf/perf.c
> index c719e6ccd9e27778..164b3c78baff4204 100644
> --- a/tools/perf/perf.c
> +++ b/tools/perf/perf.c
> @@ -558,7 +558,7 @@ int main(int argc, const char **argv)
>         pthread__block_sigwinch();
>
>         while (1) {
> -               int done_help;
> +               static int done_help;
>
>                 run_argv(&argc, &argv);
>
> @@ -566,7 +566,7 @@ int main(int argc, const char **argv)
>                         break;
>
>                 if (!done_help) {
> -                       struct cmdnames main_cmds;
> +                       struct cmdnames main_cmds = { 0, };
>
>                         for (unsigned int i = 0; i < ARRAY_SIZE(commands); i++) {
>                                 add_cmdname(&main_cmds,
> ⬢[acme@toolbox perf-tools-next]$ perf raccord
> perf: 'raccord' is not a perf-command. See 'perf --help'.
> ⬢[acme@toolbox perf-tools-next]$

Perhaps change the 'a' to an 'e', the 2nd patch just improves the
weights on removing characters. For me:

```
$ /tmp/perf/perf raccord
perf: 'raccord' is not a perf-command. See 'perf --help'.

Did you mean this?
       record
```

For a memory sanitizer build with the 2 patches and the 2 line changes
I sent before. Fwiw, I'm trying to get memory sanitizer to test "perf
test" but running into an issue with scandirat not having an msan
interceptor.

Thanks,
Ian

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

[Arnaldo Carvalho de Melo]

On Wed, Mar 20, 2024 at 12:32:50PM -0300, Arnaldo Carvalho de Melo wrote:
> On Wed, Mar 20, 2024 at 12:20:20PM -0300, Arnaldo Carvalho de Melo wrote:
> > On Wed, Mar 20, 2024 at 08:12:56AM -0700, Ian Rogers wrote:
> > > Ah, the change:
> > > 
> > > -               static int done_help;
> > > +               int done_help;
> > > 
> > > created an uninitialized use. Compiler warning/sanitizers? Anyway,
> > > done_help needs hoisting out of the loop and initializing to zero, or
> > > being made static again (ugh).
> > 
> > ⬢[acme@toolbox perf-tools-next]$ git diff
> > diff --git a/tools/perf/perf.c b/tools/perf/perf.c
> > index c719e6ccd9e27778..f9532b20e87cbf05 100644
> > --- a/tools/perf/perf.c
> > +++ b/tools/perf/perf.c
> > @@ -459,7 +459,7 @@ static int libperf_print(enum libperf_print_level level,
> >  
> >  int main(int argc, const char **argv)
> >  {
> > -       int err;
> > +       int err, done_help = 0;
> >         const char *cmd;
> >         char sbuf[STRERR_BUFSIZE];
> >  
> > @@ -558,8 +558,6 @@ int main(int argc, const char **argv)
> >         pthread__block_sigwinch();
> >  
> >         while (1) {
> > -               int done_help;
> > -
> >                 run_argv(&argc, &argv);
> >  
> >                 if (errno != ENOENT)
> > ⬢[acme@toolbox perf-tools-next]$ perf raccord
> >   Fatal: Out of memory, realloc failed
> > ⬢[acme@toolbox perf-tools-next]$
> > 
> > Then:
> > 
> > diff --git a/tools/perf/perf.c b/tools/perf/perf.c
> > index c719e6ccd9e27778..54f62aa6612bc290 100644
> > --- a/tools/perf/perf.c
> > +++ b/tools/perf/perf.c
> > @@ -558,7 +558,7 @@ int main(int argc, const char **argv)
> >         pthread__block_sigwinch();
> >  
> >         while (1) {
> > -               int done_help;
> > +               static int done_help;
> >  
> >                 run_argv(&argc, &argv);
> >  
> > ⬢[acme@toolbox perf-tools-next]$ perf raccord
> >   Fatal: Out of memory, realloc failed
> > ⬢[acme@toolbox perf-tools-next]$
> 
> That main_cmds variable is uninitialized, which ends up making
> add_cmdname() to explode, are you sure this is working on your side?
> 
> Further clarifying, this is without considering the second patch, I
> haven't got to it yet, from what I recall from the description it
> shouldn't matter tho.
> 
> - Arnaldo
> 
> ⬢[acme@toolbox perf-tools-next]$ git diff
> diff --git a/tools/perf/perf.c b/tools/perf/perf.c
> index c719e6ccd9e27778..164b3c78baff4204 100644
> --- a/tools/perf/perf.c
> +++ b/tools/perf/perf.c
> @@ -558,7 +558,7 @@ int main(int argc, const char **argv)
>         pthread__block_sigwinch();
>  
>         while (1) {
> -               int done_help;
> +               static int done_help;
>  
>                 run_argv(&argc, &argv);
>  
> @@ -566,7 +566,7 @@ int main(int argc, const char **argv)
>                         break;
>  
>                 if (!done_help) {
> -                       struct cmdnames main_cmds;
> +                       struct cmdnames main_cmds = { 0, };
>  
>                         for (unsigned int i = 0; i < ARRAY_SIZE(commands); i++) {
>                                 add_cmdname(&main_cmds,
> ⬢[acme@toolbox perf-tools-next]$ perf raccord
> perf: 'raccord' is not a perf-command. See 'perf --help'.
> ⬢[acme@toolbox perf-tools-next]$ 

But:

⬢[acme@toolbox perf-tools-next]$ perf raccord
perf: 'raccord' is not a perf-command. See 'perf --help'.
⬢[acme@toolbox perf-tools-next]$ perf ricord
perf: 'ricord' is not a perf-command. See 'perf --help'.

Did you mean this?
	record
⬢[acme@toolbox perf-tools-next]$ perf ricord
perf: 'ricord' is not a perf-command. See 'perf --help'.

Did you mean this?
	record
⬢[acme@toolbox perf-tools-next]$

So I'll add that and go on from there.

- Arnaldo

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

[Arnaldo Carvalho de Melo]

On Wed, Mar 20, 2024 at 12:41:45PM -0300, Arnaldo Carvalho de Melo wrote:
> ⬢[acme@toolbox perf-tools-next]$ perf raccord
> perf: 'raccord' is not a perf-command. See 'perf --help'.

This works with the second patch.

- Arnaldo

> ⬢[acme@toolbox perf-tools-next]$ perf ricord
> perf: 'ricord' is not a perf-command. See 'perf --help'.
> 
> Did you mean this?
> 	record
> ⬢[acme@toolbox perf-tools-next]$ perf ricord
> perf: 'ricord' is not a perf-command. See 'perf --help'.
> 
> Did you mean this?
> 	record
> ⬢[acme@toolbox perf-tools-next]$
> 
> So I'll add that and go on from there.
> 
> - Arnaldo

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

[Ian Rogers]

On Wed, Mar 20, 2024 at 8:44 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> On Wed, Mar 20, 2024 at 12:41:45PM -0300, Arnaldo Carvalho de Melo wrote:
> > ⬢[acme@toolbox perf-tools-next]$ perf raccord
> > perf: 'raccord' is not a perf-command. See 'perf --help'.
>
> This works with the second patch.
>
> - Arnaldo
>
> > ⬢[acme@toolbox perf-tools-next]$ perf ricord
> > perf: 'ricord' is not a perf-command. See 'perf --help'.
> >
> > Did you mean this?
> >       record
> > ⬢[acme@toolbox perf-tools-next]$ perf ricord
> > perf: 'ricord' is not a perf-command. See 'perf --help'.
> >
> > Did you mean this?
> >       record
> > ⬢[acme@toolbox perf-tools-next]$
> >
> > So I'll add that and go on from there.

Thanks, let me know if I need to send a v3 with the 2 uninitialized
variables fixed.

Thanks,
Ian

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

[Arnaldo Carvalho de Melo]

On Wed, Mar 20, 2024 at 10:07:55AM -0700, Ian Rogers wrote:
> On Wed, Mar 20, 2024 at 8:44 AM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> >
> > On Wed, Mar 20, 2024 at 12:41:45PM -0300, Arnaldo Carvalho de Melo wrote:
> > > ⬢[acme@toolbox perf-tools-next]$ perf raccord
> > > perf: 'raccord' is not a perf-command. See 'perf --help'.
> >
> > This works with the second patch.
> >
> > - Arnaldo
> >
> > > ⬢[acme@toolbox perf-tools-next]$ perf ricord
> > > perf: 'ricord' is not a perf-command. See 'perf --help'.
> > >
> > > Did you mean this?
> > >       record
> > > ⬢[acme@toolbox perf-tools-next]$ perf ricord
> > > perf: 'ricord' is not a perf-command. See 'perf --help'.
> > >
> > > Did you mean this?
> > >       record
> > > ⬢[acme@toolbox perf-tools-next]$
> > >
> > > So I'll add that and go on from there.
> 
> Thanks, let me know if I need to send a v3 with the 2 uninitialized
> variables fixed.

No need, its all in tmp.perf-tools-next now, will go to perf-tools-next
soon,

- Arnaldo