[PATCH] Fix PR gdb/17035: "show user" doesn't list user-defined commands that have empty bodies.

[PATCH] Fix PR gdb/17035: "show user" doesn't list user-defined commands that have empty bodies.

[Gabriel Krisman Bertazi]

User-defined commands that have empty bodies weren't being shown because
the print function returned too soon.  Now, it prints the command's name
before checking if it has any body at all.  This also fixes the same
problem on "show user <myemptycommand>", which wasn't being printed due
to a similar reason.

gdb/
2014-08-20  Gabriel Krisman Bertazi  <gabriel@krisman.be>

	* cli/cli-cmds.c (show_user): Don't return with error message
	  when c->user_commands is NULL.
	* cli/cli-script.c (show_user_1): Verify cmdlines only after
	  printing command name.

gdb/testsuite/
2014-08-20  Gabriel Krisman Bertazi  <gabriel@krisman.be>

	* gdb.base/commands.exp: Include tests to verify user-defined
	  commands with empty bodies.
	* gdb.base/default.exp: Update testcase output.
	* gdb.base/setshow.exp: Update testcase output.
---
 gdb/cli/cli-cmds.c                  |  5 ++---
 gdb/cli/cli-script.c                |  4 ++--
 gdb/testsuite/gdb.base/commands.exp | 22 ++++++++++++++++++++++
 gdb/testsuite/gdb.base/default.exp  |  2 +-
 gdb/testsuite/gdb.base/setshow.exp  |  2 +-
 5 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index b415267..4c42ff1 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -1245,9 +1245,8 @@ show_user (char *args, int from_tty)
       const char *comname = args;
 
       c = lookup_cmd (&comname, cmdlist, "", 0, 1);
-      /* c->user_commands would be NULL if it's a python/scheme command.  */
-      if (c->class != class_user || !c->user_commands)
-	error (_("Not a user command."));
+      if (c->class != class_user)
+        error (_("Not a user command."));
       show_user_1 (c, "", args, gdb_stdout);
     }
   else
diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index 0f0a97e..37cb82a 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -1717,10 +1717,10 @@ show_user_1 (struct cmd_list_element *c, const char *prefix, const char *name,
     }
 
   cmdlines = c->user_commands;
-  if (!cmdlines)
-    return;
   fprintf_filtered (stream, "User command \"%s%s\":\n", prefix, name);
 
+  if (!cmdlines)
+    return;
   print_command_lines (current_uiout, cmdlines, 1);
   fputs_filtered ("\n", stream);
 }
diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp
index 7363420..b2d1c76 100644
--- a/gdb/testsuite/gdb.base/commands.exp
+++ b/gdb/testsuite/gdb.base/commands.exp
@@ -243,6 +243,28 @@ proc user_defined_command_test {} {
    gdb_test "show user mycommand" \
 	"  while \\\$arg0.*set.*    if \\\(\\\$arg0.*p/x.*    else\[^\n\].*p/x.*    end\[^\n\].*  end\[^\n\].*" \
 	   "display user command in user_defined_command_test"
+
+    #  Create and test an user-defined command with an empty body.
+    gdb_test_multiple "define myemptycommand" \
+	"define myemptycommand in user_defined_command_test" {
+	    -re "End with"  {
+		pass "define myemptycommand in user_defined_command_test"
+	    }
+	}
+    gdb_test "end" \
+	"" \
+	"End definition of user-defined command with empty body."
+
+    gdb_test_no_output "myemptycommand" \
+	"execute user-defined empty command in user_defined_command_test"
+
+    gdb_test "show user" \
+	"User command \"myemptycommand.*" \
+	"display empty command in command list in user_defined_command_test"
+
+    gdb_test "show user myemptycommand" \
+	"User command \"myemptycommand.*" \
+	"display user-defined emtpy command in user_defined_command_test"
 }
 
 proc watchpoint_command_test {} {
diff --git a/gdb/testsuite/gdb.base/default.exp b/gdb/testsuite/gdb.base/default.exp
index 6674df3..c9fbedd 100644
--- a/gdb/testsuite/gdb.base/default.exp
+++ b/gdb/testsuite/gdb.base/default.exp
@@ -693,7 +693,7 @@ gdb_test "show prompt" "Gdb's prompt is \"$gdb_prompt \".*" "show prompt"
 #test show radix
 gdb_test "show radix" "Input and output radices set to decimal 10, hex a, octal 12." "show radix"
 #test show user
-gdb_test_no_output "show user" "show user"
+gdb_test "show user" "User command \"user-defined\".*" "show user"
 #test show values
 gdb_test_no_output "show values" "show values"
 #test show verbose
diff --git a/gdb/testsuite/gdb.base/setshow.exp b/gdb/testsuite/gdb.base/setshow.exp
index 639ca72..302039f 100644
--- a/gdb/testsuite/gdb.base/setshow.exp
+++ b/gdb/testsuite/gdb.base/setshow.exp
@@ -275,7 +275,7 @@ gdb_test_no_output "set write on" "set write on"
 # This is only supported on targets which use exec.o.
 gdb_test "show write" "Writing into executable and core files is on..*" "show write (on)" 
 #test show user
-gdb_test_no_output "show user" "show user" 
+gdb_test "show user" "User command \"user-defined\".*" "show user"
 #test set verbose on
 gdb_test_no_output "set verbose on" "set verbose on" 
 #test show verbose on
-- 
1.9.3

Re: [PATCH] Fix PR gdb/17035: "show user" doesn't list user-defined commands that have empty bodies.

[Pedro Alves]

Hi Gabriel,

Thanks for the patch!

On 08/20/2014 07:12 AM, Gabriel Krisman Bertazi wrote:
>        c = lookup_cmd (&comname, cmdlist, "", 0, 1);
> -      /* c->user_commands would be NULL if it's a python/scheme command.  */
> -      if (c->class != class_user || !c->user_commands)
> -	error (_("Not a user command."));
> +      if (c->class != class_user)
> +        error (_("Not a user command."));

Doesn't this mean this reverts part of 7d74f2446, and thus now we'd
show python/scheme commands?

IIUC 7d74f2446, it looks like gdb.python/py-cmd.exp is missing
a test that makes sure "show user" doesn't list the user-defined python command.

(hmm, this "show user" vs "help user-defined" difference isn't very
intuitive)

> --- a/gdb/testsuite/gdb.base/default.exp
> +++ b/gdb/testsuite/gdb.base/default.exp
> @@ -693,7 +693,7 @@ gdb_test "show prompt" "Gdb's prompt is \"$gdb_prompt \".*" "show prompt"
>  #test show radix
>  gdb_test "show radix" "Input and output radices set to decimal 10, hex a, octal 12." "show radix"
>  #test show user
> -gdb_test_no_output "show user" "show user"
> +gdb_test "show user" "User command \"user-defined\".*" "show user"
>  #test show values

What is this printing now ?

Thanks,
Pedro Alves

Re: [PATCH] Fix PR gdb/17035: "show user" doesn't list user-defined commands that have empty bodies.

[Sergio Durigan Junior]

On Wednesday, August 20 2014, Gabriel Krisman Bertazi wrote:

> diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
> index b415267..4c42ff1 100644
> --- a/gdb/cli/cli-cmds.c
> +++ b/gdb/cli/cli-cmds.c
> @@ -1245,9 +1245,8 @@ show_user (char *args, int from_tty)
>        const char *comname = args;
>  
>        c = lookup_cmd (&comname, cmdlist, "", 0, 1);
> -      /* c->user_commands would be NULL if it's a python/scheme command.  */
> -      if (c->class != class_user || !c->user_commands)
> -	error (_("Not a user command."));
> +      if (c->class != class_user)
> +        error (_("Not a user command."));
   ^^^^^^^^

Missing TAB.

Minor-tiny nit.

Cheers,

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: [PATCH] Fix PR gdb/17035: "show user" doesn't list user-defined commands that have empty bodies.

[Gabriel Krisman Bertazi]

[-- Attachment #1: Type: text/plain, Size: 1870 bytes --]

Pedro Alves <palves@redhat.com> writes:

>>        c = lookup_cmd (&comname, cmdlist, "", 0, 1);
>> -      /* c->user_commands would be NULL if it's a python/scheme command.  */
>> -      if (c->class != class_user || !c->user_commands)
>> -	error (_("Not a user command."));
>> +      if (c->class != class_user)
>> +        error (_("Not a user command."));
>
> Doesn't this mean this reverts part of 7d74f2446, and thus now we'd
> show python/scheme commands?
>
> IIUC 7d74f2446, it looks like gdb.python/py-cmd.exp is missing
> a test that makes sure "show user" doesn't list the user-defined python command.
>
> (hmm, this "show user" vs "help user-defined" difference isn't very
> intuitive)

Hello Pedro,

Thanks for your review.

Indeed, this means we'd start to show python/scheme commands, but I
wasn't aware of commit 7d74f2446.

I don't see that as a problem, because python/scheme are also user
commands (or, at least, are part of class_user class).  Considering that
"help user-defined" also shows python/scheme commands, I believe it
would be more practical to just list them on "show user" (should we
update the doc string, as well).

Either way, the "Not a user command" error message is quite misleading
for python/scheme commands, though.

If you disagree, I'd be happy to update the patch to rely on another
condition to avoid printing python/scheme commands.
c->function.cfunc/sfunc == NULL seems to work pretty well to avoid
printing python commands.

What do you think?

> What is this printing now ?

Now, it prints:

(gdb) show user
User command "emptycmd":
User command "cmd1":
  print "Hello"

User command "python-hello":
User command "user-defined":

Using the verification for cfunc/sfunc above, also avoids
printing the "user-defined" line.

Thanks,

-- 
Gabriel Krisman Bertazi

[-- Attachment #2: Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH] Fix PR gdb/17035: "show user" doesn't list user-defined commands that have empty bodies.

[Pedro Alves]

Hi Gabriel,

On 08/23/2014 03:27 AM, Gabriel Krisman Bertazi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>>>        c = lookup_cmd (&comname, cmdlist, "", 0, 1);
>>> -      /* c->user_commands would be NULL if it's a python/scheme command.  */
>>> -      if (c->class != class_user || !c->user_commands)
>>> -	error (_("Not a user command."));
>>> +      if (c->class != class_user)
>>> +        error (_("Not a user command."));
>>
>> Doesn't this mean this reverts part of 7d74f2446, and thus now we'd
>> show python/scheme commands?
>>
>> IIUC 7d74f2446, it looks like gdb.python/py-cmd.exp is missing
>> a test that makes sure "show user" doesn't list the user-defined python command.
>>
>> (hmm, this "show user" vs "help user-defined" difference isn't very
>> intuitive)
> 
> Hello Pedro,
> 
> Thanks for your review.
> 
> Indeed, this means we'd start to show python/scheme commands, but I
> wasn't aware of commit 7d74f2446.

I found it by noticing you were removing the
"c->user_commands would be NULL if it's a python/scheme command"
comment, and using "git blame" to find out why that comment was
there in the first place.

> 
> I don't see that as a problem, because python/scheme are also user
> commands (or, at least, are part of class_user class).  Considering that
> "help user-defined" also shows python/scheme commands, I believe it
> would be more practical to just list them on "show user" (should we
> update the doc string, as well).

I think we'll need to track down the original discussion, and
ask whoever was involved.  Otherwise, we'd just be going in
circles.  :-)

E.g., that commit changed the online help and the manual to
try to make that clear.

(gdb) help show user
Show definitions of non-python/scheme user defined commands.
Argument is the name of the user defined command.
With no argument, show definitions of all user defined commands.


> Either way, the "Not a user command" error message is quite misleading
> for python/scheme commands, though.

> 
> If you disagree, I'd be happy to update the patch to rely on another
> condition to avoid printing python/scheme commands.
> c->function.cfunc/sfunc == NULL seems to work pretty well to avoid
> printing python commands.
> 
> What do you think?
> 
>> What is this printing now ?
> 
> Now, it prints:
> 
> (gdb) show user
> User command "emptycmd":
> User command "cmd1":
>   print "Hello"
> 
> User command "python-hello":
> User command "user-defined":
> 

The question was actually in context of the gdb.base/default.exp change:

> --- a/gdb/testsuite/gdb.base/default.exp
> +++ b/gdb/testsuite/gdb.base/default.exp
> @@ -693,7 +693,7 @@ gdb_test "show prompt" "Gdb's prompt is \"$gdb_prompt \".*" "show prompt"
>  #test show radix
>  gdb_test "show radix" "Input and output radices set to decimal 10, hex a, octal 12." "show radix"
>  #test show user
> -gdb_test_no_output "show user" "show user"
> +gdb_test "show user" "User command \"user-defined\".*" "show user"

What are we showing now by default?

> Using the verification for cfunc/sfunc above, also avoids
> printing the "user-defined" line.

Hmm, but what is that "user-defined" command ?

Thanks,
Pedro Alves

Re: [PATCH] Fix PR gdb/17035: "show user" doesn't list user-defined commands that have empty bodies.

[Gabriel Krisman Bertazi]

[-- Attachment #1: Type: text/plain, Size: 2589 bytes --]

Hello Pedro,

I appreciate your comments,

>> I don't see that as a problem, because python/scheme are also user
>> commands (or, at least, are part of class_user class).  Considering that
>> "help user-defined" also shows python/scheme commands, I believe it
>> would be more practical to just list them on "show user" (should we
>> update the doc string, as well).
>
> I think we'll need to track down the original discussion, and
> ask whoever was involved.  Otherwise, we'd just be going in
> circles.  :-)
>
> E.g., that commit changed the online help and the manual to
> try to make that clear.
>
> (gdb) help show user
> Show definitions of non-python/scheme user defined commands.
> Argument is the name of the user defined command.
> With no argument, show definitions of all user defined commands.
>

Reviewing the original discussion, (for the record,
<https://sourceware.org/ml/gdb-patches/2012-02/msg00246.html>), Doug has
made a pretty good point on not showing python/scheme commands here.

Since this is a simple bug fix, I'd rather not modify the behavior
here, so I rewrote the patch to not show python/scheme commands.  It got
a little simpler, since I don't have to touch a lot of testcases
anymore.

Now we use the fact that the c->function union is NULL when we have a
python/scheme command (they both use c->func directly).

I also added two testcases to make sure we are not printing scheme or
python commands, as you suggested.

>> Using the verification for cfunc/sfunc above, also avoids
>> printing the "user-defined" line.
>
> Hmm, but what is that "user-defined" command ?

user-defined is a null command.  It does nothing.  It is defined as
follows:

  add_cmd ("user-defined", class_user, NULL, _("\
User-defined commands.\n\
The commands in this class are those defined by the user.\n\
Use the \"define\" command to define a command."), &cmdlist);

I guess it's only purpose is at help user-defined.

Do you think it is acceptable now?

gdb/
2014-08-20  Gabriel Krisman Bertazi  <gabriel@krisman.be>

	* cli/cli-cmds.c (show_user): Verify if c->function is NULL to
	  decide whether c is a python/scheme command.
	* cli/cli-script.c (show_user_1): Only verify cmdlines after
	  printing command name.

gdb/testsuite/
2014-08-20  Gabriel Krisman Bertazi  <gabriel@krisman.be>

	* gdb.base/commands.exp: Include tests to verify user-defined
	  commands with empty bodies.
	* gdb.python/py-cmd.exp: Don't show user-defined python commands
	  in `show user command`
	* gdb.python/scm-cmd.exp: Don't show user-defined scheme commands
	  in `show user command`


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: show_user_v2.patch --]
[-- Type: text/x-patch, Size: 4960 bytes --]

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index b415267..4f624f2 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -1245,8 +1245,9 @@ show_user (char *args, int from_tty)
       const char *comname = args;
 
       c = lookup_cmd (&comname, cmdlist, "", 0, 1);
-      /* c->user_commands would be NULL if it's a python/scheme command.  */
-      if (c->class != class_user || !c->user_commands)
+      /* c->function.cfunc would be NULL if it's a python/scheme
+	 command.  */
+      if (c->class != class_user || c->function.cfunc == NULL)
 	error (_("Not a user command."));
       show_user_1 (c, "", args, gdb_stdout);
     }
@@ -1254,7 +1255,8 @@ show_user (char *args, int from_tty)
     {
       for (c = cmdlist; c; c = c->next)
 	{
-	  if (c->class == class_user || c->prefixlist != NULL)
+	  if ((c->class == class_user && c->function.cfunc != NULL)
+	      || c->prefixlist != NULL)
 	    show_user_1 (c, "", c->name, gdb_stdout);
 	}
     }
diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index 0f0a97e..37cb82a 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -1717,10 +1717,10 @@ show_user_1 (struct cmd_list_element *c, const char *prefix, const char *name,
     }
 
   cmdlines = c->user_commands;
-  if (!cmdlines)
-    return;
   fprintf_filtered (stream, "User command \"%s%s\":\n", prefix, name);
 
+  if (!cmdlines)
+    return;
   print_command_lines (current_uiout, cmdlines, 1);
   fputs_filtered ("\n", stream);
 }
diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp
index 7363420..b2d1c76 100644
--- a/gdb/testsuite/gdb.base/commands.exp
+++ b/gdb/testsuite/gdb.base/commands.exp
@@ -243,6 +243,28 @@ proc user_defined_command_test {} {
    gdb_test "show user mycommand" \
 	"  while \\\$arg0.*set.*    if \\\(\\\$arg0.*p/x.*    else\[^\n\].*p/x.*    end\[^\n\].*  end\[^\n\].*" \
 	   "display user command in user_defined_command_test"
+
+    #  Create and test an user-defined command with an empty body.
+    gdb_test_multiple "define myemptycommand" \
+	"define myemptycommand in user_defined_command_test" {
+	    -re "End with"  {
+		pass "define myemptycommand in user_defined_command_test"
+	    }
+	}
+    gdb_test "end" \
+	"" \
+	"End definition of user-defined command with empty body."
+
+    gdb_test_no_output "myemptycommand" \
+	"execute user-defined empty command in user_defined_command_test"
+
+    gdb_test "show user" \
+	"User command \"myemptycommand.*" \
+	"display empty command in command list in user_defined_command_test"
+
+    gdb_test "show user myemptycommand" \
+	"User command \"myemptycommand.*" \
+	"display user-defined emtpy command in user_defined_command_test"
 }
 
 proc watchpoint_command_test {} {
diff --git a/gdb/testsuite/gdb.guile/scm-cmd.exp b/gdb/testsuite/gdb.guile/scm-cmd.exp
index a407f63..36ef03d 100644
--- a/gdb/testsuite/gdb.guile/scm-cmd.exp
+++ b/gdb/testsuite/gdb.guile/scm-cmd.exp
@@ -134,6 +134,10 @@ gdb_test "help user-defined" \
     "User-defined commands.\[\r\n\]+The commands in this class are those defined by the user.\[\r\n\]+Use the \"define\" command to define a command.\[\r\n\]+List of commands:\[\r\n\]+test-help -- Docstring\[\r\n\]+Type \"help\" followed by command name for full documentation.\[\r\n\]+Type \"apropos word\" to search for commands related to \"word\".\[\r\n\]+Command name abbreviations are allowed if unambiguous.\[\r\n\]+" \
     "see user-defined command in `help user-defined`"
 
+# Make sure the command does not show up in `show user`.
+gdb_test "show user test-help" "Not a user command\." \
+    "Don't show user-defined scheme command in `show user command`"
+
 # Test expression completion on fields.
 
 gdb_test_multiline "expression completion command" \
diff --git a/gdb/testsuite/gdb.python/py-cmd.exp b/gdb/testsuite/gdb.python/py-cmd.exp
index a87aecb..3ae4053 100644
--- a/gdb/testsuite/gdb.python/py-cmd.exp
+++ b/gdb/testsuite/gdb.python/py-cmd.exp
@@ -161,6 +161,10 @@ gdb_test "test_help ugh" "test_cmd output, arg = ugh" "call simple user-defined
 # Make sure the command shows up in `help user-defined`.
 gdb_test "help user-defined" "User-defined commands.\[\r\n\]+The commands in this class are those defined by the user.\[\r\n\]+Use the \"define\" command to define a command.\[\r\n\]+\[\r\n\]+List of commands:\[\r\n\]+\[\r\n\]+test_help -- Docstring\[\r\n\]+\[\r\n\]+Type \"help\" followed by command name for full documentation.\[\r\n\]+Type \"apropos word\" to search for commands related to \"word\".\[\r\n\]+Command name abbreviations are allowed if unambiguous.\[\r\n\]+" "see user-defined command in `help user-defined`"
 
+# Make sure the command does not show up in `show user`.
+gdb_test "show user test_help" "Not a user command\." \
+    "Don't show user-defined python command in `show user command`"
+
 # Test expression completion on fields
 gdb_py_test_multiple "expression completion command" \
   "python" "" \

[-- Attachment #3: Type: text/plain, Size: 30 bytes --]



-- 
Gabriel Krisman Bertazi

Re: [PATCH] Fix PR gdb/17035: "show user" doesn't list user-defined commands that have empty bodies.

[Pedro Alves]

On 08/31/2014 07:35 PM, Gabriel Krisman Bertazi wrote:
> Hello Pedro,
> 
> I appreciate your comments,
> 
>>> >> I don't see that as a problem, because python/scheme are also user
>>> >> commands (or, at least, are part of class_user class).  Considering that
>>> >> "help user-defined" also shows python/scheme commands, I believe it
>>> >> would be more practical to just list them on "show user" (should we
>>> >> update the doc string, as well).
>> >
>> > I think we'll need to track down the original discussion, and
>> > ask whoever was involved.  Otherwise, we'd just be going in
>> > circles.  :-)
>> >
>> > E.g., that commit changed the online help and the manual to
>> > try to make that clear.
>> >
>> > (gdb) help show user
>> > Show definitions of non-python/scheme user defined commands.
>> > Argument is the name of the user defined command.
>> > With no argument, show definitions of all user defined commands.
>> >
> Reviewing the original discussion, (for the record,
> <https://sourceware.org/ml/gdb-patches/2012-02/msg00246.html>), Doug has
> made a pretty good point on not showing python/scheme commands here.
> 
> Since this is a simple bug fix, I'd rather not modify the behavior
> here, so I rewrote the patch to not show python/scheme commands.  It got
> a little simpler, since I don't have to touch a lot of testcases
> anymore.
> 
> Now we use the fact that the c->function union is NULL when we have a
> python/scheme command (they both use c->func directly).
> 
> I also added two testcases to make sure we are not printing scheme or
> python commands, as you suggested.

Thanks!

> 
>>> >> Using the verification for cfunc/sfunc above, also avoids
>>> >> printing the "user-defined" line.
>> >
>> > Hmm, but what is that "user-defined" command ?
> user-defined is a null command.  It does nothing.  It is defined as
> follows:
> 
>   add_cmd ("user-defined", class_user, NULL, _("\
> User-defined commands.\n\
> The commands in this class are those defined by the user.\n\
> Use the \"define\" command to define a command."), &cmdlist);
> 
> I guess it's only purpose is at help user-defined.

Yeah.

> 
> Do you think it is acceptable now?

Yeah.  Close now.  Only nits on the details.

> 
> gdb/
> 2014-08-20  Gabriel Krisman Bertazi  <gabriel@krisman.be>
> 
> 	* cli/cli-cmds.c (show_user): Verify if c->function is NULL to
> 	  decide whether c is a python/scheme command.
> 	* cli/cli-script.c (show_user_1): Only verify cmdlines after
> 	  printing command name.
> 
> gdb/testsuite/
> 2014-08-20  Gabriel Krisman Bertazi  <gabriel@krisman.be>
> 
> 	* gdb.base/commands.exp: Include tests to verify user-defined
> 	  commands with empty bodies.

> 	* gdb.python/py-cmd.exp: Don't show user-defined python commands
> 	  in `show user command`

Prefix that with "Test that we" or some such.  Add missing period
at the end.


> 	* gdb.python/scm-cmd.exp: Don't show user-defined scheme commands
> 	  in `show user command`

Likewise.

> 
> 
> show_user_v2.patch
> 
> 
> diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
> index b415267..4f624f2 100644
> --- a/gdb/cli/cli-cmds.c
> +++ b/gdb/cli/cli-cmds.c
> @@ -1245,8 +1245,9 @@ show_user (char *args, int from_tty)
>        const char *comname = args;
>  
>        c = lookup_cmd (&comname, cmdlist, "", 0, 1);
> -      /* c->user_commands would be NULL if it's a python/scheme command.  */
> -      if (c->class != class_user || !c->user_commands)
> +      /* c->function.cfunc would be NULL if it's a python/scheme
> +	 command.  */
> +      if (c->class != class_user || c->function.cfunc == NULL)
>  	error (_("Not a user command."));
>        show_user_1 (c, "", args, gdb_stdout);
>      }
> @@ -1254,7 +1255,8 @@ show_user (char *args, int from_tty)
>      {
>        for (c = cmdlist; c; c = c->next)
>  	{
> -	  if (c->class == class_user || c->prefixlist != NULL)
> +	  if ((c->class == class_user && c->function.cfunc != NULL)


> -      /* c->user_commands would be NULL if it's a python/scheme command.  */
> -      if (c->class != class_user || !c->user_commands)

> +	  if ((c->class == class_user && c->function.cfunc != NULL)
> +	      || c->prefixlist != NULL)

I think it'd be great if we had a small helper function for this
predicate.  So that we could write something like:

	  if (cli_user_command_p (c))

and:

	  if (cli_user_command_p (c) || c->prefixlist != NULL)





Also, best not assume the compiler allows accessing c->function.cfunc
when it's c->function.sfunc that is actually active in the union, as
in:

> +	  if ((c->class == class_user && c->function.cfunc != NULL)



int
cli_user_command_p (struct cmd_list_element *cmd)
{
  return (cmd->func == do_cfunc && cmd->function.cfunc != NULL)
          || cmd->func == do_sfunc && cmd->function.sfunc != NULL);
}

or just (don't know if it works):

int
cli_user_command_p (struct cmd_list_element *cmd)
{
  return (cmd->func == do_cfunc || cmd->func == do_sfunc);
}


> +	      || c->prefixlist != NULL)
>  	    show_user_1 (c, "", c->name, gdb_stdout);
>  	}
>      }


> diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
> index 0f0a97e..37cb82a 100644
> --- a/gdb/cli/cli-script.c
> +++ b/gdb/cli/cli-script.c
> @@ -1717,10 +1717,10 @@ show_user_1 (struct cmd_list_element *c, const char *prefix, const char *name,
>      }
>  
>    cmdlines = c->user_commands;
> -  if (!cmdlines)
> -    return;
>    fprintf_filtered (stream, "User command \"%s%s\":\n", prefix, name);
>  
> +  if (!cmdlines)
> +    return;
>    print_command_lines (current_uiout, cmdlines, 1);
>    fputs_filtered ("\n", stream);
>  }
> diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp
> index 7363420..b2d1c76 100644
> --- a/gdb/testsuite/gdb.base/commands.exp
> +++ b/gdb/testsuite/gdb.base/commands.exp
> @@ -243,6 +243,28 @@ proc user_defined_command_test {} {
>     gdb_test "show user mycommand" \
>  	"  while \\\$arg0.*set.*    if \\\(\\\$arg0.*p/x.*    else\[^\n\].*p/x.*    end\[^\n\].*  end\[^\n\].*" \
>  	   "display user command in user_defined_command_test"
> +
> +    #  Create and test an user-defined command with an empty body.

s/an user/a user/

> +    gdb_test_multiple "define myemptycommand" \
> +	"define myemptycommand in user_defined_command_test" {
> +	    -re "End with"  {
> +		pass "define myemptycommand in user_defined_command_test"
> +	    }
> +	}
> +    gdb_test "end" \
> +	"" \
> +	"End definition of user-defined command with empty body."

lowercase "end".  No period at the end.

> +
> +    gdb_test_no_output "myemptycommand" \
> +	"execute user-defined empty command in user_defined_command_test"
> +
> +    gdb_test "show user" \
> +	"User command \"myemptycommand.*" \
> +	"display empty command in command list in user_defined_command_test"
> +
> +    gdb_test "show user myemptycommand" \
> +	"User command \"myemptycommand.*" \
> +	"display user-defined emtpy command in user_defined_command_test"

Typo: "empty"

>  proc watchpoint_command_test {} {
> diff --git a/gdb/testsuite/gdb.guile/scm-cmd.exp b/gdb/testsuite/gdb.guile/scm-cmd.exp
> index a407f63..36ef03d 100644
> --- a/gdb/testsuite/gdb.guile/scm-cmd.exp
> +++ b/gdb/testsuite/gdb.guile/scm-cmd.exp
> @@ -134,6 +134,10 @@ gdb_test "help user-defined" \
>      "User-defined commands.\[\r\n\]+The commands in this class are those defined by the user.\[\r\n\]+Use the \"define\" command to define a command.\[\r\n\]+List of commands:\[\r\n\]+test-help -- Docstring\[\r\n\]+Type \"help\" followed by command name for full documentation.\[\r\n\]+Type \"apropos word\" to search for commands related to \"word\".\[\r\n\]+Command name abbreviations are allowed if unambiguous.\[\r\n\]+" \
>      "see user-defined command in `help user-defined`"
>  
> +# Make sure the command does not show up in `show user`.
> +gdb_test "show user test-help" "Not a user command\." \
> +    "Don't show user-defined scheme command in `show user command`"
> +

lowercase "don't".

>  # Test expression completion on fields.
>  
>  gdb_test_multiline "expression completion command" \
> diff --git a/gdb/testsuite/gdb.python/py-cmd.exp b/gdb/testsuite/gdb.python/py-cmd.exp
> index a87aecb..3ae4053 100644
> --- a/gdb/testsuite/gdb.python/py-cmd.exp
> +++ b/gdb/testsuite/gdb.python/py-cmd.exp
> @@ -161,6 +161,10 @@ gdb_test "test_help ugh" "test_cmd output, arg = ugh" "call simple user-defined
>  # Make sure the command shows up in `help user-defined`.
>  gdb_test "help user-defined" "User-defined commands.\[\r\n\]+The commands in this class are those defined by the user.\[\r\n\]+Use the \"define\" command to define a command.\[\r\n\]+\[\r\n\]+List of commands:\[\r\n\]+\[\r\n\]+test_help -- Docstring\[\r\n\]+\[\r\n\]+Type \"help\" followed by command name for full documentation.\[\r\n\]+Type \"apropos word\" to search for commands related to \"word\".\[\r\n\]+Command name abbreviations are allowed if unambiguous.\[\r\n\]+" "see user-defined command in `help user-defined`"
>  
> +# Make sure the command does not show up in `show user`.
> +gdb_test "show user test_help" "Not a user command\." \
> +    "Don't show user-defined python command in `show user command`"

lowercase "don't".

Thanks,
Pedro Alves

Re: [PATCH] Fix PR gdb/17035: "show user" doesn't list user-defined commands that have empty bodies.

[Gabriel Krisman Bertazi]

[-- Attachment #1: Type: text/plain, Size: 1091 bytes --]

Pedro Alves <palves@redhat.com> writes:

> Yeah.  Close now.  Only nits on the details.

Guess I fixed all the things you pointed out.

> cli_user_command_p (struct cmd_list_element *cmd)

I implemented that as well.  Thanks.

Updated patch below. :)

gdb/
2014-08-20  Gabriel Krisman Bertazi  <gabriel@krisman.be>

	* cli/cli-cmds.c (show_user): Use cli_user_command_p to
	  decide whether we display the command on "show user".
	* cli/cli-script.c (show_user_1): Only verify cmdlines after
	  printing command name.
	* cli/cli-decode.h (cli_user_command_p): Declare new function.
	* cli/cli-decode.c (cli_user_command_p): Create helper function
	  to verify whether cmd_list_element is a user-defined command.

gdb/testsuite/
2014-08-20  Gabriel Krisman Bertazi  <gabriel@krisman.be>

	* gdb.base/commands.exp: Add tests to verify user-defined
	  commands with empty bodies.
	* gdb.python/py-cmd.exp: Test that we don't show user-defined
	  python commands in `show user command`.
	* gdb.python/scm-cmd.exp: Test that we don't show user-defined
	  scheme commands in `show user command`.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: show_user_v3.patch --]
[-- Type: text/x-patch, Size: 5744 bytes --]

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index b415267..b0f1bdf 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -1245,8 +1245,7 @@ show_user (char *args, int from_tty)
       const char *comname = args;
 
       c = lookup_cmd (&comname, cmdlist, "", 0, 1);
-      /* c->user_commands would be NULL if it's a python/scheme command.  */
-      if (c->class != class_user || !c->user_commands)
+      if (!cli_user_command_p (c))
 	error (_("Not a user command."));
       show_user_1 (c, "", args, gdb_stdout);
     }
@@ -1254,7 +1253,7 @@ show_user (char *args, int from_tty)
     {
       for (c = cmdlist; c; c = c->next)
 	{
-	  if (c->class == class_user || c->prefixlist != NULL)
+	  if (cli_user_command_p (c) || c->prefixlist != NULL)
 	    show_user_1 (c, "", c->name, gdb_stdout);
 	}
     }
diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index c409d9c..914a8e3 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -1884,3 +1884,10 @@ cmd_func (struct cmd_list_element *cmd, char *args, int from_tty)
   else
     error (_("Invalid command"));
 }
