This is the mail archive of the
mailing list for the binutils project.
Re: [PATCH] Verify warning flags for CC_FOR_BUILD compiler
- From: Vlad Zakharov <Vladislav dot Zakharov at synopsys dot com>
- To: "binutils at sourceware dot org" <binutils at sourceware dot org>, "nickc at redhat dot com" <nickc at redhat dot com>
- Cc: "Anton dot Kolesov at synopsys dot com" <Anton dot Kolesov at synopsys dot com>, "Alexey dot Brodkin at synopsys dot com" <Alexey dot Brodkin at synopsys dot com>, "Cupertino dot Miranda at synopsys dot com" <Cupertino dot Miranda at synopsys dot com>
- Date: Mon, 26 Sep 2016 10:43:55 +0000
- Subject: Re: [PATCH] Verify warning flags for CC_FOR_BUILD compiler
- Authentication-results: sourceware.org; auth=none
- References: <firstname.lastname@example.org> <email@example.com> <firstname.lastname@example.org>
On Mon, 2016-09-26 at 10:32 +0100, Nick Clifton wrote:
> Hi Vlad,
> > Please treat this message as a polite reminder to review the patch.
> Oops- sorry for dropping the ball on this one.
> I have a couple of questions about the patch:
> > >
> > > # Add -Wshadow if the compiler is a sufficiently recent version of GCC.
> > > -AC_EGREP_CPP([^[0-3]$],[__GNUC__],,GCC_WARN_CFLAGS="$GCC_WARN_CFLAGS -Wshadow")
> > > +AC_EGREP_CPP([^[0-3]$],[__GNUC__],,GCC_asdfasdfWARN_CFLAGS="$GCC_WARN_CFLAGS -Wshadow")
> What is this for ? Why is the change being made and what is the significance of 'asdfasdf' ?
> Other than that though the patch looks good to me. I am withholding approval
> pending an answer to the above question, (which I suspect might turn out to be
> a typo), but I think that we are almost there.
Oh, sorry for that, it is really a typo.
Of course there shouldn't be any updates at that line. It's just my silly
> One other small point - it is easier for us if you include the proposed ChangeLog
> entries for your patch as plain text rather then context diffs. This is because
> the diff almost never applies directly to the sources, since the changelogs change
> so frequently.
> Oh, and it is good practice to omit diffs for auto-generated files like Makefile.in,
> as you have done, but you should mention in the changelog entry that the file has
> been regenerated. Ie:
> 2016-09-07 Vlad Zakharov <email@example.com>
> * Makefile.am: Replace AM_CLFAGS with AM_CFLAGS_FOR_BUILD
> when building with CC_FOR_BUILD compiler.
> * Makefile.in: Regenerate.
I have taken note of your comments about ChangeLogs and auto-generated files.
Should I resend my patch with these minor
Vlad Zakharov <firstname.lastname@example.org>