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

Message ID 87a96frxu5.fsf@anubis.Home
State New, archived
Headers

Commit Message

Gabriel Krisman Bertazi Sept. 4, 2014, 7:26 p.m. UTC
  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`.
  

Comments

Sergio Durigan Junior Sept. 4, 2014, 7:57 p.m. UTC | #1
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,
  
Pedro Alves Sept. 5, 2014, 1:12 p.m. UTC | #2
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
  
Gabriel Krisman Bertazi Sept. 8, 2014, 12:44 a.m. UTC | #3
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>.
  
Sergio Durigan Junior Sept. 9, 2014, 9:19 p.m. UTC | #4
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,
  

Patch

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" "" \