[06/24] Fix "set enum-command value garbage"

Message ID 20190522205327.2568-7-palves@redhat.com
State Superseded
Headers show
Series
  • gdb::option framework, "print -OPT", other cmd options
Related show

Commit Message

Pedro Alves May 22, 2019, 8:53 p.m.
With enum commands, we currently fail to notice garbage after the
value.

Currently:

  (gdb) set print entry-values compact foo
  (gdb) show print entry-values foo
  Printing of function arguments at function entry is "compact".

After this fix:

 (gdb) set print entry-values compact foo
  Garbage after item: "foo"

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* cli/cli-setshow.c (do_set_command) <var_enum>: Detect garbage
	after item.
---
 gdb/cli/cli-setshow.c | 4 ++++
 1 file changed, 4 insertions(+)

-- 
2.14.5

Comments

Sergio Durigan Junior May 23, 2019, 7:13 p.m. | #1
On Wednesday, May 22 2019, Pedro Alves wrote:

> With enum commands, we currently fail to notice garbage after the

> value.

>

> Currently:

>

>   (gdb) set print entry-values compact foo

>   (gdb) show print entry-values foo

>   Printing of function arguments at function entry is "compact".

>

> After this fix:

>

>  (gdb) set print entry-values compact foo

>   Garbage after item: "foo"


I think it would be clearer to specify which item here, otherwise the
user might think that the item is "foo".  Maybe:

  Invalid data after item "compact": "foo"

What do you think?  Otherwise, LGTM.  Thanks.

> gdb/ChangeLog:

> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

>

> 	* cli/cli-setshow.c (do_set_command) <var_enum>: Detect garbage

> 	after item.

> ---

>  gdb/cli/cli-setshow.c | 4 ++++

>  1 file changed, 4 insertions(+)

>

> diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c

> index 96d7bf5c3c0..9d6479ffca2 100644

> --- a/gdb/cli/cli-setshow.c

> +++ b/gdb/cli/cli-setshow.c

> @@ -413,6 +413,10 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c)

>  	if (nmatches > 1)

>  	  error (_("Ambiguous item \"%s\"."), arg);

>  

> +	arg = skip_spaces (arg + len);

> +	if (*arg != '\0')

> +	  error (_("Garbage after item: \"%s\""), arg);

> +

>  	if (*(const char **) c->var != match)

>  	  {

>  	    *(const char **) c->var = match;

> -- 

> 2.14.5


-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
Pedro Alves May 24, 2019, 11:39 a.m. | #2
On 5/23/19 8:13 PM, Sergio Durigan Junior wrote:
> On Wednesday, May 22 2019, Pedro Alves wrote:

> 

>> With enum commands, we currently fail to notice garbage after the

>> value.

>>

>> Currently:

>>

>>   (gdb) set print entry-values compact foo

>>   (gdb) show print entry-values foo

>>   Printing of function arguments at function entry is "compact".

>>

>> After this fix:

>>

>>  (gdb) set print entry-values compact foo

>>   Garbage after item: "foo"

> 

> I think it would be clearer to specify which item here, otherwise the

> user might think that the item is "foo".  Maybe:

> 

>   Invalid data after item "compact": "foo"

> 

> What do you think?  Otherwise, LGTM.  Thanks.


Agreed.  I'm changing the code like this:

diff --git c/gdb/cli/cli-setshow.c w/gdb/cli/cli-setshow.c
index 9d6479ffca2..47e50941e3b 100644
--- c/gdb/cli/cli-setshow.c
+++ w/gdb/cli/cli-setshow.c
@@ -413,9 +413,10 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c)
        if (nmatches > 1)
          error (_("Ambiguous item \"%s\"."), arg);
 
-       arg = skip_spaces (arg + len);
-       if (*arg != '\0')
-         error (_("Garbage after item: \"%s\""), arg);
+       const char *after = skip_spaces (arg + len);
+       if (*after != '\0')
+         error (_("Garbage after item \"%.*s\": \"%s\""),
+                len, arg, after);
 

Also changed the test like this, in patch #9:

--- i/gdb/testsuite/gdb.base/settings.exp
+++ w/gdb/testsuite/gdb.base/settings.exp
@@ -378,7 +378,13 @@ proc_with_prefix test-enum {} {
 
     # Valid value followed by garbage.
     gdb_test "$set_cmd xxx 1" \
-       "Garbage after item: \"1\""
+       "Garbage after item \"xxx\": \"1\""
+    # Valid value followed by garbage, with extra spaces.
+    gdb_test "$set_cmd xxx      1" \
+       "Garbage after item \"xxx\": \"1\""
+    # Abbreviated value followed by garbage.
+    gdb_test "$set_cmd xx 1" \
+       "Garbage after item \"xx\": \"1\""

Thanks,
Pedro Alves

Patch

diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c
index 96d7bf5c3c0..9d6479ffca2 100644
--- a/gdb/cli/cli-setshow.c
+++ b/gdb/cli/cli-setshow.c
@@ -413,6 +413,10 @@  do_set_command (const char *arg, int from_tty, struct cmd_list_element *c)
 	if (nmatches > 1)
 	  error (_("Ambiguous item \"%s\"."), arg);
 
+	arg = skip_spaces (arg + len);
+	if (*arg != '\0')
+	  error (_("Garbage after item: \"%s\""), arg);
+
 	if (*(const char **) c->var != match)
 	  {
 	    *(const char **) c->var = match;