svn commit: r226665 - head/sys/conf

[Alexander Best]

On Thu Oct 27 11, Bruce Evans wrote:
> On Wed, 26 Oct 2011, Alexander Best wrote:
> 
> >On Sun Oct 23 11, Robert Millan wrote:
> >>Log:
> >>  Conditionalize a pair of FreeBSD GCC extensions so that its CFLAGS are 
> >>  only
> >>  used with FreeBSD GCC.
> >
> >any reason -f* flags were added to CWARNFLAGS in the first place? they 
> >aren't
> >really warnings, are they, so adding them to CFLAGS makes more sense imo.
> 
> -fformat-extensions is added to CWARNFLAGS because it is a modifier
> to -Wformat.  It was originally (in bsd.kern.mk 1.10) placed immediately
> after -Wformat in CWARNFLAGS, and was added in the same commit as
> -Wformat (since -Wformat could not be added before -fformat-extensions
> was developed to allow it to be added without breaking kernel builds).
> This is partially explained in the commit message.  This was obfuscated
> (in bsd.kern.mk 1.14) by removing -Wformat.  1.13 had made -Wformat
> redundant by adding -Wall.  This is not explained in the commit message.
> 
> >diff --git a/sys/conf/kern.mk b/sys/conf/kern.mk
> >index a0a595f..91560e0 100644
> >--- a/sys/conf/kern.mk
> >+++ b/sys/conf/kern.mk
> >@@ -3,10 +3,9 @@
> >#
> ># Warning flags for compiling the kernel and components of the kernel:
> >#
> >-CWARNFLAGS?=   -Wall -Wredundant-decls -Wnested-externs 
> >-Wstrict-prototypes \
> >-               -Wmissing-prototypes -Wpointer-arith -Winline -Wcast-qual \
> >-               -Wundef -Wno-pointer-sign -fformat-extensions \
> >-               -Wmissing-include-dirs -fdiagnostics-show-option
> >+CWARNFLAGS?=   -Wall -Wredundant-decls -Wnested-externs 
> >-Wstrict-prototypes\
> >+               -Wmissing-prototypes -Wpointer-arith -Winline -Wcast-qual\
> >+               -Wundef -Wno-pointer-sign -Wmissing-include-dirs
> 
> Neither of these should be moved, except within CWARNFLAGS.
> 
> For -fformat-extensions, see above.
> 
> -fdiagnostics-show option is even more closely related with warning flags
> and even better documented in its commit message, as you should know since
> you submitted it :-).  Actually, the commit message seems to tie it
> more closely with warning flags than it actually is -- it says that
> "it will report which -W* flag was responsible for triggering a certain
> warning", while gcc(1) says that it is for reporting of general diagnostics.
> 
> -Wformat and -fformat-extensions were originally at the end of CWARNFLAGS.
> -fformat-extensions wasn't moved to be with -Wall when that was added.
> Keeping it at the end is reasonable.  But it shouldn't be in the middle
> of random -W flags like it now is.  All of the -W options are unfortunately
> unsorted by adding them to the end of the unsorted list.  Some old versions
> even added -std=c99 to the end.  Since -std has nothing to do with warning
> flags, that was nonsense and has been fixed.
> 
> -fdiagnostics-show-option controls all the warning flags, so it would be
> better placed at the beginning.

thanks for the detailed explanation. how does the following patch look like,
which basically sorts the CWARNINGSFLAGS alphatically and semantically:

diff --git a/sys/conf/kern.mk b/sys/conf/kern.mk
index a0a595f..b45be2a 100644
--- a/sys/conf/kern.mk
+++ b/sys/conf/kern.mk
@@ -1,12 +1,15 @@
 # $FreeBSD$
 
 #
-# Warning flags for compiling the kernel and components of the kernel:
+# Warning flags for compiling the kernel and components of the kernel.
+# Additionally enable FreeBSD kernel-specific printf format specifiers
+# and instruct gcc to enable some diagnostics, which make it easier to
+# pinpoint tinderbox failures.
 #
-CWARNFLAGS?=   -Wall -Wredundant-decls -Wnested-externs -Wstrict-prototypes \
-               -Wmissing-prototypes -Wpointer-arith -Winline -Wcast-qual \
-               -Wundef -Wno-pointer-sign -fformat-extensions \
-               -Wmissing-include-dirs -fdiagnostics-show-option
+CWARNFLAGS?=   -fdiagnostics-show-option -Wall -Wcast-qual -Winline\
+               -Wmissing-include-dirs -Wmissing-prototypes -Wnested-externs\
+               -Wpointer-arith -Wredundant-decls -Wstrict-prototypes -Wundef\
+               -Wno-pointer-sign -fformat-extensions
 #
 # The following flags are next up for working on:
 #      -Wextra
@@ -83,7 +86,7 @@ CFLAGS+=      -mno-sse
 .else
 CFLAGS+=       -mno-aes -mno-avx
 .endif
-CFLAGS+=       -mcmodel=kernel -mno-red-zone -mno-mmx -msoft-float \
+CFLAGS+=       -mcmodel=kernel -mno-red-zone -mno-mmx -msoft-float\
                -fno-asynchronous-unwind-tables
 INLINE_LIMIT?= 8000
 .endif

a flag i am unsure about is "-Wno-pointer-sign". one could argue that it should
come right after "-Wpointer-arith", since we use it to back out a certain
warning that is being implied by it. on the other hand one could argue that
it's more intuitive to first enable *all* warnings via -W* flags and then
afterwards set the -Wno* flags. although currently there's only a single -Wno*
flag in CWARNFLAGS, there might be more in the future. that way we would have
all those -Wno* flags in one block.

cheers.
alex

> 
> Bruce