+
+int
+cli_user_command_p (struct cmd_list_element *cmd)
+{
+  return (cmd->class == class_user
+	  && (cmd->func == do_cfunc || cmd->func == do_sfunc));
+}
diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
index 865d4a0..41ce299 100644
--- a/gdb/cli/cli-decode.h
+++ b/gdb/cli/cli-decode.h
@@ -233,4 +233,9 @@ extern void print_doc_line (struct ui_file *, const char *);
 
 extern const char * const auto_boolean_enums[];
 
+/* Verify whether a given cmd_list_element is a user-defined command.
+   Return 1 if it is user-defined.  Return 0 otherwise.  */
+
+int cli_user_command_p (struct cmd_list_element *);
+
 #endif /* !defined (CLI_DECODE_H) */
diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index 0f0a97e..37cb82a 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -1717,10 +1717,10 @@ show_user_1 (struct cmd_list_element *c, const char *prefix, const char *name,
     }
 
   cmdlines = c->user_commands;
-  if (!cmdlines)
-    return;
   fprintf_filtered (stream, "User command \"%s%s\":\n", prefix, name);
 
+  if (!cmdlines)
+    return;
   print_command_lines (current_uiout, cmdlines, 1);
   fputs_filtered ("\n", stream);
 }
diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp
index 7363420..74eb306 100644
--- a/gdb/testsuite/gdb.base/commands.exp
+++ b/gdb/testsuite/gdb.base/commands.exp
@@ -243,6 +243,28 @@ proc user_defined_command_test {} {
    gdb_test "show user mycommand" \
 	"  while \\\$arg0.*set.*    if \\\(\\\$arg0.*p/x.*    else\[^\n\].*p/x.*    end\[^\n\].*  end\[^\n\].*" \
 	   "display user command in user_defined_command_test"
+
+    # Create and test a user-defined command with an empty body.
+    gdb_test_multiple "define myemptycommand" \
+	"define myemptycommand in user_defined_command_test" {
+	    -re "End with"  {
+		pass "define myemptycommand in user_defined_command_test"
+	    }
+	}
+    gdb_test "end" \
+	"" \
+	"end definition of user-defined command with empty body"
+
+    gdb_test_no_output "myemptycommand" \
+	"execute user-defined empty command in user_defined_command_test"
+
+    gdb_test "show user" \
+	"User command \"myemptycommand.*" \
+	"display empty command in command list in user_defined_command_test"
+
+    gdb_test "show user myemptycommand" \
+	"User command \"myemptycommand.*" \
+	"display user-defined empty command in user_defined_command_test"
 }
 
 proc watchpoint_command_test {} {
diff --git a/gdb/testsuite/gdb.guile/scm-cmd.exp b/gdb/testsuite/gdb.guile/scm-cmd.exp
index a407f63..13ce9c2 100644
--- a/gdb/testsuite/gdb.guile/scm-cmd.exp
+++ b/gdb/testsuite/gdb.guile/scm-cmd.exp
@@ -134,6 +134,10 @@ gdb_test "help user-defined" \
     "User-defined commands.\[\r\n\]+The commands in this class are those defined by the user.\[\r\n\]+Use the \"define\" command to define a command.\[\r\n\]+List of commands:\[\r\n\]+test-help -- Docstring\[\r\n\]+Type \"help\" followed by command name for full documentation.\[\r\n\]+Type \"apropos word\" to search for commands related to \"word\".\[\r\n\]+Command name abbreviations are allowed if unambiguous.\[\r\n\]+" \
     "see user-defined command in `help user-defined`"
 
+# Make sure the command does not show up in `show user`.
+gdb_test "show user test-help" "Not a user command\." \
+    "don't show user-defined scheme command in `show user command`"
+
 # Test expression completion on fields.
 
 gdb_test_multiline "expression completion command" \
diff --git a/gdb/testsuite/gdb.python/py-cmd.exp b/gdb/testsuite/gdb.python/py-cmd.exp
index a87aecb..c84401f 100644
--- a/gdb/testsuite/gdb.python/py-cmd.exp
+++ b/gdb/testsuite/gdb.python/py-cmd.exp
@@ -161,6 +161,10 @@ gdb_test "test_help ugh" "test_cmd output, arg = ugh" "call simple user-defined
 # Make sure the command shows up in `help user-defined`.
 gdb_test "help user-defined" "User-defined commands.\[\r\n\]+The commands in this class are those defined by the user.\[\r\n\]+Use the \"define\" command to define a command.\[\r\n\]+\[\r\n\]+List of commands:\[\r\n\]+\[\r\n\]+test_help -- Docstring\[\r\n\]+\[\r\n\]+Type \"help\" followed by command name for full documentation.\[\r\n\]+Type \"apropos word\" to search for commands related to \"word\".\[\r\n\]+Command name abbreviations are allowed if unambiguous.\[\r\n\]+" "see user-defined command in `help user-defined`"
 
+# Make sure the command does not show up in `show user`.
+gdb_test "show user test_help" "Not a user command\." \
+    "don't show user-defined python command in `show user command`"
+
 # Test expression completion on fields
 gdb_py_test_multiple "expression completion command" \
   "python" "" \

[-- Attachment #3: Type: text/plain, Size: 29 bytes --]


-- 
Gabriel Krisman Bertazi

Re: [PATCH] Fix PR gdb/17035: "show user" doesn't list user-defined commands that have empty bodies.

[Sergio Durigan Junior]

On Thursday, September 04 2014, Gabriel Krisman Bertazi wrote:

> diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
> index c409d9c..914a8e3 100644
> --- a/gdb/cli/cli-decode.c
> +++ b/gdb/cli/cli-decode.c
> @@ -1884,3 +1884,10 @@ cmd_func (struct cmd_list_element *cmd, char *args, int from_tty)
>    else
>      error (_("Invalid command"));
>  }
> +
> +int
> +cli_user_command_p (struct cmd_list_element *cmd)
> +{
> +  return (cmd->class == class_user
> +	  && (cmd->func == do_cfunc || cmd->func == do_sfunc));
> +}

Thanks for the patch.

You correctly commented the function prototype on cli/cli-decode.h, but
it is also a good practice to put a comment in the function definition
as well, like:

  /* See declaration on cli/cli-decode.h.  */
  int
  cli_user_command_p (struct cmd_list_element *cmd)
  ...

Other than that, looks OK.

Cheers,

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: [PATCH] Fix PR gdb/17035: "show user" doesn't list user-defined commands that have empty bodies.

[Pedro Alves]

On 09/04/2014 08:26 PM, Gabriel Krisman Bertazi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> Yeah.  Close now.  Only nits on the details.
> 
> Guess I fixed all the things you pointed out.

Excellent!

> 
>> cli_user_command_p (struct cmd_list_element *cmd)
> 
> I implemented that as well.  Thanks.
> 
> Updated patch below. :)

