[PATCH v3 2/6] gdbsupport: Adapt construct_inferior_arguments

[Simon Marchi]

Just some nits.

On 2020-05-12 11:42 a.m., Michael Weghorn via Gdb-patches wrote:
> diff --git a/gdbsupport/common-inferior.cc b/gdbsupport/common-inferior.cc
> index a7d631f357..cadbd50e9c 100644
> --- a/gdbsupport/common-inferior.cc
> +++ b/gdbsupport/common-inferior.cc
> @@ -27,15 +27,12 @@ bool startup_with_shell = true;
>  
>  /* See common-inferior.h.  */
>  
> -char *
> -construct_inferior_arguments (int argc, char **argv)
> +std::string
> +construct_inferior_arguments (int argc, char * const *argv)
>  {
> -  char *result;
> +  gdb_assert (argc >= 0);
>  
> -  /* 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);
> +  std::string result;
>  
>    if (startup_with_shell)
>      {
> @@ -52,48 +49,39 @@ construct_inferior_arguments (int argc, char **argv)
>        static const char quote = '\'';
>  #endif
>        int i;

Can you inline this in the for loop while at it?

> -      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++ = ' ';
> +	    result += ' ';
>  
>  	  /* Need to handle empty arguments specially.  */
>  	  if (argv[i][0] == '\0')
>  	    {
> -	      *out++ = quote;
> -	      *out++ = quote;
> +	      result += quote;
> +	      result += quote;
>  	    }
>  	  else
>  	    {
>  #ifdef __MINGW32__
> -	      int quoted = 0;
> +	      bool quoted = 0;

Replace 0 with false.

>  
>  	      if (strpbrk (argv[i], special))
>  		{
> -		  quoted = 1;
> -		  *out++ = quote;
> +		  quoted = true;
> +		  result += quote;
>  		}
>  #endif
> -	      for (cp = argv[i]; *cp; ++cp)
> +	      for (char *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;
> +		      result += quote;
> +		      result += '\n';
> +		      result += quote;
>  		    }
>  		  else
>  		    {
> @@ -102,24 +90,22 @@ construct_inferior_arguments (int argc, char **argv)
>  #else
>  		      if (strchr (special, *cp) != NULL)
>  #endif
> -			*out++ = '\\';
> -		      *out++ = *cp;
> +			result += '\\';
> +		      result += *cp;
>  		    }
>  		}
>  #ifdef __MINGW32__
>  	      if (quoted)
> -		*out++ = quote;
> +		result += quote;
>  #endif
>  	    }
>  	}
> -      *out = '\0';
>      }
>    else
>      {
>        /* In this case we can't handle arguments that contain spaces,
>  	 tabs, or newlines -- see breakup_args().  */
>        int i;

Can you also inline this one in the for loops?

Simon