[PATCH 0/7] Minor improvements to cmd_list_element

[PATCH 0/7] Minor improvements to cmd_list_element

[Simon Marchi]

While trying to fix some bug, I read a lot of the command-related code,
hoping to find ways to make it cleaner and simpler.  I didn't really
succeed, but I managed to gather these small improvements that I think
slightly help readability.

Simon Marchi (7):
  gdb: move cmd_list_element::prefixname to cli/cli-decode.c
  gdb: don't handle old == nullptr in add_alias_cmd
  gdb: rename cmd_list_element::prefixlist to subcommands
  gdb: rename cmd_list_element::cmd_pointer to target
  gdb: add cmd_list_element::is_alias
  gdb: add cmd_list_element::is_prefix
  gdb: add cmd_list_element::is_command_class_help

 gdb/auto-load.c                       |   2 +-
 gdb/cli/cli-cmds.c                    |   6 +-
 gdb/cli/cli-decode.c                  | 217 +++++++++++++-------------
 gdb/cli/cli-decode.h                  |  34 ++--
 gdb/cli/cli-script.c                  |  30 ++--
 gdb/cli/cli-setshow.c                 |  12 +-
 gdb/command.h                         |   5 +-
 gdb/completer.c                       |   8 +-
 gdb/guile/scm-cmd.c                   |   4 +-
 gdb/python/py-cmd.c                   |   6 +-
 gdb/top.c                             |   6 +-
 gdb/unittests/command-def-selftests.c |   8 +-
 12 files changed, 166 insertions(+), 172 deletions(-)

-- 
2.31.1

[PATCH 1/7] gdb: move cmd_list_element::prefixname to cli/cli-decode.c

[Simon Marchi]

I don't think this method really benefits from being implemented in the
header file, especially because it's recursive, it can't be inlined.
Move it to the source file, so it's no re-compiled by every CU
including cli/cli-decode.h.

I also noticed this method could be const, make it so.

gdb/ChangeLog:

	* cli/cli-decode.h (prefixname): Make const, move implementation
	to cli/cli-decode.c.
	* cli/cli-decode.c (cmd_list_element::prefixname): New.

Change-Id: I1597cace98d9a4ba71f51f1f495e73cc07b5dcf3
---
 gdb/cli/cli-decode.c | 17 +++++++++++++++++
 gdb/cli/cli-decode.h | 16 ++--------------
 2 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 32edb526c875..a3b153f06e48 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -160,6 +160,23 @@ set_cmd_completer_handle_brkchars (struct cmd_list_element *cmd,
   cmd->completer_handle_brkchars = func;
 }
 
+std::string
+cmd_list_element::prefixname () const
+{
+  if (this->prefixlist == nullptr)
+    /* Not a prefix command.  */
+    return "";
+
+  std::string prefixname;
+  if (this->prefix != nullptr)
+    prefixname = this->prefix->prefixname ();
+
+  prefixname += this->name;
+  prefixname += " ";
+
+  return prefixname;
+}
+
 /* Add element named NAME.
    Space for NAME and DOC must be allocated by the caller.
    CLASS is the top level category into which commands are broken down
diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
index da6013a6cc52..e6d6f32cbfa9 100644
--- a/gdb/cli/cli-decode.h
+++ b/gdb/cli/cli-decode.h
@@ -76,20 +76,8 @@ struct cmd_list_element
      space.  It is used before the word "command" in describing the
      commands reached through this prefix.
 
-     For non-prefix commands, an empty string is returned.  */
-  std::string prefixname ()
-  {
-    if (prefixlist == nullptr)
-      /* Not a prefix command.  */
-      return "";
-
-    std::string prefixname;
-    if (prefix != nullptr)
-      prefixname = prefix->prefixname ();
-    prefixname += name;
-    prefixname += " ";
-    return prefixname;
-  }
+     For non-prefix commands, return an empty string.  */
+  std::string prefixname () const;
 
   /* Points to next command in this list.  */
   struct cmd_list_element *next = nullptr;
-- 
2.31.1

[PATCH 2/7] gdb: don't handle old == nullptr in add_alias_cmd

[Simon Marchi]

I don't think this can ever happen, that we add an alias command and
pass a nullptr old (target) command.  Remove the "if" handling this,
replace with an assert.

gdb/ChangeLog:

	* cli/cli-decode.c (add_alias_cmd): Don't handle old == 0.

