Fix PR gdb/17035: "show user" doesn't list user-defined commands that have empty bodies.
Commit Message
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(-)
Comments
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
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,
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,
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
@@ -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
@@ -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);
}
@@ -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 {} {
@@ -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
@@ -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