From patchwork Mon Feb 18 03:31:52 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joey Ye X-Patchwork-Id: 221139 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id E34B62C008C for ; Mon, 18 Feb 2013 14:32:39 +1100 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1361763160; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: From:To:Cc:References:In-Reply-To:Subject:Date:Message-ID: MIME-Version:Content-Type:Content-Transfer-Encoding:Mailing-List: Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:Sender:Delivered-To; bh=e4jZx1+NVDHkrz07fFP2ILYS2tM=; b=DLfYmSwNjrFGD7bkFXSa41UXCWHuw+NGoiazUB7hBB4D3Gib4Cq/NuEHyL8W8k AuCNKaB0fFHiMuLzu33/GVlZTHbC7KfmYblZxXAgV2b58r1fuSmj0XA0/wM/L9zr OR3aMR57a6P0QUzCUDDzV5MGCAlEZhHDF+4zVzZQhUr0Q= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:From:To:Cc:References:In-Reply-To:Subject:Date:Message-ID:MIME-Version:X-MC-Unique:Content-Type:Content-Transfer-Encoding:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=B/xXCbBO1u3CWEIolBrL/WxHCuWYW6U3Yn93US+dmGB/xJ6XkwylkcgyOXDtxj 8mkUkG0sXdbFGpK15+bk+i738JBbXUycqOvw3h09GkU3X8tlx3wcqNRQpzZx4Mw9 KjI/cytB1aO9rtyTujjwErY26GeotpOISqjT7ZqOuanFc=; Received: (qmail 26188 invoked by alias); 18 Feb 2013 03:32:31 -0000 Received: (qmail 26168 invoked by uid 22791); 18 Feb 2013 03:32:29 -0000 X-SWARE-Spam-Status: No, hits=-0.5 required=5.0 tests=AWL, BAYES_50, KHOP_RCVD_UNTRUST, KHOP_SPAMHAUS_DROP, KHOP_THREADED, MSGID_MULTIPLE_AT, RCVD_IN_DNSWL_LOW X-Spam-Check-By: sourceware.org Received: from service87.mimecast.com (HELO service87.mimecast.com) (91.220.42.44) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 18 Feb 2013 03:32:22 +0000 Received: from cam-owa1.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.21]) by service87.mimecast.com; Mon, 18 Feb 2013 03:32:19 +0000 Received: from E103005 ([10.1.255.212]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.0); Mon, 18 Feb 2013 03:32:18 +0000 From: "Joey Ye" To: "'Joseph Myers'" Cc: References: <000001ce0cb4$6b491000$41db3000$@ye@arm.com> In-Reply-To: Subject: RE: [PATCH] Fix PR50293 - LTO plugin with space in path Date: Mon, 18 Feb 2013 11:31:52 +0800 Message-ID: <000101ce0d88$7febdb80$7fc39280$@ye@arm.com> MIME-Version: 1.0 X-MC-Unique: 113021803321903401 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org 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 /* 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; +} 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 *);