Change-Id: Ibb39e8dc4e0c465fa42e6826215f30a0a0aef932
---
 gdb/cli/cli-decode.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index a3b153f06e48..1bfc9477905a 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -310,18 +310,7 @@ add_alias_cmd (const char *name, cmd_list_element *old,
 	       enum command_class theclass, int abbrev_flag,
 	       struct cmd_list_element **list)
 {
-  if (old == 0)
-    {
-      struct cmd_list_element *prehook, *prehookee, *posthook, *posthookee;
-      struct cmd_list_element *aliases = delete_cmd (name, list,
-						     &prehook, &prehookee,
-						     &posthook, &posthookee);
-
-      /* If this happens, it means a programmer error somewhere.  */
-      gdb_assert (!aliases && !prehook && !prehookee
-		  && !posthook && ! posthookee);
-      return 0;
-    }
+  gdb_assert (old != nullptr);
 
   struct cmd_list_element *c = add_cmd (name, theclass, old->doc, list);
 
-- 
2.31.1

Re: [PATCH 2/7] gdb: don't handle old == nullptr in add_alias_cmd

[Tom Tromey]

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> I don't think this can ever happen, that we add an alias command and
Simon> pass a nullptr old (target) command.  Remove the "if" handling this,
Simon> replace with an assert.

I am not sure this can't happen due to user input.

This overload:

    struct cmd_list_element *
    add_alias_cmd (const char *name, const char *oldname,

does:

  old = lookup_cmd (&tmp, *list, "", NULL, 1, 1);

  return add_alias_cmd (name, old, theclass, abbrev_flag, list);

But when allow_unknown==1, I think lookup_cmd can return null.

Maybe that call ought to be changed to pass 0.  I am not really sure.

Tom

Re: [PATCH 2/7] gdb: don't handle old == nullptr in add_alias_cmd

[Simon Marchi]

On 2021-05-16 9:37 a.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> I don't think this can ever happen, that we add an alias command and
> Simon> pass a nullptr old (target) command.  Remove the "if" handling this,
> Simon> replace with an assert.
> 
> I am not sure this can't happen due to user input.

What do you mean by user input here?  The user of GDB, or the user /
caller of this function?

> 
> This overload:
> 
>     struct cmd_list_element *
>     add_alias_cmd (const char *name, const char *oldname,
> 
> does:
> 
>   old = lookup_cmd (&tmp, *list, "", NULL, 1, 1);
> 
>   return add_alias_cmd (name, old, theclass, abbrev_flag, list);
> 
> But when allow_unknown==1, I think lookup_cmd can return null.
> 
> Maybe that call ought to be changed to pass 0.  I am not really sure.

From all I could find, there is never the intent to call add_alias_cmd
and pass a command name that doesn't exist.  Calls to this function
always come from either

 - the "alias" command, but it will check that the target command exists
 - hard-coded calls in _initialize functions

I'm a bit afraid that it would be possible for a command to be added in
a source file and an alias for that command to be added in another
source file.  In that case, it would be possible for NULL to be passed.
However, given that I've made it an assert and GDB is still able to
start up means that it doesn't happen with the current order the
_initialize functions are called (of course, I couldn't test all
configurations on all native platforms).  And that order is stable, it's
based on the order the files are listed in to the Makefile.

But when I look a bit closer at the removed code: if the old command is
NULL when trying to add an alias, then we just remove any existing
command with the alias' name, and that's it.  So the end result of
trying to alias "foo" to "bar" when "bar" doesn't exist is to delete any
existing command called "foo".  That doesn't seem very useful to me.

Still, to see if there is such hidden dependencies in the order in which
_initialize functions are called, I reversed the order in which
_initialize functions are called in init.c.  Doing so breaks GDB because
"maintenance flush-symbol-cache" is aliased to "maintenance flush
symbol-cache" (in symtab.c) before the "maintenance flush" command is
created (in maint.c).  So the lookup_cmd for "maintenance flush
symbol-cache fails" / returns NULL.  If that happened for real, it just
means we'd (silently) be left without a "maintenance flush-symbol-cache"
command, I don't find this very useful.

So, I don't think we support (or want to support) creating an alias for
a command that doesn't exist (yet).  But, it's still possible for a
prefix of the target command to not exist yet (we do support creating a
command before its prefix).  To address that, I think that all
add_alias_cmd calls should be migrated to the overload that takes a
`cmd_list_element *`, since that doesn't require doing a lookup_cmd for
the target command.

Also, note that even without my patch, reversing _initialize calls would
break anyway, when we try to deprecate that command:

  deprecate_cmd (c, "maintenancelist flush symbol-cache");

deprecate_cmd doesn't expect `c` to be NULL.

Simon

Re: [PATCH 2/7] gdb: don't handle old == nullptr in add_alias_cmd

[Tom Tromey]

>> I am not sure this can't happen due to user input.

Simon> What do you mean by user input here?  The user of GDB, or the user /
Simon> caller of this function?

I meant if the user types some alias command, since I didn't look at
the implementation of it...

Simon> From all I could find, there is never the intent to call add_alias_cmd
Simon> and pass a command name that doesn't exist.  Calls to this function
Simon> always come from either

Simon>  - the "alias" command, but it will check that the target command exists
Simon>  - hard-coded calls in _initialize functions

.. which you did.

Simon> I'm a bit afraid that it would be possible for a command to be added in
Simon> a source file and an alias for that command to be added in another
Simon> source file.  In that case, it would be possible for NULL to be
Simon> passed.

Yeah, if the assert occurs during gdb startup, then it is fine.  This
just means there's a programming error, not a failure to validate input
somewhere.

Simon>   So the end result of
Simon> trying to alias "foo" to "bar" when "bar" doesn't exist is to delete any
Simon> existing command called "foo".  That doesn't seem very useful to me.

Me either.

Thanks for looking so deeply into this.

Tom

[PATCH 3/7] gdb: rename cmd_list_element::prefixlist to subcommands

[Simon Marchi]

While browsing this code, I found the name "prefixlist" really
confusing.  I kept reading it as "list of prefixes".  Which it isn't:
it's a list of sub-commands, for a prefix command.  I think that
renaming it to "subcommands" would make things clearer.

gdb/ChangeLog:

	* Rename "prefixlist" parameters to "subcommands" throughout.
	* cli/cli-decode.h (cmd_list_element) <prefixlist>: Rename to...
	<subcommands>: ... this.
	* cli/cli-decode.c (lookup_cmd_for_prefixlist): Rename to...
	(lookup_cmd_with_subcommands): ... this.

Change-Id: I150da10d03052c2420aa5b0dee41f422e2a97928
---
 gdb/auto-load.c                       |  2 +-
 gdb/cli/cli-cmds.c                    |  6 +-
 gdb/cli/cli-decode.c                  | 91 +++++++++++++--------------
 gdb/cli/cli-decode.h                  |  2 +-
 gdb/cli/cli-script.c                  | 30 ++++-----
 gdb/cli/cli-setshow.c                 | 10 +--
 gdb/command.h                         |  2 +-
 gdb/completer.c                       |  8 +--
 gdb/guile/scm-cmd.c                   |  4 +-
 gdb/python/py-cmd.c                   |  6 +-
 gdb/top.c                             |  4 +-
 gdb/unittests/command-def-selftests.c |  8 +--
 12 files changed, 86 insertions(+), 87 deletions(-)

diff --git a/gdb/auto-load.c b/gdb/auto-load.c
index b9478c45c534..7745aa65b036 100644
--- a/gdb/auto-load.c
+++ b/gdb/auto-load.c
@@ -1465,7 +1465,7 @@ info_auto_load_cmd (const char *args, int from_tty)
     {
       ui_out_emit_tuple option_emitter (uiout, "option");
 
-      gdb_assert (!list->prefixlist);
+      gdb_assert (!list->subcommands);
       gdb_assert (list->type == not_set_cmd);
 
       uiout->field_string ("name", list->name);
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 284b75192581..c29e5979acb4 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -1625,7 +1625,7 @@ show_user (const char *args, int from_tty)
     {
       for (c = cmdlist; c; c = c->next)
 	{
-	  if (cli_user_command_p (c) || c->prefixlist != NULL)
+	  if (cli_user_command_p (c) || c->subcommands != NULL)
 	    show_user_1 (c, "", c->name, gdb_stdout);
 	}
     }
@@ -1900,7 +1900,7 @@ alias_command (const char *args, int from_tty)
       /* We've already tried to look up COMMAND.  */
       gdb_assert (c_command != NULL
 		  && c_command != (struct cmd_list_element *) -1);
-      gdb_assert (c_command->prefixlist != NULL);
+      gdb_assert (c_command->subcommands != NULL);
       c_alias = lookup_cmd_1 (& alias_prefix, cmdlist, NULL, NULL, 1);
       if (c_alias != c_command)
 	error (_("ALIAS and COMMAND prefixes do not match."));
@@ -1909,7 +1909,7 @@ alias_command (const char *args, int from_tty)
       alias_cmd = add_alias_cmd (xstrdup (alias_argv[alias_argc - 1]),
 				 command_argv[command_argc - 1],
 				 class_alias, a_opts.abbrev_flag,
-				 c_command->prefixlist);
+				 c_command->subcommands);
     }
 
   gdb_assert (alias_cmd != nullptr);
diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 1bfc9477905a..27bd311e89c1 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -56,12 +56,12 @@ static int lookup_cmd_composition_1 (const char *text,
 				     struct cmd_list_element **cmd,
 				     struct cmd_list_element *cur_list);
 
-/* Look up a command whose 'prefixlist' is KEY.  Return the command if found,
-   otherwise return NULL.  */
+/* Look up a command whose 'subcommands' field is SUBCOMMANDS.  Return the
+   command if found, otherwise return NULL.  */
 
 static struct cmd_list_element *
-lookup_cmd_for_prefixlist (struct cmd_list_element **key,
-			   struct cmd_list_element *list)
+lookup_cmd_with_subcommands (cmd_list_element **subcommands,
+			     cmd_list_element *list)
 {
   struct cmd_list_element *p = NULL;
 
@@ -69,16 +69,16 @@ lookup_cmd_for_prefixlist (struct cmd_list_element **key,
     {
       struct cmd_list_element *q;
 
-      if (p->prefixlist == NULL)
+      if (p->subcommands == NULL)
 	continue;
-      else if (p->prefixlist == key)
+      else if (p->subcommands == subcommands)
 	{
 	  /* If we found an alias, we must return the aliased
 	     command.  */
 	  return p->cmd_pointer ? p->cmd_pointer : p;
 	}
 
-      q = lookup_cmd_for_prefixlist (key, *(p->prefixlist));
+      q = lookup_cmd_with_subcommands (subcommands, *(p->subcommands));
       if (q != NULL)
 	return q;
     }
@@ -163,7 +163,7 @@ set_cmd_completer_handle_brkchars (struct cmd_list_element *cmd,
 std::string
 cmd_list_element::prefixname () const
 {
-  if (this->prefixlist == nullptr)
+  if (this->subcommands == nullptr)
     /* Not a prefix command.  */
     return "";
 
@@ -236,8 +236,7 @@ do_add_cmd (const char *name, enum command_class theclass,
 
   /* Search the prefix cmd of C, and assigns it to C->prefix.
      See also add_prefix_cmd and update_prefix_field_of_prefixed_commands.  */
-  struct cmd_list_element *prefixcmd = lookup_cmd_for_prefixlist (list,
-								  cmdlist);
+  cmd_list_element *prefixcmd = lookup_cmd_with_subcommands (list, cmdlist);
   c->prefix = prefixcmd;
 
 
@@ -323,7 +322,7 @@ add_alias_cmd (const char *name, cmd_list_element *old,
   /* NOTE: Both FUNC and all the FUNCTIONs need to be copied.  */
   c->func = old->func;
   c->function = old->function;
-  c->prefixlist = old->prefixlist;
+  c->subcommands = old->subcommands;
   c->allow_unknown = old->allow_unknown;
   c->abbrev_flag = abbrev_flag;
   c->cmd_pointer = old;
@@ -358,7 +357,7 @@ add_alias_cmd (const char *name, const char *oldname,
 static void
 update_prefix_field_of_prefixed_commands (struct cmd_list_element *c)
 {
-  for (cmd_list_element *p = *c->prefixlist; p != NULL; p = p->next)
+  for (cmd_list_element *p = *c->subcommands; p != NULL; p = p->next)
     {
       p->prefix = c;
 
@@ -371,9 +370,9 @@ update_prefix_field_of_prefixed_commands (struct cmd_list_element *c)
 	 In such a case, when 'auto-load' was created by do_add_cmd,
 	 the 'libthread-db' prefix field could not be updated, as the
 	 'auto-load' command was not yet reachable by
-	    lookup_cmd_for_prefixlist (list, cmdlist)
+	    lookup_cmd_for_subcommands (list, cmdlist)
 	    that searches from the top level 'cmdlist'.  */
-      if (p->prefixlist != nullptr)
+      if (p->subcommands != nullptr)
 	update_prefix_field_of_prefixed_commands (p);
     }
 }
@@ -381,18 +380,18 @@ update_prefix_field_of_prefixed_commands (struct cmd_list_element *c)
 
 /* Like add_cmd but adds an element for a command prefix: a name that
    should be followed by a subcommand to be looked up in another
-   command list.  PREFIXLIST should be the address of the variable
+   command list.  SUBCOMMANDS should be the address of the variable
    containing that list.  */
 
 struct cmd_list_element *
 add_prefix_cmd (const char *name, enum command_class theclass,
 		cmd_const_cfunc_ftype *fun,
-		const char *doc, struct cmd_list_element **prefixlist,
+		const char *doc, struct cmd_list_element **subcommands,
 		int allow_unknown, struct cmd_list_element **list)
 {
   struct cmd_list_element *c = add_cmd (name, theclass, fun, doc, list);
 
-  c->prefixlist = prefixlist;
+  c->subcommands = subcommands;
   c->allow_unknown = allow_unknown;
 
   /* Now that prefix command C is defined, we need to set the prefix field
@@ -412,7 +411,7 @@ do_prefix_cmd (const char *args, int from_tty, struct cmd_list_element *c)
   while (c->cmd_pointer != nullptr)
     c = c->cmd_pointer;
 
-  help_list (*c->prefixlist, c->prefixname ().c_str (),
+  help_list (*c->subcommands, c->prefixname ().c_str (),
 	     all_commands, gdb_stdout);
 }
 
@@ -420,11 +419,11 @@ do_prefix_cmd (const char *args, int from_tty, struct cmd_list_element *c)
 
 struct cmd_list_element *
 add_basic_prefix_cmd (const char *name, enum command_class theclass,
-		      const char *doc, struct cmd_list_element **prefixlist,
+		      const char *doc, struct cmd_list_element **subcommands,
 		      int allow_unknown, struct cmd_list_element **list)
 {
   struct cmd_list_element *cmd = add_prefix_cmd (name, theclass, nullptr,
-						 doc, prefixlist,
+						 doc, subcommands,
 						 allow_unknown, list);
   set_cmd_sfunc (cmd, do_prefix_cmd);
   return cmd;
@@ -436,18 +435,18 @@ add_basic_prefix_cmd (const char *name, enum command_class theclass,
 static void
 do_show_prefix_cmd (const char *args, int from_tty, struct cmd_list_element *c)
 {
-  cmd_show_list (*c->prefixlist, from_tty);
+  cmd_show_list (*c->subcommands, from_tty);
 }
 
 /* See command.h.  */
 
 struct cmd_list_element *
 add_show_prefix_cmd (const char *name, enum command_class theclass,
-		     const char *doc, struct cmd_list_element **prefixlist,
+		     const char *doc, struct cmd_list_element **subcommands,
 		     int allow_unknown, struct cmd_list_element **list)
 {
   struct cmd_list_element *cmd = add_prefix_cmd (name, theclass, nullptr,
-						 doc, prefixlist,
+						 doc, subcommands,
 						 allow_unknown, list);
   set_cmd_sfunc (cmd, do_show_prefix_cmd);
   return cmd;
@@ -460,12 +459,12 @@ struct cmd_list_element *
 add_prefix_cmd_suppress_notification
 	       (const char *name, enum command_class theclass,
 		cmd_const_cfunc_ftype *fun,
-		const char *doc, struct cmd_list_element **prefixlist,
+		const char *doc, struct cmd_list_element **subcommands,
 		int allow_unknown, struct cmd_list_element **list,
 		int *suppress_notification)
 {
   struct cmd_list_element *element
-    = add_prefix_cmd (name, theclass, fun, doc, prefixlist,
+    = add_prefix_cmd (name, theclass, fun, doc, subcommands,
 		      allow_unknown, list);
   element->suppress_notification = suppress_notification;
   return element;
@@ -476,12 +475,12 @@ add_prefix_cmd_suppress_notification
 struct cmd_list_element *
 add_abbrev_prefix_cmd (const char *name, enum command_class theclass,
 		       cmd_const_cfunc_ftype *fun, const char *doc,
-		       struct cmd_list_element **prefixlist,
+		       struct cmd_list_element **subcommands,
 		       int allow_unknown, struct cmd_list_element **list)
 {
   struct cmd_list_element *c = add_cmd (name, theclass, fun, doc, list);
 
-  c->prefixlist = prefixlist;
+  c->subcommands = subcommands;
   c->allow_unknown = allow_unknown;
   c->abbrev_flag = 1;
   return c;
@@ -1188,11 +1187,11 @@ apropos_cmd (struct ui_file *stream,
 	    print_doc_of_command (c, prefix, verbose, regex, stream);
 	}
       /* Check if this command has subcommands.  */
-      if (c->prefixlist != NULL)
+      if (c->subcommands != NULL)
 	{
 	  /* Recursively call ourselves on the subcommand list,
 	     passing the right prefix in.  */
-	  apropos_cmd (stream, *c->prefixlist, verbose, regex,
+	  apropos_cmd (stream, *c->subcommands, verbose, regex,
 		       c->prefixname ().c_str ());
 	}
     }
@@ -1235,7 +1234,7 @@ help_cmd (const char *command, struct ui_file *stream)
   lookup_cmd_composition (orig_command, &alias, &prefix_cmd, &c_cmd);
 
   /* There are three cases here.
-     If c->prefixlist is nonzero, we have a prefix command.
+     If c->subcommands is nonzero, we have a prefix command.
      Print its documentation, then list its subcommands.
 
      If c->func is non NULL, we really have a command.  Print its
@@ -1253,13 +1252,13 @@ help_cmd (const char *command, struct ui_file *stream)
   fputs_filtered (c->doc, stream);
   fputs_filtered ("\n", stream);
 
-  if (c->prefixlist == 0 && c->func != NULL)
+  if (c->subcommands == 0 && c->func != NULL)
     return;
   fprintf_filtered (stream, "\n");
 
   /* If this is a prefix command, print it's subcommands.  */
-  if (c->prefixlist)
-    help_list (*c->prefixlist, c->prefixname ().c_str (),
+  if (c->subcommands)
+    help_list (*c->subcommands, c->prefixname ().c_str (),
 	       all_commands, stream);
 
   /* If this is a class name, print all of the commands in the class.  */
@@ -1450,12 +1449,12 @@ print_help_for_command (struct cmd_list_element *c,
   fput_aliases_definition_styled (c, stream);
 
   if (recurse
-      && c->prefixlist != 0
+      && c->subcommands != 0
       && c->abbrev_flag == 0)
     /* Subcommands of a prefix command typically have 'all_commands'
        as class.  If we pass CLASS to recursive invocation,
        most often we won't see anything.  */
-    help_cmd_list (*c->prefixlist, all_commands, true, stream);
+    help_cmd_list (*c->subcommands, all_commands, true, stream);
 }
 
 /*
@@ -1520,10 +1519,10 @@ help_cmd_list (struct cmd_list_element *list, enum command_class theclass,
 
       if (recurse
 	  && (theclass == class_user || theclass == class_alias)
-	  && c->prefixlist != NULL)
+	  && c->subcommands != NULL)
 	{
 	  /* User-defined commands or aliases may be subcommands.  */
-	  help_cmd_list (*c->prefixlist, theclass, recurse, stream);
+	  help_cmd_list (*c->subcommands, theclass, recurse, stream);
 	  continue;
 	}
 
@@ -1698,9 +1697,9 @@ lookup_cmd_1 (const char **text, struct cmd_list_element *clist,
     }
   /* If we found a prefix command, keep looking.  */
 
-  if (found->prefixlist)
+  if (found->subcommands)
     {
-      c = lookup_cmd_1 (text, *found->prefixlist, result_list, default_args,
+      c = lookup_cmd_1 (text, *found->subcommands, result_list, default_args,
 			ignore_help_classes, lookup_for_completion_p);
       if (!c)
 	{
@@ -1718,7 +1717,7 @@ lookup_cmd_1 (const char **text, struct cmd_list_element *clist,
 	     we've found (if an inferior hasn't already set it).  */
 	  if (result_list != nullptr)
 	    if (!*result_list)
-	      /* This used to say *result_list = *found->prefixlist.
+	      /* This used to say *result_list = *found->subcommands.
 		 If that was correct, need to modify the documentation
 		 at the top of this function to clarify what is
 		 supposed to be going on.  */
@@ -1810,14 +1809,14 @@ lookup_cmd (const char **line, struct cmd_list_element *list,
     }
   else if (c == CMD_LIST_AMBIGUOUS)
     {
-      /* Ambigous.  Local values should be off prefixlist or called
+      /* Ambigous.  Local values should be off subcommands or called
 	 values.  */
       int local_allow_unknown = (last_list ? last_list->allow_unknown :
 				 allow_unknown);
       std::string local_cmdtype
 	= last_list ? last_list->prefixname () : cmdtype;
       struct cmd_list_element *local_list =
-	(last_list ? *(last_list->prefixlist) : list);
+	(last_list ? *(last_list->subcommands) : list);
 
       if (local_allow_unknown < 0)
 	{
@@ -1869,7 +1868,7 @@ lookup_cmd (const char **line, struct cmd_list_element *list,
       while (**line == ' ' || **line == '\t')
 	(*line)++;
 
-      if (c->prefixlist && **line && !c->allow_unknown)
+      if (c->subcommands && **line && !c->allow_unknown)
 	undef_cmd_error (c->prefixname ().c_str (), *line);
 
       /* Seems to be what he wants.  Return it.  */
@@ -2060,9 +2059,9 @@ lookup_cmd_composition_1 (const char *text,
       text += len;
       text = skip_spaces (text);
 
-      if ((*cmd)->prefixlist != nullptr && *text != '\0')
+      if ((*cmd)->subcommands != nullptr && *text != '\0')
 	{
-	  cur_list = *(*cmd)->prefixlist;
+	  cur_list = *(*cmd)->subcommands;
 	  *prefix_cmd = *cmd;
 	}
       else
@@ -2127,7 +2126,7 @@ complete_on_cmdlist (struct cmd_list_element *list,
 	if (!strncmp (ptr->name, text, textlen)
 	    && !ptr->abbrev_flag
 	    && (!ignore_help_classes || ptr->func
-		|| ptr->prefixlist))
+		|| ptr->subcommands))
 	  {
 	    if (pass == 0)
 	      {
diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
index e6d6f32cbfa9..e2428bdddf7d 100644
--- a/gdb/cli/cli-decode.h
+++ b/gdb/cli/cli-decode.h
@@ -187,7 +187,7 @@ struct cmd_list_element
 
   /* Nonzero identifies a prefix command.  For them, the address
      of the variable containing the list of subcommands.  */
-  struct cmd_list_element **prefixlist = nullptr;
+  struct cmd_list_element **subcommands = nullptr;
 
   /* The prefix command of this command.  */
   struct cmd_list_element *prefix = nullptr;
diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index eb8853a5e649..5ddfab9674a0 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -1350,10 +1350,10 @@ validate_comname (const char **comname)
       const char *tem = prefix.c_str ();
 
       c = lookup_cmd (&tem, cmdlist, "", NULL, 0, 1);
-      if (c->prefixlist == NULL)
+      if (c->subcommands == NULL)
 	error (_("\"%s\" is not a prefix command."), prefix.c_str ());
 
-      list = c->prefixlist;
+      list = c->subcommands;
       *comname = last_word;
     }
 
@@ -1414,7 +1414,7 @@ do_define_command (const char *comname, int from_tty,
 	  /* if C is a prefix command that was previously defined,
 	     tell the user its subcommands will be kept, and ask
 	     if ok to redefine the command.  */
-	  if (c->prefixlist != nullptr)
+	  if (c->subcommands != nullptr)
 	    q = (c->user_commands.get () == nullptr
 		 || query (_("Keeping subcommands of prefix command \"%s\".\n"
 			     "Redefine command \"%s\"? "), c->name, c->name));
@@ -1470,8 +1470,8 @@ do_define_command (const char *comname, int from_tty,
     cmds = *commands;
 
   {
-    struct cmd_list_element **c_prefixlist
-      = c == nullptr ? nullptr : c->prefixlist;
+    struct cmd_list_element **c_subcommands
+      = c == nullptr ? nullptr : c->subcommands;
 
     newc = add_cmd (comname, class_user, user_defined_command,
 		    (c != nullptr && c->theclass == class_user)
@@ -1480,9 +1480,9 @@ do_define_command (const char *comname, int from_tty,
 
     /* If we define or re-define a command that was previously defined
        as a prefix, keep the prefix information.  */
-    if (c_prefixlist != nullptr)
+    if (c_subcommands != nullptr)
       {
-	newc->prefixlist = c_prefixlist;
+	newc->subcommands = c_subcommands;
 	/* allow_unknown: see explanation in equivalent logic in
 	   define_prefix_command ().  */
 	newc->allow_unknown = newc->user_commands.get () != nullptr;
@@ -1595,7 +1595,7 @@ define_prefix_command (const char *comname, int from_tty)
   if (c != nullptr && c->theclass != class_user)
     error (_("Command \"%s\" is built-in."), comfull);
 
-  if (c != nullptr && c->prefixlist != nullptr)
+  if (c != nullptr && c->subcommands != nullptr)
     {
       /* c is already a user defined prefix command.  */
       return;
@@ -1609,10 +1609,10 @@ define_prefix_command (const char *comname, int from_tty)
 		   xstrdup ("User-defined."), list);
     }
 
-  /* Allocate the c->prefixlist, which marks the command as a prefix
+  /* Allocate the c->subcommands, which marks the command as a prefix
      command.  */
-  c->prefixlist = new struct cmd_list_element*;
-  *(c->prefixlist) = nullptr;
+  c->subcommands = new struct cmd_list_element*;
+  *(c->subcommands) = nullptr;
   /* If the prefix command C is not a command, then it must be followed
      by known subcommands.  Otherwise, if C is also a normal command,
      it can be followed by C args that must not cause a 'subcommand'
@@ -1665,7 +1665,7 @@ show_user_1 (struct cmd_list_element *c, const char *prefix, const char *name,
       struct command_line *cmdlines = c->user_commands.get ();
 
       fprintf_filtered (stream, "User %scommand \"",
-			c->prefixlist == NULL ? "" : "prefix ");
+			c->subcommands == NULL ? "" : "prefix ");
       fprintf_styled (stream, title_style.style (), "%s%s",
 		      prefix, name);
       fprintf_filtered (stream, "\":\n");
@@ -1676,12 +1676,12 @@ show_user_1 (struct cmd_list_element *c, const char *prefix, const char *name,
 	}
     }
 
-  if (c->prefixlist != NULL)
+  if (c->subcommands != NULL)
     {
       const std::string prefixname = c->prefixname ();
 
-      for (c = *c->prefixlist; c != NULL; c = c->next)
-	if (c->theclass == class_user || c->prefixlist != NULL)
+      for (c = *c->subcommands; c != NULL; c = c->next)
+	if (c->theclass == class_user || c->subcommands != NULL)
 	  show_user_1 (c, prefixname.c_str (), c->name, gdb_stdout);
     }
 
diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c
index 35482e1c5cc6..cb72c6241cbf 100644
--- a/gdb/cli/cli-setshow.c
+++ b/gdb/cli/cli-setshow.c
@@ -550,10 +550,10 @@ do_set_command (const char *arg, int from_tty, struct cmd_list_element *c)
 	  p = p->prefix;
 	}
 
-      /* Don't trigger any observer notification if prefixlist is not
+      /* Don't trigger any observer notification if subcommands is not
 	 setlist.  */
       i--;
-      if (cmds[i]->prefixlist != &setlist)
+      if (cmds[i]->subcommands != &setlist)
 	{
 	  xfree (cmds);
 	  xfree (name);
@@ -740,7 +740,7 @@ cmd_show_list (struct cmd_list_element *list, int from_tty)
 
       /* If we find a prefix, run its list, prefixing our output by its
 	 prefix (with "show " skipped).  */
-      if (list->prefixlist && list->cmd_pointer == nullptr)
+      if (list->subcommands && list->cmd_pointer == nullptr)
 	{
 	  ui_out_emit_tuple optionlist_emitter (uiout, "optionlist");
 	  std::string prefixname = list->prefixname ();
@@ -748,7 +748,7 @@ cmd_show_list (struct cmd_list_element *list, int from_tty)
 
 	  if (uiout->is_mi_like_p ())
 	    uiout->field_string ("prefix", new_prefix);
-	  cmd_show_list (*list->prefixlist, from_tty);
+	  cmd_show_list (*list->subcommands, from_tty);
 	}
       else if (list->theclass != no_set_class && list->cmd_pointer == nullptr)
 	{
@@ -758,7 +758,7 @@ cmd_show_list (struct cmd_list_element *list, int from_tty)
 	    {
 	      /* If we find a prefix, output it (with "show " skipped).  */
 	      std::string prefixname = list->prefix->prefixname ();
-	      prefixname = (list->prefix->prefixlist == nullptr ? ""
+	      prefixname = (list->prefix->subcommands == nullptr ? ""
 			    : strstr (prefixname.c_str (), "show ") + 5);
 	      uiout->text (prefixname.c_str ());
 	    }
diff --git a/gdb/command.h b/gdb/command.h
index 33feb19166c6..fe3c7b482f04 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -208,7 +208,7 @@ extern struct cmd_list_element *add_show_prefix_cmd
 extern struct cmd_list_element *add_prefix_cmd_suppress_notification
 			(const char *name, enum command_class theclass,
 			 cmd_const_cfunc_ftype *fun,
-			 const char *doc, struct cmd_list_element **prefixlist,
+			 const char *doc, struct cmd_list_element **subcommands,
 			 int allow_unknown,
 			 struct cmd_list_element **list,
 			 int *suppress_notification);
diff --git a/gdb/completer.c b/gdb/completer.c
index 80e8c7b743e1..6d39435c6ca3 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -1428,7 +1428,7 @@ complete_line_internal_1 (completion_tracker &tracker,
 	  if (result_list)
 	    {
 	      if (reason != handle_brkchars)
-		complete_on_cmdlist (*result_list->prefixlist, tracker, p,
+		complete_on_cmdlist (*result_list->subcommands, tracker, p,
 				     word, ignore_help_classes);
 	    }
 	  else
@@ -1456,12 +1456,12 @@ complete_line_internal_1 (completion_tracker &tracker,
 	    {
 	      /* The command is followed by whitespace; we need to
 		 complete on whatever comes after command.  */
-	      if (c->prefixlist)
+	      if (c->subcommands)
 		{
 		  /* It is a prefix command; what comes after it is
 		     a subcommand (e.g. "info ").  */
 		  if (reason != handle_brkchars)
-		    complete_on_cmdlist (*c->prefixlist, tracker, p, word,
+		    complete_on_cmdlist (*c->subcommands, tracker, p, word,
 					 ignore_help_classes);
 
 		  /* Ensure that readline does the right thing
@@ -1524,7 +1524,7 @@ complete_line_internal_1 (completion_tracker &tracker,
 	{
 	  /* There is non-whitespace beyond the command.  */
 
-	  if (c->prefixlist && !c->allow_unknown)
+	  if (c->subcommands && !c->allow_unknown)
 	    {
 	      /* It is an unrecognized subcommand of a prefix command,
 		 e.g. "info adsfkdj".  */
diff --git a/gdb/guile/scm-cmd.c b/gdb/guile/scm-cmd.c
index 1f35d881a2d1..fc7dc2b7d0be 100644
--- a/gdb/guile/scm-cmd.c
+++ b/gdb/guile/scm-cmd.c
@@ -524,10 +524,10 @@ gdbscm_parse_command_name (const char *name,
 				 gdbscm_scm_from_c_string (name), msg);
     }
 
-  if (elt->prefixlist)
+  if (elt->subcommands)
     {
       xfree (prefix_text);
-      *base_list = elt->prefixlist;
+      *base_list = elt->subcommands;
       return result;
     }
 
diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c
index 6f7794cdf53d..c2c0df5798da 100644
--- a/gdb/python/py-cmd.c
+++ b/gdb/python/py-cmd.c
@@ -111,7 +111,7 @@ cmdpy_function (struct cmd_list_element *command,
     error (_("Invalid invocation of Python command object."));
   if (! PyObject_HasAttr ((PyObject *) obj, invoke_cst))
     {
-      if (obj->command->prefixlist != nullptr)
+      if (obj->command->subcommands != nullptr)
 	{
 	  /* A prefix command does not need an invoke method.  */
 	  return;
@@ -393,9 +393,9 @@ gdbpy_parse_command_name (const char *name,
       return NULL;
     }
 
-  if (elt->prefixlist)
+  if (elt->subcommands)
     {
-      *base_list = elt->prefixlist;
+      *base_list = elt->subcommands;
       return result;
     }
 
diff --git a/gdb/top.c b/gdb/top.c
index 2eb68fd2bc0e..a391e9359de0 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -647,7 +647,7 @@ execute_command (const char *p, int from_tty)
       if (c->theclass == class_user && c->user_commands)
 	execute_user_command (c, arg);
       else if (c->theclass == class_user
-	       && c->prefixlist && !c->allow_unknown)
+	       && c->subcommands && !c->allow_unknown)
 	/* If this is a user defined prefix that does not allow unknown
 	   (in other words, C is a prefix command and not a command
 	   that can be followed by its args), report the list of
@@ -659,7 +659,7 @@ execute_command (const char *p, int from_tty)
 	  printf_unfiltered
 	    ("\"%s\" must be followed by the name of a subcommand.\n",
 	     prefixname_no_space.c_str ());
-	  help_list (*c->prefixlist, prefixname.c_str (), all_commands,
+	  help_list (*c->subcommands, prefixname.c_str (), all_commands,
 		     gdb_stdout);
 	}
       else if (c->type == set_cmd)
diff --git a/gdb/unittests/command-def-selftests.c b/gdb/unittests/command-def-selftests.c
index 040e22b93909..53e5626f6459 100644
--- a/gdb/unittests/command-def-selftests.c
+++ b/gdb/unittests/command-def-selftests.c
@@ -83,11 +83,11 @@ check_doc (struct cmd_list_element *commandlist, const char *prefix)
       /* Check if this command has subcommands and is not an
 	 abbreviation.  We skip checking subcommands of abbreviations
 	 in order to avoid duplicates in the output.  */
-      if (c->prefixlist != NULL && !c->abbrev_flag)
+      if (c->subcommands != NULL && !c->abbrev_flag)
 	{
 	  /* Recursively call ourselves on the subcommand list,
 	     passing the right prefix in.  */
-	  check_doc (*c->prefixlist, c->prefixname ().c_str ());
+	  check_doc (*c->subcommands, c->prefixname ().c_str ());
 	}
     }
 }
@@ -155,11 +155,11 @@ traverse_command_structure (struct cmd_list_element **list,
     {
       /* If this command has subcommands and is not an alias,
 	 traverse the subcommands.  */
-      if (c->prefixlist != NULL && c->cmd_pointer == nullptr)
+      if (c->subcommands != NULL && c->cmd_pointer == nullptr)
 	{
 	  /* Recursively call ourselves on the subcommand list,
 	     passing the right prefix in.  */
-	  traverse_command_structure (c->prefixlist, c->prefixname ().c_str ());
+	  traverse_command_structure (c->subcommands, c->prefixname ().c_str ());
 	}
       if (prefixcmd != c->prefix
 	  || (prefixcmd == nullptr && *list != cmdlist))
-- 
2.31.1

[PATCH 4/7] gdb: rename cmd_list_element::cmd_pointer to target

[Simon Marchi]

cmd_pointer is another field whose name I found really not clear.  Yes,
it's a pointer to a command, the type tells me that.  But what's the
relationship of that command to the current command?  This field
contains, for an alias, the command that it aliases.  So I think that
the name "alias_target" would be more appropriate.

Also, rename "old" parameters to "target" in the functions that add
aliases.

gdb/ChangeLog:

	* cli/cli-decode.h (cmd_list_element) <cmd_pointer>: Rename
	to...
	<alias_target>: ... this.
	(add_alias_cmd): Rename old to target.
	(add_info_alias): Rename old_name to target_name.
	(add_com_alias): Likewise.

Change-Id: I8db36c6dd799fae155f7acd3805f6d62d98befa9
---
 gdb/cli/cli-decode.c                  | 77 +++++++++++++--------------
 gdb/cli/cli-decode.h                  |  2 +-
 gdb/cli/cli-setshow.c                 |  4 +-
 gdb/unittests/command-def-selftests.c |  2 +-
 4 files changed, 41 insertions(+), 44 deletions(-)

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 27bd311e89c1..ec579ff19617 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -75,7 +75,7 @@ lookup_cmd_with_subcommands (cmd_list_element **subcommands,
 	{
 	  /* If we found an alias, we must return the aliased
 	     command.  */
-	  return p->cmd_pointer ? p->cmd_pointer : p;
+	  return p->alias_target ? p->alias_target : p;
 	}
 
       q = lookup_cmd_with_subcommands (subcommands, *(p->subcommands));
@@ -208,7 +208,7 @@ do_add_cmd (const char *name, enum command_class theclass,
   c->aliases = delete_cmd (name, list, &c->hook_pre, &c->hookee_pre,
 			   &c->hook_post, &c->hookee_post);
   for (iter = c->aliases; iter; iter = iter->alias_chain)
-    iter->cmd_pointer = c;
+    iter->alias_target = c;
   if (c->hook_pre)
     c->hook_pre->hookee_pre = c;
   if (c->hookee_pre)
@@ -305,45 +305,42 @@ deprecate_cmd (struct cmd_list_element *cmd, const char *replacement)
 }
 
 struct cmd_list_element *
-add_alias_cmd (const char *name, cmd_list_element *old,
+add_alias_cmd (const char *name, cmd_list_element *target,
 	       enum command_class theclass, int abbrev_flag,
 	       struct cmd_list_element **list)
 {
-  gdb_assert (old != nullptr);
+  gdb_assert (target != nullptr);
 
-  struct cmd_list_element *c = add_cmd (name, theclass, old->doc, list);
+  struct cmd_list_element *c = add_cmd (name, theclass, target->doc, list);
 
-  /* If OLD->DOC can be freed, we should make another copy.  */
-  if (old->doc_allocated)
+  /* If TARGET->DOC can be freed, we should make another copy.  */
+  if (target->doc_allocated)
     {
-      c->doc = xstrdup (old->doc);
+      c->doc = xstrdup (target->doc);
       c->doc_allocated = 1;
     }
   /* NOTE: Both FUNC and all the FUNCTIONs need to be copied.  */
-  c->func = old->func;
-  c->function = old->function;
-  c->subcommands = old->subcommands;
-  c->allow_unknown = old->allow_unknown;
+  c->func = target->func;
+  c->function = target->function;
+  c->subcommands = target->subcommands;
+  c->allow_unknown = target->allow_unknown;
   c->abbrev_flag = abbrev_flag;
-  c->cmd_pointer = old;
-  c->alias_chain = old->aliases;
-  old->aliases = c;
+  c->alias_target = target;
+  c->alias_chain = target->aliases;
+  target->aliases = c;
 
   return c;
 }
 
 struct cmd_list_element *
-add_alias_cmd (const char *name, const char *oldname,
+add_alias_cmd (const char *name, const char *target_name,
 	       enum command_class theclass, int abbrev_flag,
 	       struct cmd_list_element **list)
 {
-  const char *tmp;
-  struct cmd_list_element *old;
+  const char *tmp = target_name;
+  cmd_list_element *target = lookup_cmd (&tmp, *list, "", NULL, 1, 1);
 
-  tmp = oldname;
-  old = lookup_cmd (&tmp, *list, "", NULL, 1, 1);
-
-  return add_alias_cmd (name, old, theclass, abbrev_flag, list);
+  return add_alias_cmd (name, target, theclass, abbrev_flag, list);
 }
 
 
@@ -408,8 +405,8 @@ static void
 do_prefix_cmd (const char *args, int from_tty, struct cmd_list_element *c)
 {
   /* Look past all aliases.  */
-  while (c->cmd_pointer != nullptr)
-    c = c->cmd_pointer;
+  while (c->alias_target != nullptr)
+    c = c->alias_target;
 
   help_list (*c->subcommands, c->prefixname ().c_str (),
 	     all_commands, gdb_stdout);
@@ -951,9 +948,9 @@ delete_cmd (const char *name, struct cmd_list_element **list,
 
 	  /* If this command was an alias, remove it from the list of
 	     aliases.  */
-	  if (iter->cmd_pointer)
+	  if (iter->alias_target)
 	    {
-	      struct cmd_list_element **prevp = &iter->cmd_pointer->aliases;
+	      struct cmd_list_element **prevp = &iter->alias_target->aliases;
 	      struct cmd_list_element *a = *prevp;
 
 	      while (a != iter)
@@ -989,9 +986,9 @@ add_info (const char *name, cmd_const_cfunc_ftype *fun, const char *doc)
 /* Add an alias to the list of info subcommands.  */
 
 struct cmd_list_element *
-add_info_alias (const char *name, const char *oldname, int abbrev_flag)
+add_info_alias (const char *name, const char *target_name, int abbrev_flag)
 {
-  return add_alias_cmd (name, oldname, class_run, abbrev_flag, &infolist);
+  return add_alias_cmd (name, target_name, class_run, abbrev_flag, &infolist);
 }
 
 /* Add an element to the list of commands.  */
@@ -1010,10 +1007,10 @@ add_com (const char *name, enum command_class theclass,
    user defined aliases.  */
 
 struct cmd_list_element *
-add_com_alias (const char *name, const char *oldname, enum command_class theclass,
-	       int abbrev_flag)
+add_com_alias (const char *name, const char *target_name,
+	       command_class theclass, int abbrev_flag)
 {
-  return add_alias_cmd (name, oldname, theclass, abbrev_flag, &cmdlist);
+  return add_alias_cmd (name, target_name, theclass, abbrev_flag, &cmdlist);
 }
 
 /* Add an element with a suppress notification to the list of commands.  */
@@ -1046,11 +1043,11 @@ static void
 fput_alias_definition_styled (struct cmd_list_element *c,
 			      struct ui_file *stream)
 {
-  gdb_assert (c->cmd_pointer != nullptr);
+  gdb_assert (c->alias_target != nullptr);
   fputs_filtered ("  alias ", stream);
   fput_command_name_styled (c, stream);
   fprintf_filtered (stream, " = ");
-  fput_command_name_styled (c->cmd_pointer, stream);
+  fput_command_name_styled (c->alias_target, stream);
   fprintf_filtered (stream, " %s\n", c->default_args.c_str ());
 }
 
@@ -1149,7 +1146,7 @@ apropos_cmd (struct ui_file *stream,
   /* Walk through the commands.  */
   for (c=commandlist;c;c=c->next)
     {
-      if (c->cmd_pointer != nullptr)
+      if (c->alias_target != nullptr)
 	{
 	  /* Command aliases/abbreviations are skipped to ensure we print the
 	     doc of a command only once, when encountering the aliased
@@ -1490,7 +1487,7 @@ help_cmd_list (struct cmd_list_element *list, enum command_class theclass,
 	  continue;
 	}
 
-      if (c->cmd_pointer != nullptr && theclass != class_alias)
+      if (c->alias_target != nullptr && theclass != class_alias)
 	{
 	  /* Do not show an alias, unless specifically showing the
 	     list of aliases:  for all other classes, an alias is
@@ -1512,7 +1509,7 @@ help_cmd_list (struct cmd_list_element *list, enum command_class theclass,
 	     list of sub-commands of the aliased command.  */
 	  print_help_for_command
 	    (c,
-	     recurse && (theclass != class_alias || c->cmd_pointer == nullptr),
+	     recurse && (theclass != class_alias || c->alias_target == nullptr),
 	     stream);
 	  continue;
 	}
@@ -1675,7 +1672,7 @@ lookup_cmd_1 (const char **text, struct cmd_list_element *clist,
 
   *text += len;
 
-  if (found->cmd_pointer)
+  if (found->alias_target)
     {
       /* We drop the alias (abbreviation) in favor of the command it
        is pointing to.  If the alias is deprecated, though, we need to
@@ -1692,7 +1689,7 @@ lookup_cmd_1 (const char **text, struct cmd_list_element *clist,
 	 of the command it is pointing to.  */
       if (default_args != nullptr)
 	*default_args = found->default_args;
-      found = found->cmd_pointer;
+      found = found->alias_target;
       found_alias = true;
     }
   /* If we found a prefix command, keep looking.  */
@@ -2047,12 +2044,12 @@ lookup_cmd_composition_1 (const char *text,
 	return 0;
       else
 	{
-	  if ((*cmd)->cmd_pointer)
+	  if ((*cmd)->alias_target)
 	    {
 	      /* If the command was actually an alias, we note that an
 		 alias was used (by assigning *ALIAS) and we set *CMD.  */
 	      *alias = *cmd;
-	      *cmd = (*cmd)->cmd_pointer;
+	      *cmd = (*cmd)->alias_target;
 	    }
 	}
 
diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
index e2428bdddf7d..68a9b8586b40 100644
--- a/gdb/cli/cli-decode.h
+++ b/gdb/cli/cli-decode.h
@@ -229,7 +229,7 @@ struct cmd_list_element
 
   /* Pointer to command that is aliased by this one, so the
      aliased command can be located in case it has been hooked.  */
-  struct cmd_list_element *cmd_pointer = nullptr;
+  struct cmd_list_element *alias_target = nullptr;
 
   /* Start of a linked list of all aliases of this command.  */
   struct cmd_list_element *aliases = nullptr;
diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c
index cb72c6241cbf..cb821c5b3c03 100644
--- a/gdb/cli/cli-setshow.c
+++ b/gdb/cli/cli-setshow.c
@@ -740,7 +740,7 @@ cmd_show_list (struct cmd_list_element *list, int from_tty)
 
       /* If we find a prefix, run its list, prefixing our output by its
 	 prefix (with "show " skipped).  */
-      if (list->subcommands && list->cmd_pointer == nullptr)
+      if (list->subcommands && list->alias_target == nullptr)
 	{
 	  ui_out_emit_tuple optionlist_emitter (uiout, "optionlist");
 	  std::string prefixname = list->prefixname ();
@@ -750,7 +750,7 @@ cmd_show_list (struct cmd_list_element *list, int from_tty)
 	    uiout->field_string ("prefix", new_prefix);
 	  cmd_show_list (*list->subcommands, from_tty);
 	}
-      else if (list->theclass != no_set_class && list->cmd_pointer == nullptr)
+      else if (list->theclass != no_set_class && list->alias_target == nullptr)
 	{
 	  ui_out_emit_tuple option_emitter (uiout, "option");
 
diff --git a/gdb/unittests/command-def-selftests.c b/gdb/unittests/command-def-selftests.c
index 53e5626f6459..123667d33699 100644
--- a/gdb/unittests/command-def-selftests.c
+++ b/gdb/unittests/command-def-selftests.c
@@ -155,7 +155,7 @@ traverse_command_structure (struct cmd_list_element **list,
     {
       /* If this command has subcommands and is not an alias,
 	 traverse the subcommands.  */
-      if (c->subcommands != NULL && c->cmd_pointer == nullptr)
+      if (c->subcommands != NULL && c->alias_target == nullptr)
 	{
 	  /* Recursively call ourselves on the subcommand list,
 	     passing the right prefix in.  */
-- 
2.31.1

[PATCH 5/7] gdb: add cmd_list_element::is_alias

[Simon Marchi]

Add the cmd_list_element::is_alias helper to check whether a command is
an alias.  I find it easier to understand the intention in:

  if (c->is_alias ())

than

  if (c->alias_target != nullptr)

Change all the spots that are reading alias_target just to compare it to
NULL/nullptr to use is_alias instead.

gdb/ChangeLog:

	* cli/cli-decode.h (cmd_list_element) <is_alias>: New, use it.

Change-Id: I26ed56f99ee47fe884fdfedf87016501631693ce
---
 gdb/cli/cli-decode.c                  | 18 +++++++++---------
 gdb/cli/cli-decode.h                  |  4 ++++
 gdb/cli/cli-setshow.c                 |  4 ++--
 gdb/unittests/command-def-selftests.c |  2 +-
 4 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index ec579ff19617..29b4ed2f6bd7 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -75,7 +75,7 @@ lookup_cmd_with_subcommands (cmd_list_element **subcommands,
 	{
 	  /* If we found an alias, we must return the aliased
 	     command.  */
-	  return p->alias_target ? p->alias_target : p;
+	  return p->is_alias () ? p->alias_target : p;
 	}
 
       q = lookup_cmd_with_subcommands (subcommands, *(p->subcommands));
@@ -405,7 +405,7 @@ static void
 do_prefix_cmd (const char *args, int from_tty, struct cmd_list_element *c)
 {
   /* Look past all aliases.  */
-  while (c->alias_target != nullptr)
+  while (c->is_alias ())
     c = c->alias_target;
 
   help_list (*c->subcommands, c->prefixname ().c_str (),
@@ -948,7 +948,7 @@ delete_cmd (const char *name, struct cmd_list_element **list,
 
 	  /* If this command was an alias, remove it from the list of
 	     aliases.  */
-	  if (iter->alias_target)
+	  if (iter->is_alias ())
 	    {
 	      struct cmd_list_element **prevp = &iter->alias_target->aliases;
 	      struct cmd_list_element *a = *prevp;
@@ -1043,7 +1043,7 @@ static void
 fput_alias_definition_styled (struct cmd_list_element *c,
 			      struct ui_file *stream)
 {
-  gdb_assert (c->alias_target != nullptr);
+  gdb_assert (c->is_alias ());
   fputs_filtered ("  alias ", stream);
   fput_command_name_styled (c, stream);
   fprintf_filtered (stream, " = ");
@@ -1146,7 +1146,7 @@ apropos_cmd (struct ui_file *stream,
   /* Walk through the commands.  */
   for (c=commandlist;c;c=c->next)
     {
-      if (c->alias_target != nullptr)
+      if (c->is_alias ())
 	{
 	  /* Command aliases/abbreviations are skipped to ensure we print the
 	     doc of a command only once, when encountering the aliased
@@ -1487,7 +1487,7 @@ help_cmd_list (struct cmd_list_element *list, enum command_class theclass,
 	  continue;
 	}
 
-      if (c->alias_target != nullptr && theclass != class_alias)
+      if (c->is_alias () && theclass != class_alias)
 	{
 	  /* Do not show an alias, unless specifically showing the
 	     list of aliases:  for all other classes, an alias is
@@ -1509,7 +1509,7 @@ help_cmd_list (struct cmd_list_element *list, enum command_class theclass,
 	     list of sub-commands of the aliased command.  */
 	  print_help_for_command
 	    (c,
-	     recurse && (theclass != class_alias || c->alias_target == nullptr),
+	     recurse && (theclass != class_alias || !c->is_alias ()),
 	     stream);
 	  continue;
 	}
@@ -1672,7 +1672,7 @@ lookup_cmd_1 (const char **text, struct cmd_list_element *clist,
 
   *text += len;
 
-  if (found->alias_target)
+  if (found->is_alias ())
     {
       /* We drop the alias (abbreviation) in favor of the command it
        is pointing to.  If the alias is deprecated, though, we need to
@@ -2044,7 +2044,7 @@ lookup_cmd_composition_1 (const char *text,
 	return 0;
       else
 	{
-	  if ((*cmd)->alias_target)
+	  if ((*cmd)->is_alias ())
 	    {
 	      /* If the command was actually an alias, we note that an
 		 alias was used (by assigning *ALIAS) and we set *CMD.  */
diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
index 68a9b8586b40..8caf60234548 100644
--- a/gdb/cli/cli-decode.h
+++ b/gdb/cli/cli-decode.h
@@ -79,6 +79,10 @@ struct cmd_list_element
      For non-prefix commands, return an empty string.  */
   std::string prefixname () const;
 
+  /* Return true if this command is an alias of another command.  */
+  bool is_alias () const
+  { return this->alias_target != nullptr; }
+
   /* Points to next command in this list.  */
   struct cmd_list_element *next = nullptr;
 
diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c
index cb821c5b3c03..7e93a7034942 100644
--- a/gdb/cli/cli-setshow.c
+++ b/gdb/cli/cli-setshow.c
@@ -740,7 +740,7 @@ cmd_show_list (struct cmd_list_element *list, int from_tty)
 
       /* If we find a prefix, run its list, prefixing our output by its
 	 prefix (with "show " skipped).  */
-      if (list->subcommands && list->alias_target == nullptr)
+      if (list->subcommands && !list->is_alias ())
 	{
 	  ui_out_emit_tuple optionlist_emitter (uiout, "optionlist");
 	  std::string prefixname = list->prefixname ();
@@ -750,7 +750,7 @@ cmd_show_list (struct cmd_list_element *list, int from_tty)
 	    uiout->field_string ("prefix", new_prefix);
 	  cmd_show_list (*list->subcommands, from_tty);
 	}
-      else if (list->theclass != no_set_class && list->alias_target == nullptr)
+      else if (list->theclass != no_set_class && !list->is_alias ())
 	{
 	  ui_out_emit_tuple option_emitter (uiout, "option");
 
diff --git a/gdb/unittests/command-def-selftests.c b/gdb/unittests/command-def-selftests.c
index 123667d33699..4a6b6789b370 100644
--- a/gdb/unittests/command-def-selftests.c
+++ b/gdb/unittests/command-def-selftests.c
@@ -155,7 +155,7 @@ traverse_command_structure (struct cmd_list_element **list,
     {
       /* If this command has subcommands and is not an alias,
 	 traverse the subcommands.  */
-      if (c->subcommands != NULL && c->alias_target == nullptr)
+      if (c->subcommands != NULL && !c->is_alias ())
 	{
 	  /* Recursively call ourselves on the subcommand list,
 	     passing the right prefix in.  */
-- 
2.31.1

[PATCH 6/7] gdb: add cmd_list_element::is_prefix

[Simon Marchi]

Same idea as the previous patch, but for prefix instead of alias.

gdb/ChangeLog:

	* cli/cli-decode.h (cmd_list_element) <is_prefix>: New, use it.

Change-Id: I76a9d2e82fc8d7429904424674d99ce6f9880e2b
---
 gdb/auto-load.c                       |  2 +-
 gdb/cli/cli-cmds.c                    |  4 ++--
 gdb/cli/cli-decode.c                  | 26 ++++++++++++++------------
 gdb/cli/cli-decode.h                  |  4 ++++
 gdb/cli/cli-script.c                  | 12 ++++++------
 gdb/cli/cli-setshow.c                 |  4 ++--
 gdb/completer.c                       |  4 ++--
 gdb/guile/scm-cmd.c                   |  2 +-
 gdb/python/py-cmd.c                   |  4 ++--
 gdb/top.c                             |  2 +-
 gdb/unittests/command-def-selftests.c |  4 ++--
 11 files changed, 37 insertions(+), 31 deletions(-)

diff --git a/gdb/auto-load.c b/gdb/auto-load.c
index 7745aa65b036..9cd70f174c3c 100644
--- a/gdb/auto-load.c
+++ b/gdb/auto-load.c
@@ -1465,7 +1465,7 @@ info_auto_load_cmd (const char *args, int from_tty)
     {
       ui_out_emit_tuple option_emitter (uiout, "option");
 
-      gdb_assert (!list->subcommands);
+      gdb_assert (!list->is_prefix ());
       gdb_assert (list->type == not_set_cmd);
 
       uiout->field_string ("name", list->name);
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index c29e5979acb4..0bf418e510ea 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -1625,7 +1625,7 @@ show_user (const char *args, int from_tty)
     {
       for (c = cmdlist; c; c = c->next)
 	{
-	  if (cli_user_command_p (c) || c->subcommands != NULL)
+	  if (cli_user_command_p (c) || c->is_prefix ())
 	    show_user_1 (c, "", c->name, gdb_stdout);
 	}
     }
@@ -1900,7 +1900,7 @@ alias_command (const char *args, int from_tty)
       /* We've already tried to look up COMMAND.  */
       gdb_assert (c_command != NULL
 		  && c_command != (struct cmd_list_element *) -1);
-      gdb_assert (c_command->subcommands != NULL);
+      gdb_assert (c_command->is_prefix ());
       c_alias = lookup_cmd_1 (& alias_prefix, cmdlist, NULL, NULL, 1);
       if (c_alias != c_command)
 	error (_("ALIAS and COMMAND prefixes do not match."));
diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 29b4ed2f6bd7..785e726a81f6 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -69,8 +69,9 @@ lookup_cmd_with_subcommands (cmd_list_element **subcommands,
     {
       struct cmd_list_element *q;
 
-      if (p->subcommands == NULL)
+      if (!p->is_prefix ())
 	continue;
+
       else if (p->subcommands == subcommands)
 	{
 	  /* If we found an alias, we must return the aliased
@@ -163,7 +164,7 @@ set_cmd_completer_handle_brkchars (struct cmd_list_element *cmd,
 std::string
 cmd_list_element::prefixname () const
 {
-  if (this->subcommands == nullptr)
+  if (!this->is_prefix ())
     /* Not a prefix command.  */
     return "";
 
@@ -369,7 +370,7 @@ update_prefix_field_of_prefixed_commands (struct cmd_list_element *c)
 	 'auto-load' command was not yet reachable by
 	    lookup_cmd_for_subcommands (list, cmdlist)
 	    that searches from the top level 'cmdlist'.  */
-      if (p->subcommands != nullptr)
+      if (p->is_prefix ())
 	update_prefix_field_of_prefixed_commands (p);
     }
 }
@@ -1184,7 +1185,7 @@ apropos_cmd (struct ui_file *stream,
 	    print_doc_of_command (c, prefix, verbose, regex, stream);
 	}
       /* Check if this command has subcommands.  */
-      if (c->subcommands != NULL)
+      if (c->is_prefix ())
 	{
 	  /* Recursively call ourselves on the subcommand list,
 	     passing the right prefix in.  */
@@ -1249,12 +1250,13 @@ help_cmd (const char *command, struct ui_file *stream)
   fputs_filtered (c->doc, stream);
   fputs_filtered ("\n", stream);
 
-  if (c->subcommands == 0 && c->func != NULL)
+  if (!c->is_prefix () && c->func != NULL)
     return;
+
   fprintf_filtered (stream, "\n");
 
   /* If this is a prefix command, print it's subcommands.  */
-  if (c->subcommands)
+  if (c->is_prefix ())
     help_list (*c->subcommands, c->prefixname ().c_str (),
 	       all_commands, stream);
 
@@ -1446,7 +1448,7 @@ print_help_for_command (struct cmd_list_element *c,
   fput_aliases_definition_styled (c, stream);
 
   if (recurse
-      && c->subcommands != 0
+      && c->is_prefix ()
       && c->abbrev_flag == 0)
     /* Subcommands of a prefix command typically have 'all_commands'
        as class.  If we pass CLASS to recursive invocation,
@@ -1516,7 +1518,7 @@ help_cmd_list (struct cmd_list_element *list, enum command_class theclass,
 
       if (recurse
 	  && (theclass == class_user || theclass == class_alias)
-	  && c->subcommands != NULL)
+	  && c->is_prefix ())
 	{
 	  /* User-defined commands or aliases may be subcommands.  */
 	  help_cmd_list (*c->subcommands, theclass, recurse, stream);
@@ -1694,7 +1696,7 @@ lookup_cmd_1 (const char **text, struct cmd_list_element *clist,
     }
   /* If we found a prefix command, keep looking.  */
 
-  if (found->subcommands)
+  if (found->is_prefix ())
     {
       c = lookup_cmd_1 (text, *found->subcommands, result_list, default_args,
 			ignore_help_classes, lookup_for_completion_p);
@@ -1865,7 +1867,7 @@ lookup_cmd (const char **line, struct cmd_list_element *list,
       while (**line == ' ' || **line == '\t')
 	(*line)++;
 
-      if (c->subcommands && **line && !c->allow_unknown)
+      if (c->is_prefix () && **line && !c->allow_unknown)
 	undef_cmd_error (c->prefixname ().c_str (), *line);
 
       /* Seems to be what he wants.  Return it.  */
@@ -2056,7 +2058,7 @@ lookup_cmd_composition_1 (const char *text,
       text += len;
       text = skip_spaces (text);
 
-      if ((*cmd)->subcommands != nullptr && *text != '\0')
+      if ((*cmd)->is_prefix () && *text != '\0')
 	{
 	  cur_list = *(*cmd)->subcommands;
 	  *prefix_cmd = *cmd;
@@ -2123,7 +2125,7 @@ complete_on_cmdlist (struct cmd_list_element *list,
 	if (!strncmp (ptr->name, text, textlen)
 	    && !ptr->abbrev_flag
 	    && (!ignore_help_classes || ptr->func
-		|| ptr->subcommands))
+		|| ptr->is_prefix ()))
 	  {
 	    if (pass == 0)
 	      {
diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
index 8caf60234548..6204ed745e00 100644
--- a/gdb/cli/cli-decode.h
+++ b/gdb/cli/cli-decode.h
@@ -83,6 +83,10 @@ struct cmd_list_element
   bool is_alias () const
   { return this->alias_target != nullptr; }
 
+  /* Return true if this command is a prefix command.  */
+  bool is_prefix () const
+  { return this->subcommands != nullptr; }
+
   /* Points to next command in this list.  */
   struct cmd_list_element *next = nullptr;
 
diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index 5ddfab9674a0..ff93523f4daa 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -1350,7 +1350,7 @@ validate_comname (const char **comname)
       const char *tem = prefix.c_str ();
 
       c = lookup_cmd (&tem, cmdlist, "", NULL, 0, 1);
-      if (c->subcommands == NULL)
+      if (!c->is_prefix ())
 	error (_("\"%s\" is not a prefix command."), prefix.c_str ());
 
       list = c->subcommands;
@@ -1414,7 +1414,7 @@ do_define_command (const char *comname, int from_tty,
 	  /* if C is a prefix command that was previously defined,
 	     tell the user its subcommands will be kept, and ask
 	     if ok to redefine the command.  */
-	  if (c->subcommands != nullptr)
+	  if (c->is_prefix ())
 	    q = (c->user_commands.get () == nullptr
 		 || query (_("Keeping subcommands of prefix command \"%s\".\n"
 			     "Redefine command \"%s\"? "), c->name, c->name));
@@ -1595,7 +1595,7 @@ define_prefix_command (const char *comname, int from_tty)
   if (c != nullptr && c->theclass != class_user)
     error (_("Command \"%s\" is built-in."), comfull);
 
-  if (c != nullptr && c->subcommands != nullptr)
+  if (c != nullptr && c->is_prefix ())
     {
       /* c is already a user defined prefix command.  */
       return;
@@ -1665,7 +1665,7 @@ show_user_1 (struct cmd_list_element *c, const char *prefix, const char *name,
       struct command_line *cmdlines = c->user_commands.get ();
 
       fprintf_filtered (stream, "User %scommand \"",
-			c->subcommands == NULL ? "" : "prefix ");
+			c->is_prefix () ? "prefix" : "");
       fprintf_styled (stream, title_style.style (), "%s%s",
 		      prefix, name);
       fprintf_filtered (stream, "\":\n");
@@ -1676,12 +1676,12 @@ show_user_1 (struct cmd_list_element *c, const char *prefix, const char *name,
 	}
     }
 
-  if (c->subcommands != NULL)
+  if (c->is_prefix ())
     {
       const std::string prefixname = c->prefixname ();
 
       for (c = *c->subcommands; c != NULL; c = c->next)
-	if (c->theclass == class_user || c->subcommands != NULL)
+	if (c->theclass == class_user || c->is_prefix ())
 	  show_user_1 (c, prefixname.c_str (), c->name, gdb_stdout);
     }
 
diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c
index 7e93a7034942..82008ca8eed6 100644
--- a/gdb/cli/cli-setshow.c
+++ b/gdb/cli/cli-setshow.c
@@ -740,7 +740,7 @@ cmd_show_list (struct cmd_list_element *list, int from_tty)
 
       /* If we find a prefix, run its list, prefixing our output by its
 	 prefix (with "show " skipped).  */
-      if (list->subcommands && !list->is_alias ())
+      if (list->is_prefix () && !list->is_alias ())
 	{
 	  ui_out_emit_tuple optionlist_emitter (uiout, "optionlist");
 	  std::string prefixname = list->prefixname ();
@@ -758,7 +758,7 @@ cmd_show_list (struct cmd_list_element *list, int from_tty)
 	    {
 	      /* If we find a prefix, output it (with "show " skipped).  */
 	      std::string prefixname = list->prefix->prefixname ();
-	      prefixname = (list->prefix->subcommands == nullptr ? ""
+	      prefixname = (!list->prefix->is_prefix () ? ""
 			    : strstr (prefixname.c_str (), "show ") + 5);
 	      uiout->text (prefixname.c_str ());
 	    }
diff --git a/gdb/completer.c b/gdb/completer.c
index 6d39435c6ca3..6ad788b4344c 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -1456,7 +1456,7 @@ complete_line_internal_1 (completion_tracker &tracker,
 	    {
 	      /* The command is followed by whitespace; we need to
 		 complete on whatever comes after command.  */
-	      if (c->subcommands)
+	      if (c->is_prefix ())
 		{
 		  /* It is a prefix command; what comes after it is
 		     a subcommand (e.g. "info ").  */
@@ -1524,7 +1524,7 @@ complete_line_internal_1 (completion_tracker &tracker,
 	{
 	  /* There is non-whitespace beyond the command.  */
 
-	  if (c->subcommands && !c->allow_unknown)
+	  if (c->is_prefix () && !c->allow_unknown)
 	    {
 	      /* It is an unrecognized subcommand of a prefix command,
 		 e.g. "info adsfkdj".  */
diff --git a/gdb/guile/scm-cmd.c b/gdb/guile/scm-cmd.c
index fc7dc2b7d0be..39c915e0ab22 100644
--- a/gdb/guile/scm-cmd.c
+++ b/gdb/guile/scm-cmd.c
@@ -524,7 +524,7 @@ gdbscm_parse_command_name (const char *name,
 				 gdbscm_scm_from_c_string (name), msg);
     }
 
-  if (elt->subcommands)
+  if (elt->is_prefix ())
     {
       xfree (prefix_text);
       *base_list = elt->subcommands;
diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c
index c2c0df5798da..4f01fc0b5f35 100644
--- a/gdb/python/py-cmd.c
+++ b/gdb/python/py-cmd.c
@@ -111,7 +111,7 @@ cmdpy_function (struct cmd_list_element *command,
     error (_("Invalid invocation of Python command object."));
   if (! PyObject_HasAttr ((PyObject *) obj, invoke_cst))
     {
-      if (obj->command->subcommands != nullptr)
+      if (obj->command->is_prefix ())
 	{
 	  /* A prefix command does not need an invoke method.  */
 	  return;
@@ -393,7 +393,7 @@ gdbpy_parse_command_name (const char *name,
       return NULL;
     }
 
-  if (elt->subcommands)
+  if (elt->is_prefix ())
     {
       *base_list = elt->subcommands;
       return result;
diff --git a/gdb/top.c b/gdb/top.c
index a391e9359de0..a1a932ae93e9 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -647,7 +647,7 @@ execute_command (const char *p, int from_tty)
       if (c->theclass == class_user && c->user_commands)
 	execute_user_command (c, arg);
       else if (c->theclass == class_user
-	       && c->subcommands && !c->allow_unknown)
+	       && c->is_prefix () && !c->allow_unknown)
 	/* If this is a user defined prefix that does not allow unknown
 	   (in other words, C is a prefix command and not a command
 	   that can be followed by its args), report the list of
diff --git a/gdb/unittests/command-def-selftests.c b/gdb/unittests/command-def-selftests.c
index 4a6b6789b370..e99eb9bbb75d 100644
--- a/gdb/unittests/command-def-selftests.c
+++ b/gdb/unittests/command-def-selftests.c
@@ -83,7 +83,7 @@ check_doc (struct cmd_list_element *commandlist, const char *prefix)
       /* Check if this command has subcommands and is not an
 	 abbreviation.  We skip checking subcommands of abbreviations
 	 in order to avoid duplicates in the output.  */
-      if (c->subcommands != NULL && !c->abbrev_flag)
+      if (c->is_prefix () && !c->abbrev_flag)
 	{
 	  /* Recursively call ourselves on the subcommand list,
 	     passing the right prefix in.  */
@@ -155,7 +155,7 @@ traverse_command_structure (struct cmd_list_element **list,
     {
       /* If this command has subcommands and is not an alias,
 	 traverse the subcommands.  */
-      if (c->subcommands != NULL && !c->is_alias ())
+      if (c->is_prefix () && !c->is_alias ())
 	{
 	  /* Recursively call ourselves on the subcommand list,
 	     passing the right prefix in.  */
-- 
2.31.1

[PATCH 7/7] gdb: add cmd_list_element::is_command_class_help

[Simon Marchi]

Same idea as the previous patches, but for whether a command is a
"command class help" command.  I think this one is particularly useful,
because it's not obvious when reading code what "c->func == NULL" means.

Remove the cmd_func_p function, which does kind of the same thing as
cmd_list_element::is_command_class_help (except it doesn't give a clue
about the semantic of a NULL func value).

gdb/ChangeLog:

	* cli/cli-decode.h (cmd_list_element) <is_command_class_help>:
	New, use it.
	* command.h (cmd_func_p): Remove.
	* cli/cli-decode.c (cmd_func_p): Remove.

Change-Id: I521a3e1896dc93a5babe1493d18f5eb071e1b3b7
---
 gdb/cli/cli-decode.c | 25 ++++++++-----------------
 gdb/cli/cli-decode.h |  6 ++++++
 gdb/command.h        |  3 ---
 gdb/top.c            |  2 +-
 4 files changed, 15 insertions(+), 21 deletions(-)

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 785e726a81f6..fbead7004ed4 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -1250,7 +1250,7 @@ help_cmd (const char *command, struct ui_file *stream)
   fputs_filtered (c->doc, stream);
   fputs_filtered ("\n", stream);
 
-  if (!c->is_prefix () && c->func != NULL)
+  if (!c->is_prefix () && !c->is_command_class_help ())
     return;
 
   fprintf_filtered (stream, "\n");
@@ -1261,7 +1261,7 @@ help_cmd (const char *command, struct ui_file *stream)
 	       all_commands, stream);
 
   /* If this is a class name, print all of the commands in the class.  */
-  if (c->func == NULL)
+  if (c->is_command_class_help ())
     help_list (cmdlist, "", c->theclass, stream);
 
   if (c->hook_pre || c->hook_post)
@@ -1362,7 +1362,7 @@ help_all (struct ui_file *stream)
       /* If this is a class name, print all of the commands in the
 	 class.  */
 
-      if (c->func == NULL)
+      if (c->is_command_class_help ())
 	{
 	  fprintf_filtered (stream, "\nCommand class: %s\n\n", c->name);
 	  help_cmd_list (cmdlist, c->theclass, true, stream);
@@ -1498,8 +1498,8 @@ help_cmd_list (struct cmd_list_element *list, enum command_class theclass,
 	}
 
       if (theclass == all_commands
-	  || (theclass == all_classes && c->func == NULL)
-	  || (theclass == c->theclass && c->func != NULL))
+	  || (theclass == all_classes && c->is_command_class_help ())
+	  || (theclass == c->theclass && !c->is_command_class_help ()))
 	{
 	  /* show C when
 	     - showing all commands
@@ -1545,7 +1545,7 @@ find_cmd (const char *command, int len, struct cmd_list_element *clist,
   *nfound = 0;
   for (c = clist; c; c = c->next)
     if (!strncmp (command, c->name, len)
-	&& (!ignore_help_classes || c->func))
+	&& (!ignore_help_classes || !c->is_command_class_help ()))
       {
 	found = c;
 	(*nfound)++;
@@ -2124,7 +2124,7 @@ complete_on_cmdlist (struct cmd_list_element *list,
       for (ptr = list; ptr; ptr = ptr->next)
 	if (!strncmp (ptr->name, text, textlen)
 	    && !ptr->abbrev_flag
-	    && (!ignore_help_classes || ptr->func
+	    && (!ignore_help_classes || !ptr->is_command_class_help ()
 		|| ptr->is_prefix ()))
 	  {
 	    if (pass == 0)
@@ -2174,20 +2174,11 @@ complete_on_enum (completion_tracker &tracker,
       tracker.add_completion (make_completion_match_str (name, text, word));
 }
 
-
-/* Check function pointer.  */
-int
-cmd_func_p (struct cmd_list_element *cmd)
-{
-  return (cmd->func != NULL);
-}
-
-
 /* Call the command function.  */
 void
 cmd_func (struct cmd_list_element *cmd, const char *args, int from_tty)
 {
-  if (cmd_func_p (cmd))
+  if (!cmd->is_command_class_help ())
     {
       gdb::optional<scoped_restore_tmpl<int>> restore_suppress;
 
diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
index 6204ed745e00..9328659775ca 100644
--- a/gdb/cli/cli-decode.h
+++ b/gdb/cli/cli-decode.h
@@ -87,6 +87,12 @@ struct cmd_list_element
   bool is_prefix () const
   { return this->subcommands != nullptr; }
 
+  /* Return true if this command is a "command class help" command.  For
+     instance, a "stack" dummy command is registered so that one can do
+     "help stack" and show help for all commands of the "stack" class.  */
+  bool is_command_class_help () const
+  { return this->func == nullptr; }
+
   /* Points to next command in this list.  */
   struct cmd_list_element *next = nullptr;
 
diff --git a/gdb/command.h b/gdb/command.h
index fe3c7b482f04..685038716a41 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -595,9 +595,6 @@ extern void save_command_line (const char *cmd);
 
 extern void not_just_help_class_command (const char *, int);
 
-/* Check function pointer.  */
-extern int cmd_func_p (struct cmd_list_element *cmd);
-
 /* Call the command function.  */
 extern void cmd_func (struct cmd_list_element *cmd,
 		      const char *args, int from_tty);
diff --git a/gdb/top.c b/gdb/top.c
index a1a932ae93e9..b9635368cacb 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -666,7 +666,7 @@ execute_command (const char *p, int from_tty)
 	do_set_command (arg, from_tty, c);
       else if (c->type == show_cmd)
 	do_show_command (arg, from_tty, c);
-      else if (!cmd_func_p (c))
+      else if (c->is_command_class_help ())
 	error (_("That is not a command, just a help topic."));
       else if (deprecated_call_command_hook)
 	deprecated_call_command_hook (c, arg, from_tty);
-- 
2.31.1

Re: [PATCH 0/7] Minor improvements to cmd_list_element

[Tom Tromey]

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> While trying to fix some bug, I read a lot of the command-related code,
Simon> hoping to find ways to make it cleaner and simpler.  I didn't really
Simon> succeed, but I managed to gather these small improvements that I think
Simon> slightly help readability.

I sent a note on one of these, but I read them all and they look good.
Thanks for doing this, the CLI code can really use the help.

Tom

Re: [PATCH 0/7] Minor improvements to cmd_list_element

[Simon Marchi]

On 2021-05-16 9:40 a.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> While trying to fix some bug, I read a lot of the command-related code,
> Simon> hoping to find ways to make it cleaner and simpler.  I didn't really
> Simon> succeed, but I managed to gather these small improvements that I think
> Simon> slightly help readability.
> 
> I sent a note on one of these, but I read them all and they look good.
> Thanks for doing this, the CLI code can really use the help.

Thanks for that comment, it made me look into that case a bit more in
depth.  I'm reasonably convinced that the current code doesn't make
sense and my patch is a (small) improvement, so I went ahead and pushed
it.  If you still have doubts, it can still be changed.

Simon