On Wed, Mar 21, 2007 at 02:41:06AM +0000, Daniel P. Berrange wrote:

Since testing the latest Fedora RPMs I've hit a number of buffer overflow issues which were caught by the extra compiler checks Fedora turns on. It would be much better if we caught these before release, so the attached patch modifies the configure script so that the following options are always turned on if the compiler supports them:

-Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -fasynchronous-unwind-tables

Since all our production builds use these flags & the patch only enables them if the compiler has support I don't see any issue with having them turned on by default.

Yes that makes sense ! In general I appreciate having as much warnings from the compiler as possible, as long as they will never break the build (i.e. remains warning and don't risk turning into errors 2 years from now when we try to rebuild the same code as packaged with a new or different compiler). For libxml2/libxslt I use the following, but don't activate them in default build (because some very old gcc version may choke on unknown options): -pedantic -W -Wformat -Wunused -Wimplicit -Wreturn-type -Wswitch -Wcomment -Wtrigraphs -Wformat -Wchar-subscripts -Wuninitialized -Wparentheses -Wshadow -Wpointer-arith -Wcast-align -Wwrite-strings -Waggregate-return -Wstrict-prototypes -Wmissing-prototypes -Wnested-externs -Winline -Wredundant-decls Also adding those extra flags should of course be in a section tested against gcc (which you patch does), I would really like to keep compiler portability, my experience is that just switching compilers when possible allow to detect different bugs than those raised by the gcc compilers. Someone with access to Solaris might for example forward here the warnings that I'm sure Sun Studio (or other ones) would raise, similary reports from icc builds would be welcome :-)

In addition I altered the existing configure compiler flag checks so that every compiler flag we turned on is explicitly checked to see if supported by the current compiler rather than doing a crude $CC=gcc heuristic which doesn't take account of differing gcc version numbers.

If we wanted super-extra paranoia we could also turn on -fstack-protector-all possibly only with --enable-compiler-warnings=maximum since it adds more non-trivial performance overhead

Patch looks fine to me but I'm not really confident with auto* to guess the result just by reading ! Please apply :-) Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/