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

[Alexey Kardashevskiy <aik@ozlabs.ru>]

On 04/15/2013 06:23 PM, Markus Armbruster wrote:
> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>
>> On 04/15/2013 05:08 PM, Markus Armbruster wrote:
>>> Alexey Kardashevskiy <aik@ozlabs.ru> 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 <aik@ozlabs.ru>
>>>> ---
>>>>    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