From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4103 invoked by alias); 16 Dec 2010 13:45:35 -0000 Received: (qmail 4082 invoked by uid 22791); 16 Dec 2010 13:45:33 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM X-Spam-Check-By: sourceware.org Received: from mail-iy0-f175.google.com (HELO mail-iy0-f175.google.com) (209.85.210.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 16 Dec 2010 13:45:28 +0000 Received: by iyj18 with SMTP id 18so1608368iyj.20 for ; Thu, 16 Dec 2010 05:45:26 -0800 (PST) MIME-Version: 1.0 Received: by 10.231.35.71 with SMTP id o7mr6061149ibd.156.1292507126061; Thu, 16 Dec 2010 05:45:26 -0800 (PST) Received: by 10.231.15.2 with HTTP; Thu, 16 Dec 2010 05:45:26 -0800 (PST) In-Reply-To: <20101216133149.GA20612@basil.fritz.box> References: <1292503308-11258-1-git-send-email-andi@firstfloor.org> <1292503308-11258-2-git-send-email-andi@firstfloor.org> <20101216133149.GA20612@basil.fritz.box> Date: Thu, 16 Dec 2010 15:46:00 -0000 Message-ID: Subject: Re: [PATCH 2/2] Improve reporting of section conflict errors From: Richard Guenther To: Andi Kleen Cc: gcc-patches@gcc.gnu.org, Andi Kleen Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes 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: 2010-12/txt/msg01309.txt.bz2 On Thu, Dec 16, 2010 at 2:31 PM, Andi Kleen wrote: > On Thu, Dec 16, 2010 at 02:25:05PM +0100, Richard Guenther wrote: >> > + =A0struct flag *fl; >> > + =A0char *start =3D buf; >> > + >> > + =A0f &=3D ~SECTION_OVERRIDE; >> >> Any reason to do this here and not in the caller? > > I can move it to the caller. Thanks. >> >> Can you split out a helper function that does return a reference to >> the name for a single section flag? =A0I suppose it might be useful >> on its own. > > What do you mean with single? If you pass only a single flag you should > already know what it is? I think this is general enough for most > debugging purposes. Single as in expose the local array you have via a helper function. But well, if you think that's not useful then leave it to whoever would have a use for it. >> > @@ -290,15 +343,23 @@ get_section (const char *name, unsigned int flag= s, tree decl) >> > =A0 =A0 } >> > =A0 else >> > =A0 =A0 { >> > + =A0 =A0 =A0unsigned oflags; >> > + >> > =A0 =A0 =A0 sect =3D *slot; >> > - =A0 =A0 =A0if ((sect->common.flags & ~SECTION_DECLARED) !=3D flags >> > + =A0 =A0 =A0oflags =3D sect->common.flags & ~SECTION_DECLARED; >> > + =A0 =A0 =A0if (oflags !=3D flags >> > =A0 =A0 =A0 =A0 =A0&& ((sect->common.flags | flags) & SECTION_OVERRIDE= ) =3D=3D 0) >> > =A0 =A0 =A0 =A0{ >> > + =A0 =A0 =A0 =A0 char buf[1024]; >> >> Ick ... but well. =A0I suppose you could use asprintf in section_print_f= lags >> (which is oddly named, better would be section_flags_string). =A0The fun= ction >> shouldn't be timing critical. > > I can rename it. Thanks. Please also make the array const as suggested. >> >> I agree the patch is useful. > > Is that an approval? I'd like to see an updated patch with the fixed size array removed. Richard. > -Andi > > -- > ak@linux.intel.com -- Speaking for myself only. >