| Message ID | 1303494642-18594-1-git-send-email-bp@alien8.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 22.4.2011 19:50, Borislav Petkov wrote: > From: Sam Ravnborg <sam@ravnborg.org> > > Building a kernel with "make W=1" produce far too much noise > to be usefull. > > Divide the warning options in three groups: > > W=1 - warnings that may be relevant and does not occur too often > W=2 - warnings that occur quite often but may still be relevant > W=3 - the more obscure warnings, can most likely be ignored > > When building init/ on my box the levels produces: > > W=1 - 46 warnings > W=2 - 863 warnings > W=3 - 6496 warnings I guess these numbers are not valid after your changes? Not that the exact numbers are important, but maybe the distribution change? Otherwise the patch looks good, thanks a lot to both of you! Michal > > Many warnings occur from .h files so fixing one file may have a nice > effect on the total number of warnings. > > With these changes I am actually tempted to try W=1 now and then. > Previously there were just too much noise. > > Borislav: > > - make the W= levels exclusive > - move very noisy and making little sense for the kernel warnings to W=3 > - drop -Woverlength-strings due to useless warning message > - copy explanatory text for the different warning levels to 'make help' > > Signed-off-by: Sam Ravnborg <sam@ravnborg.org> > Signed-off-by: Borislav Petkov <bp@alien8.de> > Cc: Dave Jones <davej@redhat.com> > --- > Makefile | 8 ++++- > scripts/Makefile.build | 65 ++++++++++++++++++++++++++++-------------------- > 2 files changed, 44 insertions(+), 29 deletions(-) > > diff --git a/Makefile b/Makefile > index b967b96..17ce5d5 100644 > --- a/Makefile > +++ b/Makefile > @@ -103,7 +103,7 @@ ifeq ("$(origin O)", "command line") > endif > > ifeq ("$(origin W)", "command line") > - export KBUILD_ENABLE_EXTRA_GCC_CHECKS := 1 > + export KBUILD_ENABLE_EXTRA_GCC_CHECKS := $(W) > endif > > # That's our default target when none is given on the command line > @@ -1267,7 +1267,11 @@ help: > @echo ' make O=dir [targets] Locate all output files in "dir", including .config' > @echo ' make C=1 [targets] Check all c source with $$CHECK (sparse by default)' > @echo ' make C=2 [targets] Force check of all c source with $$CHECK' > - @echo ' make W=1 [targets] Enable extra gcc checks' > + @echo ' make W=n [targets] Enable extra gcc checks, n=1,2,3 where' > + @echo ' 1: warnings which may be relevant and do not occur too often' > + @echo ' 2: warnings which occur quite often but may still be relevant' > + @echo ' 3: more obscure warnings, can most likely be ignored' > + > @echo '' > @echo 'Execute "make" or "make all" to build all targets marked with [*] ' > @echo 'For further info see the ./README file' > diff --git a/scripts/Makefile.build b/scripts/Makefile.build > index d5f925a..ffb383c 100644 > --- a/scripts/Makefile.build > +++ b/scripts/Makefile.build > @@ -51,36 +51,47 @@ ifeq ($(KBUILD_NOPEDANTIC),) > endif > > # > -# make W=1 settings > +# make W=... settings > # > -# $(call cc-option... ) handles gcc -W.. options which > +# W=1 - warnings that may be relevant and does not occur too often > +# W=2 - warnings that occur quite often but may still be relevant > +# W=3 - the more obscure warnings, can most likely be ignored > +# > +# $(call cc-option, -W...) handles gcc -W.. options which > # are not supported by all versions of the compiler > ifdef KBUILD_ENABLE_EXTRA_GCC_CHECKS > -KBUILD_EXTRA_WARNINGS := -Wextra > -KBUILD_EXTRA_WARNINGS += -Wunused -Wno-unused-parameter > -KBUILD_EXTRA_WARNINGS += -Waggregate-return > -KBUILD_EXTRA_WARNINGS += -Wbad-function-cast > -KBUILD_EXTRA_WARNINGS += -Wcast-qual > -KBUILD_EXTRA_WARNINGS += -Wcast-align > -KBUILD_EXTRA_WARNINGS += -Wconversion > -KBUILD_EXTRA_WARNINGS += -Wdisabled-optimization > -KBUILD_EXTRA_WARNINGS += -Wlogical-op > -KBUILD_EXTRA_WARNINGS += -Wmissing-declarations > -KBUILD_EXTRA_WARNINGS += -Wmissing-format-attribute > -KBUILD_EXTRA_WARNINGS += $(call cc-option, -Wmissing-include-dirs,) > -KBUILD_EXTRA_WARNINGS += -Wmissing-prototypes > -KBUILD_EXTRA_WARNINGS += -Wnested-externs > -KBUILD_EXTRA_WARNINGS += -Wold-style-definition > -KBUILD_EXTRA_WARNINGS += $(call cc-option, -Woverlength-strings,) > -KBUILD_EXTRA_WARNINGS += -Wpacked > -KBUILD_EXTRA_WARNINGS += -Wpacked-bitfield-compat > -KBUILD_EXTRA_WARNINGS += -Wpadded > -KBUILD_EXTRA_WARNINGS += -Wpointer-arith > -KBUILD_EXTRA_WARNINGS += -Wredundant-decls > -KBUILD_EXTRA_WARNINGS += -Wshadow > -KBUILD_EXTRA_WARNINGS += -Wswitch-default > -KBUILD_EXTRA_WARNINGS += $(call cc-option, -Wvla,) > -KBUILD_CFLAGS += $(KBUILD_EXTRA_WARNINGS) > +warning-1 := -Wextra -Wunused -Wno-unused-parameter > +warning-1 += -Wmissing-declarations > +warning-1 += -Wmissing-format-attribute > +warning-1 += -Wmissing-prototypes > +warning-1 += -Wold-style-definition > +warning-1 += $(call cc-option, -Wmissing-include-dirs) > + > +warning-2 := -Waggregate-return > +warning-2 += -Wcast-align > +warning-2 += -Wdisabled-optimization > +warning-2 += -Wnested-externs > +warning-2 += -Wshadow > +warning-2 += $(call cc-option, -Wlogical-op) > + > +warning-3 := -Wbad-function-cast > +warning-3 += -Wcast-qual > +warning-3 += -Wconversion > +warning-3 += -Wpacked > +warning-3 += -Wpadded > +warning-3 += -Wpointer-arith > +warning-3 += -Wredundant-decls > +warning-3 += -Wswitch-default > +warning-3 += $(call cc-option, -Wpacked-bitfield-compat) > +warning-3 += $(call cc-option, -Wvla) > + > +warning := $(warning-$(KBUILD_ENABLE_EXTRA_GCC_CHECKS)) > + > +ifeq ("$(warning)","") > + $(error W=$(KBUILD_ENABLE_EXTRA_GCC_CHECKS) is unknown) > +endif > + > +KBUILD_CFLAGS += $(warning) > endif > > include scripts/Makefile.lib -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 26, 2011 at 09:52:35PM +0200, Michal Marek wrote: > On 22.4.2011 19:50, Borislav Petkov wrote: > > From: Sam Ravnborg <sam@ravnborg.org> > > > > Building a kernel with "make W=1" produce far too much noise > > to be usefull. > > > > Divide the warning options in three groups: > > > > W=1 - warnings that may be relevant and does not occur too often > > W=2 - warnings that occur quite often but may still be relevant > > W=3 - the more obscure warnings, can most likely be ignored > > > > When building init/ on my box the levels produces: > > > > W=1 - 46 warnings > > W=2 - 863 warnings > > W=3 - 6496 warnings > > I guess these numbers are not valid after your changes? Not that the > exact numbers are important, but maybe the distribution change? I think so too that those numbers don't mean a lot. Instead, this feature makes more sense IMHO if you use it on a single file: make W=1 <file.c> 2>before.log <make your changes> make W=1 <file.c> 2>after.log diff -uprN before.log after.log and you let the compiler tell you which warnings you've introduced. Then you do the same game with W=2 and W=3. Nice, huh. :)
On Tue, Apr 26, 2011 at 10:43:07PM +0200, Borislav Petkov wrote: > On Tue, Apr 26, 2011 at 09:52:35PM +0200, Michal Marek wrote: > > On 22.4.2011 19:50, Borislav Petkov wrote: > > > From: Sam Ravnborg <sam@ravnborg.org> > > > > > > Building a kernel with "make W=1" produce far too much noise > > > to be usefull. > > > > > > Divide the warning options in three groups: > > > > > > W=1 - warnings that may be relevant and does not occur too often > > > W=2 - warnings that occur quite often but may still be relevant > > > W=3 - the more obscure warnings, can most likely be ignored > > > > > > When building init/ on my box the levels produces: > > > > > > W=1 - 46 warnings > > > W=2 - 863 warnings > > > W=3 - 6496 warnings > > > > I guess these numbers are not valid after your changes? Not that the > > exact numbers are important, but maybe the distribution change? > > I think so too that those numbers don't mean a lot. The numbers _only_ tell you that the amoun of warnings on W=1 is down to a manageable number and they increase be the higher level. Will you cook up a more correct changelog and resubmit? Sam -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Apr 22, 2011 at 07:50:42PM +0200, Borislav Petkov wrote: > From: Sam Ravnborg <sam@ravnborg.org> > > Building a kernel with "make W=1" produce far too much noise > to be usefull. > > Divide the warning options in three groups: > > W=1 - warnings that may be relevant and does not occur too often > W=2 - warnings that occur quite often but may still be relevant > W=3 - the more obscure warnings, can most likely be ignored > > When building init/ on my box the levels produces: > > W=1 - 46 warnings > W=2 - 863 warnings > W=3 - 6496 warnings > > Many warnings occur from .h files so fixing one file may have a nice > effect on the total number of warnings. > > With these changes I am actually tempted to try W=1 now and then. > Previously there were just too much noise. > > Borislav: > > - make the W= levels exclusive I do not see the point in this really. This is not what most people would expect. When you ask for more you get more - not something else. We see it with verbose levels where -vv give more output than -v etc. Anyway - the important thing is to keep the relevant warnings at W=1 level. Which is independendt of this change. So consider the input and decide - I do not want to make a fuzz about it. Sam -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Apr 27, 2011 at 10:25:55AM +0200, Sam Ravnborg wrote: > > - make the W= levels exclusive > I do not see the point in this really. This is not what most people would > expect. > When you ask for more you get more - not something else. > > We see it with verbose levels where -vv give more output than -v etc. > > Anyway - the important thing is to keep the relevant warnings at W=1 level. > Which is independendt of this change. > So consider the input and decide - I do not want to make a fuzz about it. I know, -vv.. increases verbosity is probably part of old unix tradition or common sense but it doesn't make much sense in this case, IMHO. When I use this, I want to see what the most relevant warnings are, maybe have a crack at them to fix them, and _then_ look at the less important ones (for an arbitrary definition of important warnings). If we do this inclusive, then W=2 dumps the, let's call it, level 1 _plus_ the new level 2 warnings, polluting the output with something I've already seen, but only partially. And then I start to think, did I see this one already, didn't I, which was it? By the time you enable W=3, the output becomes pretty useless. For example, W=3 generates 190+ MB logfile here only with level 3 warnings. Now imagine all 3 levels combined. Dividing the output by level of importance doesn't have this problem and is much more workable, IMHO. But this is just my use case, it could be that I'm completely alone on this one. I'd love to hear what other people think. FWIW, we might even make this behavior configurable by having make W=1o meaning level 1 warnings only or whatever sick idea we come up with eventually. ;-) Thanks.
On Wed, 27 Apr 2011 13:35:13 +0200, Borislav Petkov said: > If we do this inclusive, then W=2 dumps the, let's call it, level 1 > _plus_ the new level 2 warnings, polluting the output with something > I've already seen, but only partially. And then I start to think, did > I see this one already, didn't I, which was it? By the time you enable > W=3, the output becomes pretty useless. For example, W=3 generates 190+ > MB logfile here only with level 3 warnings. Now imagine all 3 levels > combined. If each level is averaging 10x the previous level, then all 3 levels will only be 11% bigger, or 211MB. You *really* want to get *all* the warnings - quite often, you'll be looking at a set of 15 or 20 level-3 warnings. And if you had the Level-2's in there as well, you'd immediately realize that the single level-2 was the real root-cause of all the cascating warnings. Also, having it as a "volume control" means I don't have to go reading the Makefile to figure out which number I need to specify for which message - I can just say W=3 and *know* the flavor I want will be in there somewhere. Let's face it, if there's over 10 or 15 warnings involved, grep or your editor's "locate next" will do a better job of finding it than you ever can...
diff --git a/Makefile b/Makefile index b967b96..17ce5d5 100644 --- a/Makefile +++ b/Makefile @@ -103,7 +103,7 @@ ifeq ("$(origin O)", "command line") endif ifeq ("$(origin W)", "command line") - export KBUILD_ENABLE_EXTRA_GCC_CHECKS := 1 + export KBUILD_ENABLE_EXTRA_GCC_CHECKS := $(W) endif # That's our default target when none is given on the command line @@ -1267,7 +1267,11 @@ help: @echo ' make O=dir [targets] Locate all output files in "dir", including .config' @echo ' make C=1 [targets] Check all c source with $$CHECK (sparse by default)' @echo ' make C=2 [targets] Force check of all c source with $$CHECK' - @echo ' make W=1 [targets] Enable extra gcc checks' + @echo ' make W=n [targets] Enable extra gcc checks, n=1,2,3 where' + @echo ' 1: warnings which may be relevant and do not occur too often' + @echo ' 2: warnings which occur quite often but may still be relevant' + @echo ' 3: more obscure warnings, can most likely be ignored' + @echo '' @echo 'Execute "make" or "make all" to build all targets marked with [*] ' @echo 'For further info see the ./README file' diff --git a/scripts/Makefile.build b/scripts/Makefile.build index d5f925a..ffb383c 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -51,36 +51,47 @@ ifeq ($(KBUILD_NOPEDANTIC),) endif # -# make W=1 settings +# make W=... settings # -# $(call cc-option... ) handles gcc -W.. options which +# W=1 - warnings that may be relevant and does not occur too often +# W=2 - warnings that occur quite often but may still be relevant +# W=3 - the more obscure warnings, can most likely be ignored +# +# $(call cc-option, -W...) handles gcc -W.. options which # are not supported by all versions of the compiler ifdef KBUILD_ENABLE_EXTRA_GCC_CHECKS -KBUILD_EXTRA_WARNINGS := -Wextra -KBUILD_EXTRA_WARNINGS += -Wunused -Wno-unused-parameter -KBUILD_EXTRA_WARNINGS += -Waggregate-return -KBUILD_EXTRA_WARNINGS += -Wbad-function-cast -KBUILD_EXTRA_WARNINGS += -Wcast-qual -KBUILD_EXTRA_WARNINGS += -Wcast-align -KBUILD_EXTRA_WARNINGS += -Wconversion -KBUILD_EXTRA_WARNINGS += -Wdisabled-optimization -KBUILD_EXTRA_WARNINGS += -Wlogical-op -KBUILD_EXTRA_WARNINGS += -Wmissing-declarations -KBUILD_EXTRA_WARNINGS += -Wmissing-format-attribute -KBUILD_EXTRA_WARNINGS += $(call cc-option, -Wmissing-include-dirs,) -KBUILD_EXTRA_WARNINGS += -Wmissing-prototypes -KBUILD_EXTRA_WARNINGS += -Wnested-externs -KBUILD_EXTRA_WARNINGS += -Wold-style-definition -KBUILD_EXTRA_WARNINGS += $(call cc-option, -Woverlength-strings,) -KBUILD_EXTRA_WARNINGS += -Wpacked -KBUILD_EXTRA_WARNINGS += -Wpacked-bitfield-compat -KBUILD_EXTRA_WARNINGS += -Wpadded -KBUILD_EXTRA_WARNINGS += -Wpointer-arith -KBUILD_EXTRA_WARNINGS += -Wredundant-decls -KBUILD_EXTRA_WARNINGS += -Wshadow -KBUILD_EXTRA_WARNINGS += -Wswitch-default -KBUILD_EXTRA_WARNINGS += $(call cc-option, -Wvla,) -KBUILD_CFLAGS += $(KBUILD_EXTRA_WARNINGS) +warning-1 := -Wextra -Wunused -Wno-unused-parameter +warning-1 += -Wmissing-declarations +warning-1 += -Wmissing-format-attribute +warning-1 += -Wmissing-prototypes +warning-1 += -Wold-style-definition +warning-1 += $(call cc-option, -Wmissing-include-dirs) + +warning-2 := -Waggregate-return +warning-2 += -Wcast-align +warning-2 += -Wdisabled-optimization +warning-2 += -Wnested-externs +warning-2 += -Wshadow +warning-2 += $(call cc-option, -Wlogical-op) + +warning-3 := -Wbad-function-cast +warning-3 += -Wcast-qual +warning-3 += -Wconversion +warning-3 += -Wpacked +warning-3 += -Wpadded +warning-3 += -Wpointer-arith +warning-3 += -Wredundant-decls +warning-3 += -Wswitch-default +warning-3 += $(call cc-option, -Wpacked-bitfield-compat) +warning-3 += $(call cc-option, -Wvla) + +warning := $(warning-$(KBUILD_ENABLE_EXTRA_GCC_CHECKS)) + +ifeq ("$(warning)","") + $(error W=$(KBUILD_ENABLE_EXTRA_GCC_CHECKS) is unknown) +endif + +KBUILD_CFLAGS += $(warning) endif include scripts/Makefile.lib