Opened 19 years ago

Closed 19 years ago

Last modified 19 years ago

#1179 closed defect (fixed)

[PATCH] Enable more warning flags when using GCC

Reported by: pere@… Owned by: warmerdam
Priority: high Milestone:
Component: Build Problems Version: 4.4
Severity: normal Keywords:
Cc:

Description

Several mistakes in the source might go unnoticed because the compiler
fail to report them.  Adding more warning flags will increase the
chance of detecting such problems, and I suggest doing so.

The attached patch increases the warning level and make sure the
C code is compiled using CFLAGS and C++ code is compiled using CXXFLAGS.

Attachments (1)

warnings.diff (3.2 KB ) - added by pere@… 19 years ago.
Enable more warnings from GCC

Download all attachments as: .zip

Change History (5)

by pere@…, 19 years ago

Attachment: warnings.diff added

Enable more warnings from GCC

comment:1 by fwarmerdam, 19 years ago

Cc: mapserver-bugs@… added
Owner: changed from mapserverbugs to fwarmerdam
I'll take this one unless someone else really wants it.  I only plan to 
implement in 4.5, not 4.4.x. 

comment:2 by fwarmerdam, 19 years ago

Resolution: fixed
Status: newclosed
OK, I have reviewed that patch, and come to realize that amoung other things
it enables a big bunch of new strict warnings.  The MapServer team has made
no committment to addressing warnings beyond the "-Wall" warnings so I don't
feel it would be appropriate to enable all these warnings by default. 

What I have done is add *another* configure switch to enable strict 
warnings: --with-warnings. 

By default we will configure to use -Wall.  It is also possible to set specific
warnings as an argument to --with-warnings, or to select --without-warnings to
disable even -Wall. 

I also restructured the warnings logic in configure substantially, removing
the two warnings related macros from aclocal.m4, and disentangling the 
warnings logic from the debug logic. 

comment:3 by pere@…, 19 years ago

Several of the warnings emitted by gcc when increasing the warning
level are considered errors by other compilers.  Others are real
bugs in the code, which go undetected because only -Wall is used.
This is the reason I recommend and urge you to increase the
default warning levels used when compiling mapserver.

But I can live with the --with-warnings flag.  The problem is that
this will give more work for those enabling it, as they will try
to get rid of all the problems detected when using it, and
less work for the people introducing these problems without
seeing the warnings.

I'll soon try to compile mapserver using the compiler on other
unix platforms, and report the bugs detected that way.  I know for
sure several of the signed/unsigned problems (using enums as ints)
will trigger errors.  More on this later.

comment:4 by fwarmerdam, 19 years ago

Petter,

I understand your point, but standard warnings level is something that really
needs to be agreed upon by the developer community.

Until 4.4 even -Wall reports loads of warnings and because there were so many
warnings were pretty much ignored meaning that serious warnings were ignored
under the assumption there were part of the background noise.  

The developers (especially Howard and I) put quite a bit of effort into cleaning
up the warnings for -Wall for the 4.4 release with the intent that all warnings
would be taken seriously and address promptly.   I think this is progress. 

However, by introducing alot of new warnings, we now have rafts of warnings
again.  Many of which (like the unused parameters) I consider cruft not 
worth "fixing".  If we just made these warnings standard, I think you are
just going to see developers start ignoring warnings again. 

As a relative outsider to the MapServer development process, I would suggest
that trying to push a very strict warning protocol is not the best place for
you to contribute to MapServer. 

Of course, when you run into specific errors building MapServer on other
platforms, we welcome your patches.  We will try to incorporate them if
practical - though support for compilers other than gcc, and MSVC++ is not
really a priority for us (me at least).  

If you do wish to encourage more strict warnings policy, you might want to 
advocate on their behalf on the -dev list.  For instance list each of the
warnings options that you think we should have in our "standard" warnings, 
what the benefits are, and what sorts of warnings enabling it will trigger. 
If you get buy-in from several developers then it could be migrated into the
standard list along with -Wall and we could fix up the associated warnings
in the code. 



Note: See TracTickets for help on using tickets.