[libvirt] [PATCH 1/6] Use gnulib's manywarnings & warnings modules
Daniel P. Berrange
berrange at redhat.com
Tue Apr 5 10:41:36 UTC 2011
On Mon, Apr 04, 2011 at 02:29:06PM -0600, Eric Blake wrote:
> 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).
Ah, I didn't realize it was the same.
>
> > + # 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.
I also added
gl_WARN_ADD([-Wjump-misses-init])
because we loose that due to turning off -Wc++-compat
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
More information about the libvir-list
mailing list