Thanks!

Ideally you'd send a 'git am'able patch, with commit log
in place too.  That is, treat the commit log as just
another part of the patch that gets updated/resent.

> 
> gdb/
> 2014-08-20  Gabriel Krisman Bertazi  <gabriel@krisman.be>
> 
> 	* cli/cli-cmds.c (show_user): Use cli_user_command_p to
> 	  decide whether we display the command on "show user".

Note that the "decide" should be indented with a tab only, under
the '*'.

> 	* cli/cli-script.c (show_user_1): Only verify cmdlines after
> 	  printing command name.
> 	* cli/cli-decode.h (cli_user_command_p): Declare new function.
> 	* cli/cli-decode.c (cli_user_command_p): Create helper function
> 	  to verify whether cmd_list_element is a user-defined command.
> 
> gdb/testsuite/
> 2014-08-20  Gabriel Krisman Bertazi  <gabriel@krisman.be>
> 
> 	* gdb.base/commands.exp: Add tests to verify user-defined
> 	  commands with empty bodies.
> 	* gdb.python/py-cmd.exp: Test that we don't show user-defined
> 	  python commands in `show user command`.
> 	* gdb.python/scm-cmd.exp: Test that we don't show user-defined
> 	  scheme commands in `show user command`.

Likewise everywhere else.

