[2/2] gdb: use option framework for add-inferior and clone-inferior

Message ID c7cf4954483b782f2d95cbe862fbc8e83b9ce186.1727786798.git.aburgess@redhat.com
State New
Headers
Series Filename option support |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed

Commit Message

Andrew Burgess Oct. 1, 2024, 12:49 p.m. UTC
  Convert the add-inferior and clone-inferior commands to make use of
the option framework.  This improves the tab completion for these
commands.

Previously the add-inferior command used a trick to simulate
completion of -exec argument.  The command use filename completion for
everything on the command line, thus you could do:

  (gdb) add-inferior /path/to/some/fil<TAB>

and GDB would complete the file name, even though add-inferior doesn't
really take a filename as an argument.  This helped a little though
because, if the user did this:

  (gdb) add-inferior -exec /path/to/some/fil<TAB>

then the file name would be completed.  However, GDB didn't really
understand the options, so couldn't offer completion of the options
themselves.

After this commit, the add-inferior command makes use of the recently
added gdb::option::filename_option_def feature.  This means that the
user now has full completion of the option names, and that file names
will still complete for the '-exec' option, but will no longer
complete if the '-exec' option is not used.

I have also converted the clone-inferior command, though this command
does not use any file name options.  This command does now have proper
completion of the command options.
---
 gdb/inferior.c | 291 ++++++++++++++++++++++++++++++++-----------------
 1 file changed, 188 insertions(+), 103 deletions(-)
  

Patch

diff --git a/gdb/inferior.c b/gdb/inferior.c
index 21f37c8313c..57c8383b0f3 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -33,7 +33,7 @@ 
 #include "arch-utils.h"
 #include "target-descriptions.h"
 #include "target-connection.h"
-#include "readline/tilde.h"
+#include "gdbsupport/gdb_tilde_expand.h"
 #include "progspace-and-thread.h"
 #include "gdbsupport/buildargv.h"
 #include "cli/cli-style.h"
@@ -891,127 +891,207 @@  switch_to_inferior_and_push_target (inferior *new_inf,
     gdb_printf (_("Added inferior %d\n"), new_inf->num);
 }
 
+/* Option values for the "add-inferior" command.  */
+
+struct add_inferior_opts
+{
+  /* When true the new inferiors are started without a connection.  */
+  bool no_connection = false;
+
+  /* The number of new inferiors to add.  */
+  unsigned int num_copies = 1;
+
+  /* When non-empty, this is the executable for the new inferiors.  */
+  std::string exec_filename;
+};
+
+/* Option definitions for the "add-inferior" command.  */
+
+static const gdb::option::option_def add_inferior_option_defs[] = {
+  gdb::option::flag_option_def<add_inferior_opts> {
+    "no-connection",
+    [] (add_inferior_opts *opts) { return &opts->no_connection; },
+    N_("\
+If specified, the new inferiors begin with no target connection.\n\
+Without this flag the new inferiors inherit the current inferior's\n\
+connection."),
+  },
+
+  gdb::option::uinteger_option_def<add_inferior_opts> {
+    "copies",
+    [] (add_inferior_opts *opts) { return &opts->num_copies; },
+    (show_value_ftype *) nullptr, /* show_cmd_cb */
+    N_("\
+The number of inferiors to add.  The default is 1."),
+  },
+
+  gdb::option::filename_option_def<add_inferior_opts> {
+    "exec",
+    [] (add_inferior_opts *opts) { return &opts->exec_filename; },
+    nullptr, /* show_cmd_cb */
+    N_("\
+FILENAME is the file name of the executable to use as the\n\
+main program."),
+  },
+};
+
+/* Create the option_def_group for the "add-inferior" command.  */
+
+static inline gdb::option::option_def_group
+make_add_inferior_options_def_group (add_inferior_opts *opts)
+{
+  return {{add_inferior_option_defs}, opts};
+}
+
+/* Completion for the "add-inferior" command.  */
+
+static void
+add_inferior_completer (struct cmd_list_element *cmd,
+			completion_tracker &tracker,
+			const char *text, const char * /* word */)
+{
+  /* The only completion offered is for the command options.  */
+  const auto group = make_add_inferior_options_def_group (nullptr);
+  gdb::option::complete_options
+    (tracker, &text, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_ERROR, group);
+}
+
 /* add-inferior [-copies N] [-exec FILENAME] [-no-connection] */
 
 static void
 add_inferior_command (const char *args, int from_tty)
 {
-  int i, copies = 1;
-  gdb::unique_xmalloc_ptr<char> exec;
-  symfile_add_flags add_flags = 0;
-  bool no_connection = false;
+  add_inferior_opts opts;
+  const auto group = make_add_inferior_options_def_group (&opts);
+  gdb::option::process_options
+    (&args, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_ERROR, group);
+
+  /* If an executable was given then perform tilde expansion.  */
+  if (!opts.exec_filename.empty ())
+    opts.exec_filename = gdb_tilde_expand (opts.exec_filename);
 
+  symfile_add_flags add_flags = 0;
   if (from_tty)
     add_flags |= SYMFILE_VERBOSE;
 
-  if (args)
-    {
-      gdb_argv built_argv (args);
-
-      for (char **argv = built_argv.get (); *argv != NULL; argv++)
-	{
-	  if (**argv == '-')
-	    {
-	      if (strcmp (*argv, "-copies") == 0)
-		{
-		  ++argv;
-		  if (!*argv)
-		    error (_("No argument to -copies"));
-		  copies = parse_and_eval_long (*argv);
-		}
-	      else if (strcmp (*argv, "-no-connection") == 0)
-		no_connection = true;
-	      else if (strcmp (*argv, "-exec") == 0)
-		{
-		  ++argv;
-		  if (!*argv)
-		    error (_("No argument to -exec"));
-		  exec.reset (tilde_expand (*argv));
-		}
-	    }
-	  else
-	    error (_("Invalid argument"));
-	}
-    }
-
   inferior *orginf = current_inferior ();
 
   scoped_restore_current_pspace_and_thread restore_pspace_thread;
 
-  for (i = 0; i < copies; ++i)
+  for (unsigned int i = 0; i < opts.num_copies; ++i)
     {
       inferior *inf = add_inferior_with_spaces ();
 
-      switch_to_inferior_and_push_target (inf, no_connection, orginf);
+      switch_to_inferior_and_push_target (inf, opts.no_connection, orginf);
 
-      if (exec != NULL)
+      if (!opts.exec_filename.empty ())
 	{
-	  exec_file_attach (exec.get (), from_tty);
-	  symbol_file_add_main (exec.get (), add_flags);
+	  const char *exec = opts.exec_filename.c_str ();
+	  exec_file_attach (exec, from_tty);
+	  symbol_file_add_main (exec, add_flags);
 	}
     }
 }
 
