From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20971 invoked by alias); 5 Sep 2016 08:00:31 -0000 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 Received: (qmail 20925 invoked by uid 89); 5 Sep 2016 08:00:25 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.0 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx2.suse.de Received: from mx2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 05 Sep 2016 08:00:15 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 0895DAB1A; Mon, 5 Sep 2016 08:00:04 +0000 (UTC) Date: Mon, 05 Sep 2016 09:56:00 -0000 From: Richard Biener To: Jakub Jelinek cc: Kirill Yukhin , Jeff Law , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] Fix alter_output_for_subst_insn (PR other/77421) In-Reply-To: <20160902151933.GH14857@tucnak.redhat.com> Message-ID: References: <20160902151933.GH14857@tucnak.redhat.com> User-Agent: Alpine 2.11 (LSU 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-SW-Source: 2016-09/txt/msg00189.txt.bz2 On Fri, 2 Sep 2016, Jakub Jelinek wrote: > Hi! > > The two ports that use define_subst (ix86 and visium) don't do anything in > this function, other than returning early - return insn_out, so all I could > do is look at the intent. > The *insn_out == '*' || *insn_out != '@' got flagged by some tool, the > "*insn_out == '*' || " part is unnecessary, since '*' != '@'. I guess the > reason for it being there is that template starting with * is C code (which > this function has nothing to do for), template starting with @ is what the > function wants to do something about and other template strings it wants to > ignore too. > > But, when I got to this function, I found various weirdo formatting and > other issues, the most important is that it leaks memory and in my > understanding allocates that buffer completely uselessly, as it is pretty > much a strdup of the insn_out after skipping initial spaces (and all @, > which is weird, IMNSHO the only @ that it should skip is the very first > one), except that '\0' isn't there and the length is remembered. But, we > don't change it at all, so we can as well use the original insn_out. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Ok. Thanks, Richard. > 2016-09-02 Jakub Jelinek > > PR other/77421 > * gensupport.c (alter_output_for_subst_insn): Remove redundant > *insn_out == '*' test. Don't copy unnecessary to yet another > memory buffer, and don't leak it. > > --- gcc/gensupport.c.jj 2016-08-12 17:33:38.000000000 +0200 > +++ gcc/gensupport.c 2016-09-02 14:55:01.217182312 +0200 > @@ -1632,33 +1632,30 @@ duplicate_each_alternative (const char * > static const char * > alter_output_for_subst_insn (rtx insn, int alt) > { > - const char *insn_out, *sp ; > - char *old_out, *new_out, *cp; > - int i, j, new_len; > + const char *insn_out, *old_out; > + char *new_out, *cp; > + size_t old_len, new_len; > + int j; > > insn_out = XTMPL (insn, 3); > > - if (alt < 2 || *insn_out == '*' || *insn_out != '@') > + if (alt < 2 || *insn_out != '@') > return insn_out; > > - old_out = XNEWVEC (char, strlen (insn_out)), > - sp = insn_out; > + old_out = insn_out + 1; > + while (ISSPACE (*old_out)) > + old_out++; > + old_len = strlen (old_out); > > - while (ISSPACE (*sp) || *sp == '@') > - sp++; > - > - for (i = 0; *sp;) > - old_out[i++] = *sp++; > - > - new_len = alt * (i + 1) + 1; > + new_len = alt * (old_len + 1) + 1; > > new_out = XNEWVEC (char, new_len); > new_out[0] = '@'; > > - for (j = 0, cp = new_out + 1; j < alt; j++, cp += i + 1) > + for (j = 0, cp = new_out + 1; j < alt; j++, cp += old_len + 1) > { > - memcpy (cp, old_out, i); > - *(cp+i) = (j == alt - 1) ? '\0' : '\n'; > + memcpy (cp, old_out, old_len); > + cp[old_len] = (j == alt - 1) ? '\0' : '\n'; > } > > return new_out; > > Jakub > > -- Richard Biener SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)