Re: [PATCH 1/2] Move the code to execute a cmd_list_element out from execute_command

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

Hi,

Thanks for working on this.  I have a little feedback:

* Marco Barisione <mbarisione@undo.io> [2020-09-14 09:39:24 +0000]:

You should imagine this patch, once applied is being looked at in
isolation, without an understanding that there's a follow on patch.
As such it's usually a nice idea to write a short commit message that
just mentions you're performing this restructuring in preparation for
a follow on patch.

> gdb/ChangeLog:
> 
> 	* gdbcmd.h (execute_cmd_list_command): Add declaration.
> 	* top.c (execute_command): Move out the code to execute a
> 	command from a cmd_list_element.
> 	(execute_cmd_list_command): Add from code originally in
> 	execute_command.
> 	* top.h (execute_cmd_list_command): Add declaration.

Why is execute_cmd_list_command declared twice?  This doesn't feel
right.

Thanks,
Andrew

> ---
>  gdb/gdbcmd.h |  3 +++
>  gdb/top.c    | 66 ++++++++++++++++++++++++++++++----------------------
>  gdb/top.h    |  2 ++
>  3 files changed, 43 insertions(+), 28 deletions(-)
> 
> diff --git a/gdb/gdbcmd.h b/gdb/gdbcmd.h
> index 4406094ea5..5be474ec1a 100644
> --- a/gdb/gdbcmd.h
> +++ b/gdb/gdbcmd.h
> @@ -134,6 +134,9 @@ extern struct cmd_list_element *save_cmdlist;
>  
>  extern void execute_command (const char *, int);
>  
> +extern void execute_cmd_list_command (struct cmd_list_element *,
> +				      const char *, int);
> +
>  /* Execute command P and returns its output.  If TERM_OUT,
>     the output is built using terminal output behaviour such
>     as cli_styling.  */
> diff --git a/gdb/top.c b/gdb/top.c
> index acd31afb9a..8eae15a488 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -581,7 +581,6 @@ execute_command (const char *p, int from_tty)
>        const char *arg;
>        std::string default_args;
>        std::string default_args_and_arg;
> -      int was_sync = current_ui->prompt_state == PROMPT_BLOCKED;
>  
>        line = p;
>  
> @@ -641,33 +640,7 @@ execute_command (const char *p, int from_tty)
>        if (c->deprecated_warn_user)
>  	deprecated_cmd_warning (line);
>  
> -      /* c->user_commands would be NULL in the case of a python command.  */
> -      if (c->theclass == class_user && c->user_commands)
> -	execute_user_command (c, arg);
> -      else if (c->theclass == class_user
> -	       && c->prefixlist && !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
> -	   subcommands.  */
> -	{
> -	  printf_unfiltered
> -	    ("\"%.*s\" must be followed by the name of a subcommand.\n",
> -	     (int) strlen (c->prefixname) - 1, c->prefixname);
> -	  help_list (*c->prefixlist, c->prefixname, all_commands, gdb_stdout);
> -	}
> -      else if (c->type == set_cmd)
> -	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))
> -	error (_("That is not a command, just a help topic."));
> -      else if (deprecated_call_command_hook)
> -	deprecated_call_command_hook (c, arg, from_tty);
> -      else
> -	cmd_func (c, arg, from_tty);
> -
> -      maybe_wait_sync_command_done (was_sync);
> +      execute_cmd_list_command (c, arg, from_tty);
>  
>        /* If this command has been post-hooked, run the hook last.  */
>        execute_cmd_post_hook (c);
> @@ -690,6 +663,43 @@ execute_command (const char *p, int from_tty)
>    cleanup_if_error.release ();
>  }
>  
> +/* Execute the C command.  */
> +
> +void
> +execute_cmd_list_command (struct cmd_list_element *c, const char *arg,
> +			  int from_tty)
> +{
> +  bool was_sync = current_ui->prompt_state == PROMPT_BLOCKED;
> +
> +  /* c->user_commands would be NULL in the case of a python command.  */
> +  if (c->theclass == class_user && c->user_commands)
> +    execute_user_command (c, arg);
> +  else if (c->theclass == class_user
> +	   && c->prefixlist && !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
> +	 subcommands.  */
> +      printf_unfiltered
> +	("\"%.*s\" must be followed by the name of a subcommand.\n",
> +	 (int) strlen (c->prefixname) - 1, c->prefixname);
> +      help_list (*c->prefixlist, c->prefixname, all_commands, gdb_stdout);
> +    }
> +  else if (c->type == set_cmd)
> +    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))
> +    error (_("That is not a command, just a help topic."));
> +  else if (deprecated_call_command_hook)
> +    deprecated_call_command_hook (c, arg, from_tty);
> +  else
> +    cmd_func (c, arg, from_tty);
> +
> +  maybe_wait_sync_command_done (was_sync);
> +}
> +
>  /* Run execute_command for P and FROM_TTY.  Sends its output to FILE,
>     do not display it to the screen.  BATCH_FLAG will be
>     temporarily set to true.  */
> diff --git a/gdb/top.h b/gdb/top.h
> index fd99297715..ba6c596da7 100644
> --- a/gdb/top.h
> +++ b/gdb/top.h
> @@ -241,6 +241,8 @@ extern void quit_force (int *, int);
>  extern void quit_command (const char *, int);
>  extern void quit_cover (void);
>  extern void execute_command (const char *, int);
> +extern void execute_cmd_list_command (struct cmd_list_element *,
> +				      const char *, int);
>  
>  /* If the interpreter is in sync mode (we're running a user command's
>     list, running command hooks or similars), and we just ran a
> -- 
> 2.20.1
>