-/* clone-inferior [-copies N] [ID] [-no-connection] */
+/* Option values for the "clone-inferior" command.  */
+
+struct clone_inferior_opts
+{
+  /* When true the new inferiors are started without a connection.  */
+  bool no_connection = false;
+
+  /* The number of new inferiors to create by cloning.  */
+  unsigned int num_copies = 1;
+};
+
+
+/* Option definitions for the "clone-inferior" command.  */
+
+static const gdb::option::option_def clone_inferior_option_defs[] = {
+  gdb::option::flag_option_def<clone_inferior_opts> {
+    "no-connection",
+    [] (clone_inferior_opts *opts) { return &opts->no_connection; },
+    N_("\
+If specified, the new inferiors begin with no target connection.\n\
+Without this flag the new inferiors to inherit the copied inferior's\n\
+connection."),
+  },
+
+  gdb::option::uinteger_option_def<clone_inferior_opts> {
+    "copies",
+    [] (clone_inferior_opts *opts) { return &opts->num_copies; },
+    (show_value_ftype *) nullptr, /* show_cmd_cb */
+    N_("\
+The number of copies of inferior ID to create.  The default is 1."),
+  },
+};
+
+/* Create the option_def_group for the "clone-inferior" command.  */
+
+static inline gdb::option::option_def_group
+make_clone_inferior_options_def_group (clone_inferior_opts *opts)
+{
+  return {{clone_inferior_option_defs}, opts};
+}
+
+/* Completion for the "clone-inferior" command.  */
+
+static void
+clone_inferior_completer (struct cmd_list_element *cmd,
+			completion_tracker &tracker,
+			const char *text, const char * /* word */)
+{
+  /* The only completion offered is for the command options.  */
+  const auto group = make_clone_inferior_options_def_group (nullptr);
+  gdb::option::complete_options
+    (tracker, &text, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, group);
+}
+
+/* clone-inferior [-copies N] [-no-connection] [ID] */
 
 static void
 clone_inferior_command (const char *args, int from_tty)
 {
-  int i, copies = 1;
-  struct inferior *orginf = NULL;
-  bool no_connection = false;
+  clone_inferior_opts opts;
+  const auto group = make_clone_inferior_options_def_group (&opts);
+  gdb::option::process_options
+    (&args, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, group);
 
-  if (args)
+  struct inferior *orginf = NULL;
+  if (args != nullptr && *args != '\0')
     {
-      gdb_argv built_argv (args);
+      gdb_argv argv (args);
 
-      char **argv = built_argv.get ();
-      for (; *argv != NULL; argv++)
+      gdb_assert (argv.count () > 0);
+
+      for (const char *arg : argv)
 	{
-	  if (**argv == '-')
+	  if (orginf == nullptr)
 	    {
-	      if (strcmp (*argv, "-copies") == 0)
-		{
-		  ++argv;
-		  if (!*argv)
-		    error (_("No argument to -copies"));
-		  copies = parse_and_eval_long (*argv);
-
-		  if (copies < 0)
-		    error (_("Invalid copies number"));
-		}
-	      else if (strcmp (*argv, "-no-connection") == 0)
-		no_connection = true;
+	      /* The first non-option argument specifies the number of the
+		 inferior to clone.  */
+	      int num = parse_and_eval_long (arg);
+	      orginf = find_inferior_id (num);
+
+	      if (orginf == nullptr)
+		error (_("Inferior ID %d not known."), num);
 	    }
 	  else
-	    {
-	      if (orginf == NULL)
-		{
-		  int num;
-
-		  /* The first non-option (-) argument specified the
-		     program space ID.  */
-		  num = parse_and_eval_long (*argv);
-		  orginf = find_inferior_id (num);
-
-		  if (orginf == NULL)
-		    error (_("Inferior ID %d not known."), num);
-		  continue;
-		}
-	      else
-		error (_("Invalid argument"));
-	    }
+	    error (_("Unexpected argument: %s."), arg);
 	}
     }
+  else
+    {
+      /* If no inferior id was specified, then the user wants to clone the
+	 current inferior.  */
+      orginf = current_inferior ();
+    }
 
-  /* If no inferior id was specified, then the user wants to clone the
-     current inferior.  */
-  if (orginf == NULL)
-    orginf = current_inferior ();
+  gdb_assert (orginf != nullptr);
 
   scoped_restore_current_pspace_and_thread restore_pspace_thread;
 
-  for (i = 0; i < copies; ++i)
+  for (unsigned int i = 0; i < opts.num_copies; ++i)
     {
       struct program_space *pspace;
       struct inferior *inf;
@@ -1025,7 +1105,7 @@  clone_inferior_command (const char *args, int from_tty)
       inf->aspace = pspace->aspace;
       inf->set_arch (orginf->arch ());
 
-      switch_to_inferior_and_push_target (inf, no_connection, orginf);
+      switch_to_inferior_and_push_target (inf, opts.no_connection, orginf);
 
       /* If the original inferior had a user specified target
 	 description, make the clone use it too.  */
@@ -1107,31 +1187,36 @@  Usage: info inferiors [ID]...\n\
 If IDs are specified, the list is limited to just those inferiors.\n\
 By default all inferiors are displayed."));
 
-  c = add_com ("add-inferior", no_class, add_inferior_command, _("\
+  const auto add_inf_opts = make_add_inferior_options_def_group (nullptr);
+  static std::string add_inferior_command_help
+    = gdb::option::build_help (_("\
 Add a new inferior.\n\
-Usage: add-inferior [-copies N] [-exec FILENAME] [-no-connection]\n\
-N is the optional number of inferiors to add, default is 1.\n\
-FILENAME is the file name of the executable to use\n\
-as main program.\n\
-By default, the new inferior inherits the current inferior's connection.\n\
-If -no-connection is specified, the new inferior begins with\n\
-no target connection yet."));
-  set_cmd_completer (c, deprecated_filename_completer);
+Usage: add-inferior [-copies NUMBER] [-exec FILENAME] [-no-connection]\n\
+\n\
+Options:\n\
+%OPTIONS%"), add_inf_opts);
+  c = add_com ("add-inferior", no_class, add_inferior_command,
+	       add_inferior_command_help.c_str ());
+  set_cmd_completer_handle_brkchars (c, add_inferior_completer);
 
   add_com ("remove-inferiors", no_class, remove_inferior_command, _("\
 Remove inferior ID (or list of IDs).\n\
 Usage: remove-inferiors ID..."));
 
-  add_com ("clone-inferior", no_class, clone_inferior_command, _("\
-Clone inferior ID.\n\
-Usage: clone-inferior [-copies N] [-no-connection] [ID]\n\
-Add N copies of inferior ID.  The new inferiors have the same\n\
-executable loaded as the copied inferior.  If -copies is not specified,\n\
-adds 1 copy.  If ID is not specified, it is the current inferior\n\
-that is cloned.\n\
-By default, the new inferiors inherit the copied inferior's connection.\n\
-If -no-connection is specified, the new inferiors begin with\n\
-no target connection yet."));
+  const auto clone_inf_opts = make_clone_inferior_options_def_group (nullptr);
+  static std::string clone_inferior_command_help
+    = gdb::option::build_help (_("\
+Clone an existing inferior.\n\
+Usage: clone-inferior [-copies NUMBER] [-no-connection] [ID]\n\
+ID is the inferior number to clone, this can be found with the\n\
+'info inferiors' command.  If no ID is specified, then the current\n\
+inferior is cloned.\n\
+\n\
+Options:\n\
+%OPTIONS%"), clone_inf_opts);
+  c = add_com ("clone-inferior", no_class, clone_inferior_command,
+	       clone_inferior_command_help.c_str ());
+  set_cmd_completer_handle_brkchars (c, clone_inferior_completer);
 
   add_cmd ("inferiors", class_run, detach_inferior_command, _("\
 Detach from inferior ID (or list of IDS).\n\