>  extern const char * const auto_boolean_enums[];
>
> +/* Verify whether a given cmd_list_element is a user-defined command.
> +   Return 1 if it is user-defined.  Return 0 otherwise.  */
> +
> +int cli_user_command_p (struct cmd_list_element *);

Note that every other declaration in the header uses explicit
"extern".  Please add that for consistency.  OK with these
little nits fixed.  Please push.

Thanks!

Pedro Alves

Re: [PATCH] Fix PR gdb/17035: "show user" doesn't list user-defined commands that have empty bodies.

[Gabriel Krisman Bertazi]

[-- Attachment #1: Type: text/plain, Size: 332 bytes --]

Pedro Alves <palves@redhat.com> writes:

> Note that every other declaration in the header uses explicit
> "extern".  Please add that for consistency.  OK with these
> little nits fixed.  Please push.
>

Pedro,

Thanks, pushed.

<https://sourceware.org/ml/gdb-cvs/2014-09/msg00021.html>.

-- 
Gabriel Krisman Bertazi

[-- Attachment #2: Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH] Fix PR gdb/17035: "show user" doesn't list user-defined commands that have empty bodies.

[Sergio Durigan Junior]

On Sunday, September 07 2014, Gabriel Krisman Bertazi wrote:

> Thanks, pushed.
>
> <https://sourceware.org/ml/gdb-cvs/2014-09/msg00021.html>.

Hey,

Now that you can push things to the repository, you have to add yourself
to the gdb/MAINTAINERS file, in the Write After Approval section.

No need to ask for permission for this patch, but please send an FYI to
the list and include a ChangeLog entry as well.

Cheers,

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/