On Wed, 31 Mar 2010 at 5:02:08 +0200, Tamas TEVESZ wrote:
>
> i seem to remember carlos having been proud of being able to compile
> wm with no warnings.
With no _useless_ warnings.
> Subject: [PATCH] Autoconf mods
>
> - according to the automake manual, `acinclude.m4' is the old style
> of doing stuff, putting local macros in their own directory is the
> way to go, so move acincluce.m4 to m4/windowmaker.m4
> - reflect this in autogen.sh and Makefile.am
> - while there, add a `conditionally set cflags' macro from the
> autoconf macro archive
Nice stuff, thanks for doing this!
> - use this to slightly pump warning levels up if we are on gcc
[...]
> +AX_CFLAGS_GCC_OPTION(-Wall)
> +AX_CFLAGS_GCC_OPTION(-Wextra)
Ok, I want to raise a discussion on this, because adding these flags
now make my "new warning detector" break down, which was the idea
behind adding the option --disable-verbose-compile a while back.
Using --disable-verbose-compile my compilation looks like this:
[ma...@pilar:wmaker.git]$ make
Making all in wrlib
Making all in .
CC raster.lo
CC draw.lo
CC color.lo
CC load.lo
CC save.lo
CC gradient.lo
CC xpixmap.lo
CC convert.lo
CC x86_specific.lo
CC context.lo
CC misc.lo
CC scale.lo
CC rotate.lo
CC convolve.lo
CC nxpm.lo
CC xpm.lo
CC xutil.lo
CC ppm.lo
CC png.lo
CC jpeg.lo
CC tiff.lo
CC gif.lo
CC libwraster.la
Making all in WINGs
Making all in WINGs
Making all in .
CC configuration.lo
CC dragcommon.lo
CC dragdestination.lo
CC dragsource.lo
CC selection.lo
CC wappresource.lo
CC wballoon.lo
CC wbox.lo
...
and it becomes obvious when a patch introduces something wrong which comes
with a warning.
For example, I made a wrong patch just now to make my point here. And this is
what I get now:
...
CC tree.lo
CC userdefaults.lo
CC usleep.lo
CC wapplication.lo
CC wutil.lo
Making all in po
Making all in Documentation
Making all in Resources
Making all in src
CC actions.o
actions.c: In function ‘wSetFocusTo’:
actions.c:134: warning: passing argument 1 of ‘wApplicationOf’ makes integer
from pointer without a cast
application.h:56: note: expected ‘Window’ but argument is of type ‘Window *’
CC appicon.o
CC application.o
CC appmenu.o
CC balloon.o
CC client.o
CC colormap.o
...
Now you can immediately be convinced that even a blind man would see this
warning crying out loud that you made something wrong.
But that is true only because the background noise is _zero_, which was the
motivation for --disable-verbose-compile.
However, adding -Wall and -Wextra makes the background noise a lot higher, and
most of the warning are useless:
[ma...@pilar:wmaker.git]$ make
Making all in wrlib
Making all in .
CC raster.lo
raster.c: In function ‘RGetSubImage’:
raster.c:126: warning: comparison between signed and unsigned integer
expressions
raster.c:128: warning: comparison between signed and unsigned integer
expressions
raster.c:142: warning: comparison between signed and unsigned integer
expressions
raster.c: In function ‘calculateCombineArea’:
raster.c:295: warning: unused parameter ‘src’
raster.c: In function ‘RCombineArea’:
raster.c:347: warning: comparison between signed and unsigned integer
expressions
raster.c:359: warning: comparison between signed and unsigned integer
expressions
raster.c:360: warning: comparison between signed and unsigned integer
expressions
raster.c:383: warning: comparison between signed and unsigned integer
expressions
raster.c:384: warning: comparison between signed and unsigned integer
expressions
raster.c: In function ‘RCopyArea’:
raster.c:423: warning: comparison between signed and unsigned integer
expressions
raster.c:435: warning: comparison between signed and unsigned integer
expressions
raster.c:436: warning: comparison between signed and unsigned integer
expressions
raster.c:460: warning: comparison between signed and unsigned integer
expressions
raster.c:466: warning: comparison between signed and unsigned integer
expressions
raster.c:467: warning: comparison between signed and unsigned integer
expressions
raster.c: In function ‘RCombineAreaWithOpaqueness’:
raster.c:506: warning: comparison between signed and unsigned integer
expressions
raster.c:507: warning: comparison between signed and unsigned integer
expressions
raster.c:529: warning: comparison between signed and unsigned integer
expressions
raster.c:530: warning: comparison between signed and unsigned integer
expressions
raster.c: In function ‘RMakeTiledImage’:
raster.c:593: warning: comparison between signed and unsigned integer
expressions
raster.c:593: warning: comparison between signed and unsigned integer
expressions
raster.c:595: warning: comparison between signed and unsigned integer
expressions
raster.c:595: warning: comparison between signed and unsigned integer
expressions
raster.c:605: warning: comparison between signed and unsigned integer
expressions
raster.c:606: warning: comparison between signed and unsigned integer
expressions
raster.c:608: warning: comparison between signed and unsigned integer
expressions
raster.c:608: warning: signed and unsigned type in conditional expression
raster.c: In function ‘RMakeCenteredImage’:
raster.c:638: warning: comparison between signed and unsigned integer
expressions
raster.c:647: warning: comparison between signed and unsigned integer
expressions
CC draw.lo
CC color.lo
CC load.lo
CC save.lo
CC gradient.lo
gradient.c: In function ‘renderHGradient’:
gradient.c:120: warning: comparison between signed and unsigned integer
expressions
gradient.c:130: warning: comparison between signed and unsigned integer
expressions
gradient.c: In function ‘renderVGradient’:
and it goes like this all the way down.
I am not saying that all of the "new" warnings with Tamas' patch are useless
(like _some_ of the "unused variable" kind of warning, which happens to hit
on wmgenmenu.c -- shame on me), but we should find the useful warnings
and check only for those. For example the -Wstrict-prototypes which Linus
pointed out in this discussion about useless warnings
http://kerneltrap.org/node/7434
So I will apply this patch as is now so that people can see the horror
themselves
and perhaps be motivated to find out the useful warnings and fix them. But
later on I will revert the -Wall -Wextra lines and let people who want to
see those add them in CFLAGS during ./configure time.
What do people think? If anyone cares...
--
To unsubscribe, send mail to [email protected].
