From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1286 invoked by alias); 17 Feb 2013 22:16:36 -0000 Received: (qmail 1274 invoked by uid 22791); 17 Feb 2013 22:16:35 -0000 X-SWARE-Spam-Status: No, hits=-4.8 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 17 Feb 2013 22:16:31 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1U7CX7-0003cV-I8 from joseph_myers@mentor.com ; Sun, 17 Feb 2013 14:16:29 -0800 Received: from SVR-IES-FEM-01.mgc.mentorg.com ([137.202.0.104]) by svr-orw-fem-01.mgc.mentorg.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Sun, 17 Feb 2013 14:16:29 -0800 Received: from digraph.polyomino.org.uk (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.1.289.1; Sun, 17 Feb 2013 22:16:27 +0000 Received: from jsm28 (helo=localhost) by digraph.polyomino.org.uk with local-esmtp (Exim 4.76) (envelope-from ) id 1U7CX4-0007FH-3L; Sun, 17 Feb 2013 22:16:26 +0000 Date: Sun, 17 Feb 2013 22:16:00 -0000 From: "Joseph S. Myers" To: Joey Ye CC: Subject: Re: [PATCH] Fix PR50293 - LTO plugin with space in path In-Reply-To: <000001ce0cb4$6b491000$41db3000$@ye@arm.com> Message-ID: References: <000001ce0cb4$6b491000$41db3000$@ye@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2013-02/txt/msg00829.txt.bz2 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 joseph@codesourcery.com