This is the mail archive of the mailing list for the binutils project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Verify warning flags for CC_FOR_BUILD compiler

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,
as you have done, but you should mention in the changelog entry that the file has
been regenerated.  Ie:

2016-09-07  Vlad Zakharov  <>

	when building with CC_FOR_BUILD compiler.
	* Regenerate.



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]