Re: [PATCH 3/7] gdb: new function to wrap up executing command line scripts/commands

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

* Aktemur, Tankut Baris <tankut.baris.aktemur@intel.com> [2020-10-08 15:25:25 +0000]:

> On Wednesday, October 7, 2020 10:05 PM, Andrew Burgess wrote:
> > Small refactor to wrap up executing the scripts and commands passed
> > using the -x, -ex, -ix, -iex command line flags.
> > 
> > There should be no user visible changes after this commit.
> > 
> > gdb/ChangeLog:
> > 
> > 	* main.c (execute_cmdargs): New function.
> > 	(captured_main_1): Make use of execute_cmdargs.
> > ---
> >  gdb/ChangeLog |  5 +++++
> >  gdb/main.c    | 53 ++++++++++++++++++++-------------------------------
> >  2 files changed, 26 insertions(+), 32 deletions(-)
> > 
> > diff --git a/gdb/main.c b/gdb/main.c
> > index f6377f2df28..f85b7dbfdaa 100644
> > --- a/gdb/main.c
> > +++ b/gdb/main.c
> > @@ -515,6 +515,25 @@ struct cmdarg
> >    char *string;
> >  };
> > 
> > +/* From CMDARG_VEC execute command files (matching FILE_TYPE) or commands
> > +   (matching CMD_TYPE).  Update the value in *RET.  */
> 
> Is there any particular reason to have the output parameter?  The function could have 
> returned its result instead of writing it to *ret, right?
> 
> > +static void
> > +execute_cmdargs (const std::vector<struct cmdarg> *cmdarg_vec,
> > +		 cmdarg_kind file_type, cmdarg_kind cmd_type,
> > +		 int *ret)
> > +{
> > +  for (const auto &cmdarg_p : *cmdarg_vec)
> > +    {
> > +      if (cmdarg_p.type == file_type)
> > +	*ret = catch_command_errors (source_script, cmdarg_p.string,
> > +				     !batch_flag);
> > +      else if (cmdarg_p.type == cmd_type)
> > +	*ret = catch_command_errors (execute_command, cmdarg_p.string,
> > +				     !batch_flag);
> > +    }
> 
> Well, it's the original code that was doing it, but I found it a bit odd that
> ret is being overwritten without checking the value until almost the very end
> of captured_main_1.

I find this odd too, but the last thing I wanted to do was combine a
change in behaviour with a restructuring, so I left things as they
were.

I'll update the comment on the function to (hopefully) document the
existing (odd) behaviour a little better.

Thanks,
Andrew