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.
> +/* 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.
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
jos...@codesourcery.com
