Re: [PATCH 0/4] gdb: Move construct_inferior_arguments to gdbsupport

[Simon Marchi <simark@simark.ca>]

On 2020-04-29 7:16 a.m., Michael Weghorn via Gdb-patches wrote:
> This moves the function construct_inferior_arguments from
> gdb/inferior.h and gdb/infcmd.c to gdbsupport/common-inferior.{h,cc}.
> 
> The intention is to use it from gdbserver in a follow-up commit.
> 
> gdb/ChangeLog:
> 
> 2020-04-29  Michael Weghorn  <m.weghorn@posteo.de>
> 
>         * infcmd.c, inferior.h: (construct_inferior_arguments):
>           Moved function from here to gdbsupport/common-inferior.{h,cc}
> 
> gdbsupport/ChangeLog:
> 
> 2020-04-29  Michael Weghorn  <m.weghorn@posteo.de>
> 
>         * common-inferior.h, common-inferior.cc: (construct_inferior_arguments):
>         Move function here from gdb/infcmd.c, gdb/inferior.h
> ---
>  gdb/infcmd.c                  | 124 ----------------------------------
>  gdb/inferior.h                |   2 -
>  gdbsupport/common-inferior.cc | 123 +++++++++++++++++++++++++++++++++
>  gdbsupport/common-inferior.h  |   2 +
>  4 files changed, 125 insertions(+), 126 deletions(-)
> 
> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index 9bbb413d4e..8f7482347c 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -259,130 +259,6 @@ server's cwd if remote debugging.\n"));
>  			"when starting the inferior is \"%s\".\n"), cwd);
>  }
>  
> -\f
> -/* Compute command-line string given argument vector.  This does the
> -   same shell processing as fork_inferior.  */
> -
> -char *
> -construct_inferior_arguments (int argc, char **argv)
> -{
> -  char *result;
> -
> -  /* 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);
> -
> -  if (startup_with_shell)
> -    {
> -#ifdef __MINGW32__
> -      /* This holds all the characters considered special to the
> -	 Windows shells.  */
> -      static const char special[] = "\"!&*|[]{}<>?`~^=;, \t\n";
> -      static const char quote = '"';
> -#else
> -      /* This holds all the characters considered special to the
> -	 typical Unix shells.  We include `^' because the SunOS
> -	 /bin/sh treats it as a synonym for `|'.  */
> -      static const char special[] = "\"!#$&*()\\|[]{}<>?'`~^; \t\n";
> -      static const char quote = '\'';
> -#endif
> -      int i;
> -      int length = 0;
> -      char *out, *cp;
> -
> -      /* We over-compute the size.  It shouldn't matter.  */
> -      for (i = 0; i < argc; ++i)
> -	length += 3 * strlen (argv[i]) + 1 + 2 * (argv[i][0] == '\0');
> -
> -      result = (char *) xmalloc (length);
> -      out = result;
> -
> -      for (i = 0; i < argc; ++i)
> -	{
> -	  if (i > 0)
> -	    *out++ = ' ';
> -
> -	  /* Need to handle empty arguments specially.  */
> -	  if (argv[i][0] == '\0')
> -	    {
> -	      *out++ = quote;
> -	      *out++ = quote;
> -	    }
> -	  else
> -	    {
> -#ifdef __MINGW32__
> -	      int quoted = 0;
> -
> -	      if (strpbrk (argv[i], special))
> -		{
> -		  quoted = 1;
> -		  *out++ = quote;
> -		}
> -#endif
> -	      for (cp = argv[i]; *cp; ++cp)
> -		{
> -		  if (*cp == '\n')
> -		    {
> -		      /* A newline cannot be quoted with a backslash (it
> -			 just disappears), only by putting it inside
> -			 quotes.  */
> -		      *out++ = quote;
> -		      *out++ = '\n';
> -		      *out++ = quote;
> -		    }
> -		  else
> -		    {
> -#ifdef __MINGW32__
> -		      if (*cp == quote)
> -#else
> -		      if (strchr (special, *cp) != NULL)
> -#endif
> -			*out++ = '\\';
> -		      *out++ = *cp;
> -		    }
> -		}
> -#ifdef __MINGW32__
> -	      if (quoted)
> -		*out++ = quote;
> -#endif
> -	    }
> -	}
> -      *out = '\0';
> -    }
> -  else
> -    {
> -      /* In this case we can't handle arguments that contain spaces,
> -	 tabs, or newlines -- see breakup_args().  */
> -      int i;
> -      int length = 0;
> -
> -      for (i = 0; i < argc; ++i)
> -	{
> -	  char *cp = strchr (argv[i], ' ');
> -	  if (cp == NULL)
> -	    cp = strchr (argv[i], '\t');
> -	  if (cp == NULL)
> -	    cp = strchr (argv[i], '\n');
> -	  if (cp != NULL)
> -	    error (_("can't handle command-line "
> -		     "argument containing whitespace"));
> -	  length += strlen (argv[i]) + 1;
> -	}
> -
> -      result = (char *) xmalloc (length);
> -      result[0] = '\0';
> -      for (i = 0; i < argc; ++i)
> -	{
> -	  if (i > 0)
> -	    strcat (result, " ");
> -	  strcat (result, argv[i]);
> -	}
> -    }
> -
> -  return result;
> -}
> -\f
>  
>  /* This function strips the '&' character (indicating background
>     execution) that is added as *the last* of the arguments ARGS of a
> diff --git a/gdb/inferior.h b/gdb/inferior.h
> index 1ac51369df..95af474eed 100644
> --- a/gdb/inferior.h
> +++ b/gdb/inferior.h
> @@ -184,8 +184,6 @@ extern void child_interrupt (struct target_ops *self);
>     STARTUP_INFERIOR.  */
>  extern ptid_t gdb_startup_inferior (pid_t pid, int num_traps);
>  
> -extern char *construct_inferior_arguments (int, char **);
> -
>  /* From infcmd.c */
>  
>  /* Initial inferior setup.  Determines the exec file is not yet known,
> diff --git a/gdbsupport/common-inferior.cc b/gdbsupport/common-inferior.cc
> index ed16e89a52..71b9a11e02 100644
> --- a/gdbsupport/common-inferior.cc
> +++ b/gdbsupport/common-inferior.cc
> @@ -24,3 +24,126 @@
>  /* See common-inferior.h.  */
>  
>  bool startup_with_shell = true;
> +
> +/* Compute command-line string given argument vector.  This does the
> +   same shell processing as fork_inferior.  */

Let's just do a bit of spring cleaning and align this code with our current
standards while moving it.  Move the function comment to the .h, and in the.cc
file write:

  /* See common-inferior.h. */

Other than that, this patch LGTM.

Simon