Date: Wed, 26 Oct 2011 23:31:28 +0000 From: Alexander Best <arundel@freebsd.org> To: Bruce Evans <brde@optusnet.com.au> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, Robert Millan <rmh@FreeBSD.org> Subject: Re: svn commit: r226665 - head/sys/conf Message-ID: <20111026233128.GA8686@freebsd.org> In-Reply-To: <20111027071802.T3703@besplex.bde.org> References: <201110231627.p9NGR47P046269@svn.freebsd.org> <20111026182854.GA73322@freebsd.org> <20111027071802.T3703@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20111026233128.GA8686>