Re: [libvirt] [PATCH 1/6] Use gnulib's manywarnings & warnings modules

On 04/04/2011 10:19 AM, Daniel P. Berrange wrote: > Remove custom code for checking compiler warnings, using > gl_WARN_ADD instead. Don't list all flags ourselves, use > gnulib's gl_MANYWARN_ALL_GCC to get all possible GCC flags, > then turn off the ones we don't want yet. > > * acinclude.m4: Rewrite to use gl_WARN_ADD and gl_MANYWARN_ALL_GCC > * bootstrap.conf: Add warnings & manywarnings > * configure.ac: Switch to gl_WARN_ADD > * m4/compiler-flags.m4: Obsoleted by gl_WARN_ADD > --- > acinclude.m4 | 158 +++++++++++++++++++++++++++----------------------- > bootstrap.conf | 2 + > configure.ac | 15 +++-- > m4/compiler-flags.m4 | 48 --------------- > 4 files changed, 96 insertions(+), 127 deletions(-) > delete mode 100644 m4/compiler-flags.m4 > + > + # List of warnings that are not relevant / wanted > + dontwarn="$dontwarn -Wc++-compat" # Don't care about C++ compiler compat > + dontwarn="$dontwarn -Wtraditional" # Don't care about ancient C standard compat > + dontwarn="$dontwarn -Wtraditional-conversion" # Don't care about ancient C standard compat > + dontwarn="$dontwarn -Wsystem-headers" # Ignore warnings in /usr/include > + dontwarn="$dontwarn -Wpadded" # Happy for compiler to add struct padding > + dontwarn="$dontwarn -Wunreachable-code" # GCC very confused with -O2 > + dontwarn="$dontwarn -Wconversion" # Too many to deal with > + dontwarn="$dontwarn -Wsign-conversion" # Too many to deal with > + dontwarn="$dontwarn -Wvla" # GNULIB gettext.h violates > + dontwarn="$dontwarn -Wundef" # Many GNULIB violations > + dontwarn="$dontwarn -Wcast-qual" # Need to allow bad cast for execve() > + dontwarn="$dontwarn -Wlong-long" # We need to use long long in many places > + dontwarn="$dontwarn -Wswitch-default" # We allow manual list of all enum cases without default: > + dontwarn="$dontwarn -Wswitch-enum" # We allow optional default: instead of listing all enum values > + dontwarn="$dontwarn -Wstrict-overflow" # Not a problem since we don't use -fstrict-overflow > + dontwarn="$dontwarn -Wunsafe-loop-optimizations" # Not a problem since we don't use -funsafe-loop-optimizations Seems okay (but long lines - any way to break that into 80 columns?) > + > + # We might fundamentally need some of these disabled forever, but ideally > + # we'd turn many of them on > + dontwarn="$dontwarn -Wformat-nonliteral" This one may need to always be disabled, thanks to virAsprintf (it would be nicer if we could disable for just one or two files, rather than globally). > + dontwarn="$dontwarn -Wfloat-equal" > + dontwarn="$dontwarn -Wdeclaration-after-statement" If gcc would do better, I'd love to guarantee C99 and take advantage of this (smaller scopes have maintenance benefits) - thus, I envision this one being permanent (float it up to the earlier set). > + dontwarn="$dontwarn -Wcast-qual" > + dontwarn="$dontwarn -Wconversion" > + dontwarn="$dontwarn -Wsign-conversion" > + dontwarn="$dontwarn -Wold-style-definition" > + dontwarn="$dontwarn -Wmissing-noreturn" > + dontwarn="$dontwarn -Wpacked" > + dontwarn="$dontwarn -Wunused-macros" > + dontwarn="$dontwarn -Woverlength-strings" > + dontwarn="$dontwarn -Wmissing-format-attribute" > + dontwarn="$dontwarn -Wstack-protector" Yes, I agree that the rest of these should be temporary, and are probably worth fixing. > + > + # Get all possible GCC warnings > + gl_MANYWARN_ALL_GCC([maybewarn]) > + > + # Remove the ones we don't want, blacklisted earlier > + gl_MANYWARN_COMPLEMENT([wantwarn], [$maybewarn], [$dontwarn]) > + > + # Check for $CC support of each warning > + for w in $wantwarn; do > + gl_WARN_ADD([$w]) > + done > + > + # GNULIB uses '-W' which includes a bunch of stuff, > + # kinda like -Wextra. Unfortunately, it means you "kinda like"? Exactly like! -W was the old spelling, -Wextra was the new one (added quite some time ago; something like gcc 3.4, off the top of my head).

> + # can't simply use '-Wsign-compare' with gl_MANYWARN_COMPLEMENT > + # So we have -W enabled, and then have to explicitly turn off > + gl_WARN_ADD(-Wno-sign-compare) > + > + # This should be < 256 really, but with PATH_MAX everywhere > + # we have doom, even with 4096. In fact we have some functions > + # with several PATH_MAX sized variables :-( We should kill off > + # all PATH_MAX usage and then lower this limit > + gl_WARN_ADD([-Wframe-larger-than=65700]) > + dnl gl_WARN_ADD([-Wframe-larger-than=4096]) > + dnl gl_WARN_ADD([-Wframe-larger-than=256]) > + > + # Extra special flags > + gl_WARN_ADD([-Wp,-D_FORTIFY_SOURCE=2]) > + dnl Fedora only uses -fstack-protector, but doesn't seem to > + dnl be great overhead in adding -fstack-protector-all instead > + dnl gl_WARN_ADD([-fstack-protector]) > + gl_WARN_ADD([-fstack-protector-all]) > + gl_WARN_ADD([--param=ssp-buffer-size=4]) > + gl_WARN_ADD([-fexceptions]) > + gl_WARN_ADD([-fasynchronous-unwind-tables]) > + gl_WARN_ADD([-fdiagnostics-show-option]) > + > + if test "$enable_compile_warnings" = "error" > + then > + gl_WARN_ADD([-Werror]) > + fi Looks like a good conversion.