Re: [PATCH 3/3] Add completer to the add-inferior command

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Lancelot SIX via Gdb-patches <gdb-patches@sourceware.org> [2021-02-13 22:07:52 +0000]:

> This commit adds a completer to the add-inferior command.  Argument
> parsing is also updated to benefit from what is offered by the
> gdb::option framework.
> 
> gdb/Changelog:
> 
> 	* inferior.c (struct add_inferior_options): Add struct.
> 	(add_inferior_options_defs): Create.
> 	(make_add_inferior_options_def_group): Create.
> 	(add_inferior_completer): Create.
> 	(add_inferior_command): Use gdb::option parsing for arguments.
> 	(initialize_inferiors): Use new completer for add-inferior
> 	command.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.base/completion.exp: Test add-completion completion.
> ---
>  gdb/inferior.c                        | 123 ++++++++++++++++++--------
>  gdb/testsuite/gdb.base/completion.exp |  11 +++
>  2 files changed, 96 insertions(+), 38 deletions(-)
> 
> diff --git a/gdb/inferior.c b/gdb/inferior.c
> index 49f869a4c78..6993d043a57 100644
> --- a/gdb/inferior.c
> +++ b/gdb/inferior.c
> @@ -788,63 +788,110 @@ switch_to_inferior_and_push_target (inferior *new_inf,
>      printf_filtered (_("Added inferior %d\n"), new_inf->num);
>  }
>  
> +/* Describes the options for the add-inferior command.  */
> +
> +struct add_inferior_options
> +{
> +    /* Number new inferiors to create.  */
> +    unsigned int copies = 1;
> +
> +    /* Instructs to start the inferior with no target connected.  */
> +    bool no_connection = false;
> +
> +    /* Path to the file name of the executable to use as main program.  */
> +    char *exec = nullptr;
> +
> +    ~add_inferior_options ()
> +      {
> +        if (exec != nullptr)
> +          {
> +            xfree (exec);
> +            exec = nullptr;
> +          }
> +      }
> +};
> +
> +/* Definition of options for the 'add-inferior' command.  */
> +
> +static const gdb::option::option_def add_inferior_options_defs[] = {
> +  gdb::option::zuinteger_option_def<add_inferior_options> {
> +      "copies",
> +      [] (add_inferior_options *opt) { return &opt->copies; },
> +      nullptr, /* show_cmd_cb */
> +      nullptr, /* set_doc */
> +      nullptr, /* show_doc */
> +      nullptr, /* help_doc */
> +  },
> +
> +  gdb::option::flag_option_def<add_inferior_options> {
> +      "no-connection",
> +      [] (add_inferior_options *opt) { return &opt->no_connection; },
> +      nullptr, /* set_doc */
> +      nullptr, /* help_doc */
> +  },
> +
> +  gdb::option::filename_option_def<add_inferior_options> {
> +      "exec",
> +      [] (add_inferior_options *opt) { return &opt->exec; },
> +      nullptr, /* show_cmd_cb */
> +      nullptr, /* set_doc */
> +      nullptr, /* show_doc */
> +      nullptr, /* help_doc */
> +  },
> +
> +};
> +
> +static gdb::option::option_def_group
> +make_add_inferior_options_def_group (struct add_inferior_options *opts)
> +{
> +  return {{add_inferior_options_defs}, opts};
> +}
> +
> +/* Completer for the add-inferior command.  */
> +
> +static void
> +add_inferior_completer (struct cmd_list_element *cmd,
> +			completion_tracker &tracker,
> +			const char *text, const char * /*word*/)
> +{
> +  const auto group = make_add_inferior_options_def_group (nullptr);
> +  if (gdb::option::complete_options
> +      (tracker, &text, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, group))
> +    return;
> +}
> +
>  /* add-inferior [-copies N] [-exec FILENAME] [-no-connection] */
>  
>  static void
>  add_inferior_command (const char *args, int from_tty)
>  {
> -  int i, copies = 1;
> +  int i;
>    gdb::unique_xmalloc_ptr<char> exec;
>    symfile_add_flags add_flags = 0;
> -  bool no_connection = false;
> +
> +  add_inferior_options opts;
> +  const gdb::option::option_def_group group
> +    = make_add_inferior_options_def_group (&opts);
> +  gdb::option::process_options
> +    (&args, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_ERROR, group);
>  
>    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 (i = 0; i < opts.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.filename != nullptr)
>  	{
> -	  exec_file_attach (exec.get (), from_tty);
> -	  symbol_file_add_main (exec.get (), add_flags);
> +	  exec_file_attach (opts.filename, from_tty);
> +	  symbol_file_add_main (opts.filename, add_flags);

This patch doesn't build for me as opts.filename is unknown.  Did you
mean opts.exec maybe?

Thanks,
Andrew



>  	}
>      }
>  }
> @@ -997,7 +1044,7 @@ 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, filename_completer);
> +  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\
> diff --git a/gdb/testsuite/gdb.base/completion.exp b/gdb/testsuite/gdb.base/completion.exp
> index 076134cdc33..c83aa8994e2 100644
> --- a/gdb/testsuite/gdb.base/completion.exp
> +++ b/gdb/testsuite/gdb.base/completion.exp
> @@ -980,3 +980,14 @@ test_gdb_complete_unique "xxx_yyy_" "xxx_yyy_zzz"
>  gdb_test_no_output "alias set aaa_bbb_ccc=set debug"
>  gdb_test_no_output "maint deprecate set aaa_bbb_ccc"
>  test_gdb_complete_unique "set aaa_bbb_" "set aaa_bbb_ccc"
> +
> +# Test the completer of the add-inferior command.
> +gdb_test_no_output "set max-completions unlimited"
> +test_gdb_complete_multiple "add-inferior " "-" "" {
> +    "-copies"
> +    "-exec"
> +    "-no-connection"
> +}
> +test_gdb_complete_unique \
> +    "add-inferior -copies 2 -exec some\\ file -n" \
> +    "add-inferior -copies 2 -exec some\\ file -no-connection"
> -- 
> 2.29.2
>