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

[Christian Biesinger <cbiesinger@google.com>]

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.

> 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
>