Re: [PATCH 1/4] gdbsupport: Extend construct_inferior_arguments to allow handling all stringify_args cases

Christian Biesinger
2020-04-29T15:46:00Z

On Wed, Apr 29, 2020 at 10:25 AM Christian Biesinger
<cbiesinger@google.com> wrote:
>
> On Wed, Apr 29, 2020 at 6:17 AM Michael Weghorn via Gdb-patches
> <gdb-patches@sourceware.org> wrote:
> >
> > Allow construct_inferior_arguments to handle zero args
> > and have it return a std::string, similar to how
> > stringify_argv in gdbsupport/common-utils does.
> >
> > Also, add a const qualifier for the second parameter,
> > since it is only read, not written to.
> >
> > The intention is to replace some existing uses of
> > stringify_argv by construct_inferior_arguments
> > in a subsequent step, since construct_inferior_arguments
> > properly handles special characters, while stringify_argv
> > doesn't.
>
> Can you clarify which revision your patch is against? My files look
> nothing like the ones you are patching.

Oh, I see now that this patch applies on top of the other patch series you sent.

> > 2020-04-29 Michael Weghorn <m.weghorn@posteo.de>
> >
> > * common-inferior.cc, common-inferior.h (construct_inferior_arguments):
> > Adapt to handle zero args and return a std::string.
> > Adapt call site.
> > ---
> > gdb/infcmd.c | 5 ++---
> > gdbsupport/common-inferior.cc | 19 +++++++++++--------
> > gdbsupport/common-inferior.h | 2 +-
> > 3 files changed, 14 insertions(+), 12 deletions(-)
> >
> > diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> > index 8f7482347c..7ad931f9b4 100644
> > --- a/gdb/infcmd.c
> > +++ b/gdb/infcmd.c
> > @@ -151,12 +151,11 @@ get_inferior_args (void)
> > {
> > if (current_inferior ()->argc != 0)
> > {
> > - char *n;
> > + std::string n;
>
> While changing this, I would just move the declaration to the line
> where it is first used.
>
> >
> > n = construct_inferior_arguments (current_inferior ()->argc,
> > current_inferior ()->argv);
> > - set_inferior_args (n);
> > - xfree (n);
> > + set_inferior_args (n.c_str());
> > }
> >
> > if (current_inferior ()->args == NULL)
> > diff --git a/gdbsupport/common-inferior.cc b/gdbsupport/common-inferior.cc
> > index 71b9a11e02..3f117d5ef0 100644
> > --- a/gdbsupport/common-inferior.cc
> > +++ b/gdbsupport/common-inferior.cc
> > @@ -28,15 +28,15 @@ bool startup_with_shell = true;
> > /* Compute command-line string given argument vector. This does the
> > same shell processing as fork_inferior. */
> >
> > -char *
> > -construct_inferior_arguments (int argc, char **argv)
> > +std::string
> > +construct_inferior_arguments (int argc, char * const *argv)
> > {
> > - char *result;
> > + gdb_assert (argc >= 0);
> > + if (argc == 0 || argv[0] == NULL) {
> > + return "";
> > + }
> >
> > - /* ARGC should always be at least 1, but we double check this
> > - here. This is also needed to silence -Werror-stringop
> > - warnings. */
> > - gdb_assert (argc > 0);
> > + char *result;
> >
> > if (startup_with_shell)
> > {
> > @@ -145,5 +145,8 @@ construct_inferior_arguments (int argc, char **argv)
> > }
> > }
> >
> > - return result;
> > + std::string str_result(result);
> > + xfree (result);
> > +
> > + return str_result;
> > }
> > diff --git a/gdbsupport/common-inferior.h b/gdbsupport/common-inferior.h
> > index eb60c8f13b..0b11e7d6a5 100644
> > --- a/gdbsupport/common-inferior.h
> > +++ b/gdbsupport/common-inferior.h
> > @@ -58,6 +58,6 @@ extern void set_inferior_cwd (const char *cwd);
> > the target is started up with a shell. */
> > extern bool startup_with_shell;
> >
> > -extern char *construct_inferior_arguments (int, char **);
> > +extern std::string construct_inferior_arguments (int, char * const *);
> >
> > #endif /* COMMON_COMMON_INFERIOR_H */
> > --
> > 2.26.2
> >