This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] Verify warning flags for CC_FOR_BUILD compiler
- From: Nick Clifton <nickc at redhat dot com>
- To: Vlad Zakharov <Vladislav dot Zakharov at synopsys dot com>, "binutils at sourceware dot org" <binutils at sourceware dot org>
- Cc: Cupertino Miranda <Cupertino dot Miranda at synopsys dot com>, Alexey Brodkin <Alexey dot Brodkin at synopsys dot com>, Anton Kolesov <Anton dot Kolesov at synopsys dot com>
- Date: Mon, 26 Sep 2016 10:32:25 +0100
- Subject: Re: [PATCH] Verify warning flags for CC_FOR_BUILD compiler
- Authentication-results: sourceware.org; auth=none
- References: <1473261502-17860-1-git-send-email-vzakhar@synopsys.com> <1474620893.7924.9.camel@synopsys.com>
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.
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 <vzakhar@synopsys.com>
* Makefile.am: Replace AM_CLFAGS with AM_CFLAGS_FOR_BUILD
when building with CC_FOR_BUILD compiler.
* Makefile.in: Regenerate.
Cheers
Nick