[PATCH] Fix PR50293 - LTO plugin with space in path
Joey Ye
joey.ye@arm.com
Mon Feb 25 01:41:00 GMT 2013
Ping
> -----Original Message-----
> From: Joey Ye [mailto:joey.ye@arm.com]
> Sent: Monday, February 18, 2013 11:32
> To: 'Joseph Myers'
> Cc: gcc-patches@gcc.gnu.org
> Subject: RE: [PATCH] Fix PR50293 - LTO plugin with space in path
>
> Joseph, Thanks for your valuable comments. See my reply and new patch
> below.
>
> > -----Original Message-----
> > From: Joseph Myers [mailto:joseph@codesourcery.com]
> > Sent: Monday, February 18, 2013 06:16
> > To: Joey Ye
> > Cc: gcc-patches@gcc.gnu.org
> > Subject: Re: [PATCH] Fix PR50293 - LTO plugin with space in path
> >
> > On Sun, 17 Feb 2013, Joey Ye wrote:
> >
> > > +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;
> +}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: space_in_plugin_spec-20130218.patch
Type: application/octet-stream
Size: 2982 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20130225/fc36b0cd/attachment.obj>
More information about the Gcc-patches
mailing list