[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On Fri, Apr 01, 2011 at 10:02:18AM -0600, Eric Blake wrote:
> On 04/01/2011 09:39 AM, Daniel P. Berrange wrote:
> > 
> > * 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         |  153 ++++++++++++++++++++++++++------------------------
> 
> I really don't like how we have crammed so much into acinclude.m4; it
> would be nicer if we could separate things into individual m4/foo.m4
> files...

The new series moves this file

> > +        # 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
> 
> We really ought to follow coreutils' lead of generating two separate
> lists of -W flags - a larger set for libvirt and a smaller set for
> gnulib.  Then exemptions such as -Wvla could be used just for gnulib.
> But that can be a followup patch.

The problem isn't with the compilation of gnulib - that doesn't use
any of these flags. The issue is with various things in the gnulib
header files, which impact compilation of the main libvirt code.
eg

  ../gnulib/lib/gettext.h:218:3: error: variable length array 'msg_ctxt_id' is used [-Wvla]


> 
> > +        dontwarn="$dontwarn -Wcast-qual"              # Need to allow bad cast for execve()
> > +        dontwarn="$dontwarn -Wlong-long"              # We need to use long long in many places
> > +
> > +        # We might fundamentally need some of these disabled forever, but ideally
> > +        # we'd turn many of them on
> > +        dontwarn="$dontwarn -Wformat-nonliteral"
> > +        dontwarn="$dontwarn -Wswitch-default"
> > +        dontwarn="$dontwarn -Wswitch-enum"
> > +        dontwarn="$dontwarn -Wstrict-overflow"
> > +        dontwarn="$dontwarn -Wfloat-equal"
> > +        dontwarn="$dontwarn -Wdeclaration-after-statement"
> > +        dontwarn="$dontwarn -Wunsafe-loop-optimizations"
> > +        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 -W"
> 
> -W is the same as -Wextra - that turns on a lot of useful warnings.  Can
> we fine-tune it to just disable the needed warnings?

Removing that means I need a manual check for -Wno-sign-compare
since gl_MANYWAY_COMPLEMENT can't see that -W include -Wsign-compare

> 
> > +        dontwarn="$dontwarn -Woverlength-strings"
> > +        dontwarn="$dontwarn -Wmissing-format-attribute"
> > +        dontwarn="$dontwarn -Wstack-protector"
> > +
> > +        # 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
> 
> Yep, gnulib's macros are nice for this!


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 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]