Re: [Qemu-devel] [PATCH] pixman: remove -Wredundand-decls

[Alexey Kardashevskiy]

On 04/15/2013 06:23 PM, Markus Armbruster wrote:
> Alexey Kardashevskiy <address@hidden> writes:
>
> > On 04/15/2013 05:08 PM, Markus Armbruster wrote:
> > > Alexey Kardashevskiy <address@hidden> writes:
> > >
> > > > The assert.h header file from Fedora18 does not have #ifdef-#endif
> > > > brackets around __assertXXXX function so it cannot compile with
> > > > the -Wredundant-decls switch on.
> > > >
> > > > Some Linux distributions (such as Debian Wheezy) still do have those
> > > > brackets arounb __assertXXXX functions (#ifndef _ASSERT_H_DECLS) but
> > > > the version of assert.h on http://sourceware.org/git/?p=glibc.git does not
> > > > so we should not be using -Wredundant-decls.
> > > >
> > > > The patch removes it.
> > > >
> > > > Signed-off-by: Alexey Kardashevskiy <address@hidden>
> > > > ---
> > > >    include/ui/qemu-pixman.h |    6 ------
> > > >    1 file changed, 6 deletions(-)
> > > >
> > > > diff --git a/include/ui/qemu-pixman.h b/include/ui/qemu-pixman.h
> > > > index b032f52..6f473f9 100644
> > > > --- a/include/ui/qemu-pixman.h
> > > > +++ b/include/ui/qemu-pixman.h
> > > > @@ -7,13 +7,7 @@
> > > >    #define QEMU_PIXMAN_H
> > > >
> > > >    /* pixman-0.16.0 headers have a redundant declaration */
> > > > -#ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE
> > > > -#pragma GCC diagnostic ignored "-Wredundant-decls"
> > > > -#endif
> > > >    #include <pixman.h>
> > > > -#ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE
> > > > -#pragma GCC diagnostic error "-Wredundant-decls"
> > > > -#endif
> > > >
> > > >    #include "qemu/typedefs.h"
> > >
> > > Patch description doesn't seem to fit the patch.  The patch doesn't
> > > remove -Wredundant-decls, it removes its suppression in one specific
> > > place.  Please advise.
> >
> > The patch removes both suppression AND enabling, the second chunk
> > enabled this check back after #include, no matter if it was enabled or
> > not.
>
> Trouble is there is no second chunk.


The patch removes 3 lines twice. The second 3 lines contain:
"#pragma GCC diagnostic error "-Wredundant-decls""

This enables the check.

My bad, it is not "patch" chunk. Sorry for confusing.

> If you're proposing to remove -Wredundant-decls globally, you'll have to
> explain what problems exactly it causes.

Besides assert.h is just broken and should not be included twice when 
-Wredundant-decls, I got a lot of this when did cross compilation (for 
ppc64 on x86_64):

  CC    hw/bt/sdp.o
In file included from 
/home/alexey/pcipassthru/qemu-impreza/include/qapi/qmp/qobject.h:36:0,
                 from 
/home/alexey/pcipassthru/qemu-impreza/include/qapi/qmp/qdict.h:16,
                 from 
/home/alexey/pcipassthru/qemu-impreza/include/ui/console.h:5,
                 from ui/qemu-pixman.c:7:
/home/alexey/pcipassthru/qemu-impreza/../lib4qemu/usr/include/assert.h:67:13: 
error: redundant redeclaration of '__assert_fail' [-Werror=redundant-decls]
/home/alexey/pcipassthru/qemu-impreza/../lib4qemu/usr/include/assert.h:67:13: 
note: previous declaration of '__assert_fail' was here
/home/alexey/pcipassthru/qemu-impreza/../lib4qemu/usr/include/assert.h:72:13: 
error: redundant redeclaration of '__assert_perror_fail' 
[-Werror=redundant-decls]
/home/alexey/pcipassthru/qemu-impreza/../lib4qemu/usr/include/assert.h:72:13: 
note: previous declaration of '__assert_perror_fail' was here
/home/alexey/pcipassthru/qemu-impreza/../lib4qemu/usr/include/assert.h:79:13: 
error: redundant redeclaration of '__assert' [-Werror=redundant-decls]
/home/alexey/pcipassthru/qemu-impreza/../lib4qemu/usr/include/assert.h:79:13: 
note: previous declaration of '__assert' was here
In file included from 
/home/alexey/pcipassthru/qemu-impreza/include/qapi/qmp/qobject.h:36:0,
                 from 
/home/alexey/pcipassthru/qemu-impreza/include/qapi/qmp/qdict.h:16,
                 from 
/home/alexey/pcipassthru/qemu-impreza/include/ui/console.h:5,
                 from ui/console.c:25:


If we disable that warning, I do not what we loose. The "bug" above is not 
a bug at all.


> > > Oh, and use a spell-checker :)
> >
> > The one build into thunderbird does not show any spelling errors
> > (except file names, of course) :)
>
> arounb
> -Wredundand-decls

Ah. My bad :(


--
Alexey