Re: [PATCH] Fix PR50293 - LTO plugin with space in path

[Georg-Johann Lay <gjl@gcc.gnu.org>]

Joey Ye schrieb:
> Ping
> 
>> Subject: RE: [PATCH] Fix PR50293 - LTO plugin with space in path

Does this patch also work with MS-Windows as host, i.e. with \ as path 
separator?

>>>> +static char * convert_white_space(char * orig);
>>> Please fix formatting in many places in this patch to follow the GNU
>>> Coding Standards.  No space after '*', but space before '('; there
>> seem
>>> to
>>> be various other formatting problems as well.
>> My bad. All fixed.
>>
>>>> +/* Insert back slash before spaces in a string, to avoid path
>>>> +   that has space in it broken into multiple arguments.  */
>>> That doesn't seem to be a proper specification of the interface to
>> this
>>> function.  What are the semantics of ORIG?  A string that is a
>> filename,
>>> or something else?  What are the exact semantics of the return value
>> for
>>> quoting - is it correct for the function to convert a (backslash,
>> space)
>>> pair to (backslash, backslash, space) or not?  Is anything special in
>>> the
>>> return value other than backslash and space, and how are any special
>>> characters in the return value to be interpreted?
>>>
>>> As it seems like this function frees the argument (why?) this also
>> needs
>>> to be specified in the comment as part of the semantics of the
>> function.
>> This function might need a string longer than original one to
>> accommodate
>> additional back slashes. So it has to xmalloc a new string. The original
>> string should be freed in such a case. However, it is tedious to caller
>> to figure out that conversion does happens and free the orig. The
>> solution
>> is for this function to free it when conversion happens.
>>
>> By doing so it is required that orig must be allocated and can be freed,
>> as the newly added comments described explicitly.
>>> It would be a good idea for you to give a more detailed explanation in
>>> the
>>> next version of the patch submission of how the path, before the patch,
>>> got processed so that the spaces were wrongly interpreted.  That might
>>> help make clearer whether the interface to this new function is
>> actually
>>> correct, since the subsequent operations on the return value should
>> act
>>> as
>>> an inverse to the operation carried out by this function.
>>>
>>> --
>>> Joseph S. Myers
>>> joseph@codesourcery.com
>> Index: gcc/gcc.c
>> ===================================================================
>> --- gcc/gcc.c	(revision 195189)
>> +++ gcc/gcc.c	(working copy)
>> @@ -265,6 +265,7 @@
>>  static const char *compare_debug_auxbase_opt_spec_function (int, const
>> char **);
>>  static const char *pass_through_libs_spec_func (int, const char **);
>>  static const char *replace_extension_spec_func (int, const char **);
>> +static char * convert_white_space (char *);
>>
>>
>>  /* The Specs Language
>>
>> @@ -6595,6 +6596,7 @@
>>  				    X_OK, false);
>>    if (lto_wrapper_file)
>>      {
>> +      lto_wrapper_file = convert_white_space (lto_wrapper_file);
>>        lto_wrapper_spec = lto_wrapper_file;
>>        obstack_init (&collect_obstack);
>>        obstack_grow (&collect_obstack, "COLLECT_LTO_WRAPPER=",
>> @@ -7005,12 +7007,13 @@
>>  			      + strlen (fuse_linker_plugin), 0))
>>  #endif
>>  	    {
>> -	      linker_plugin_file_spec = find_a_file (&exec_prefixes,
>> +	      char * temp_spec = find_a_file (&exec_prefixes,
>>  						     LTOPLUGINSONAME, R_OK,
>>  						     false);
>> -	      if (!linker_plugin_file_spec)
>> +	      if (!temp_spec)
>>  		fatal_error ("-fuse-linker-plugin, but %s not found",
>>  			     LTOPLUGINSONAME);
>> +	      linker_plugin_file_spec = convert_white_space (temp_spec);
>>  	    }
>>  #endif
>>  	  lto_gcc_spec = argv[0];
>> @@ -8506,3 +8509,52 @@
>>    free (name);
>>    return result;
>>  }
>> +
>> +/* Insert back slash before spaces in orig (usually a file path), to
>> +   avoid being broken by spec parser.
>> +
>> +   This function is needed as do_spec_1 treats white space (' ' and
>> '\t')
>> +   as the end of an argument. But in case of -plugin /usr/gcc
>> install/xxx.so,
>> +   the filename should be treated as a single argument rather than
>> being
>> +   broken into multiple. Solution is to insert '\\' before the space in
>> a
>> +   filename.
>> +
>> +   This function converts and only converts all occurrance of ' '
>> +   to '\\' + ' ' and '\t' to '\\' + '\t'.  For example:
>> +   "a b"  -> "a\\ b"
>> +   "a  b" -> "a\\ \\ b"
>> +   "a\tb" -> "a\\\tb"
>> +   "a\\ b" -> "a\\\\ b"
>> +
>> +   orig: input null-terminating string that was allocated by xalloc.
>> The
>> +   memory it points to might be freed in this function. Behavior
>> undefined
>> +   if orig isn't xalloced or is freed already at entry.
>> +
>> +   Return: orig if no conversion needed. orig if conversion needed but
>> no
>> +   sufficient memory for a new string. Otherwise a newly allocated
>> string
>> +   that was converted from orig.  */
>> +
>> +static char * convert_white_space (char *orig)
>> +{
>> +  int len, number_of_space = 0;
>> +  if (orig == NULL) return orig;
>> +
>> +  for (len=0; orig[len]; len++)
>> +    if (orig[len] == ' ' || orig[len] == '\t') number_of_space ++;
>> +
>> +  if (number_of_space)
>> +    {
>> +      char * new_spec = (char *)xmalloc (len + number_of_space + 1);
>> +      int j,k;
>> +      if (new_spec == NULL) return orig;
>> +
>> +      for (j=0, k=0; j<=len; j++, k++)
>> +	{
>> +	  if (orig[j] == ' ' || orig[j] == '\t') new_spec[k++] = '\\';
>> +	  new_spec[k] = orig[j];
>> +	}
>> +      free (orig);
>> +      return new_spec;
>> +  }
>> +  else return orig;
>> +}