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

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

[Alexey Kardashevskiy]

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"
 
-- 
1.7.10.4

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

[Markus Armbruster]

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.

Oh, and use a spell-checker :)

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

[Alexey Kardashevskiy]

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.


> Oh, and use a spell-checker :)

The one build into thunderbird does not show any spelling errors (except 
file names, of course) :)



-- 
Alexey

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

[Markus Armbruster]

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.

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

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

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

[Alexey Kardashevskiy]

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

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

[Peter Maydell]

On 15 April 2013 10:50, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> /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]

This copy of assert.h seems to be broken. The declarations
should be guarded (by _ASSERT_H_DECLS in my system's copy).
If it's widespread we might have to work around this.

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

What we lose is that we are no longer informed when people
inadvertently introduce redundant declarations into QEMU itself.

-- PMM

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

[Peter Maydell]

On 15 April 2013 11:01, Peter Maydell <peter.maydell@linaro.org> wrote:
> This copy of assert.h seems to be broken. The declarations
> should be guarded (by _ASSERT_H_DECLS in my system's copy).

FWIW, the _ASSERT_H_DECLS fix appears to be a Debian specific
patch (added in 2003; I can't find any record of it ever being
submitted upstream).

-- PMM

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

[Alexey Kardashevskiy]

On 04/15/2013 08:01 PM, Peter Maydell wrote:
> On 15 April 2013 10:50, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>> /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]
>
> This copy of assert.h seems to be broken. The declarations
> should be guarded (by _ASSERT_H_DECLS in my system's copy).

Debian? It uses eglibc which is fork (or clone?) of glibc.

> If it's widespread we might have to work around this.

It is in fedora 18 and glibc's git master branch. Why "if"?

And this is not it. I just managed to suppress others with 
-Wredundand-decls, like the one with strtold() defined twice in 
/usr/include/stdlib.h and /usr/include/bits/stdlib-ldbl.h and some others.


I suspect I am getting these errors because I am the only person who is 
trying to cross compile or/and when host and target endianness differ.

Does anyone do cross compilation (with different endianness?) on a regular 
basis? My colleagues do not :)


>> If we disable that warning, I do not what we loose. The "bug" above is not a
>> bug at all.
>
> What we lose is that we are no longer informed when people
> inadvertently introduce redundant declarations into QEMU itself.



-- 
Alexey

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

[Peter Maydell]

On 15 April 2013 11:52, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> I suspect I am getting these errors because I am the only person who is
> trying to cross compile or/and when host and target endianness differ.

Cross compilation or otherwise should have zero effect here.
You may be hitting more issues because your target libc
happens to be one that not very many other people are targetting.

-- PMM

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

[Alexey Kardashevskiy]

On 04/15/2013 09:00 PM, Peter Maydell wrote:
> On 15 April 2013 11:52, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>> I suspect I am getting these errors because I am the only person who
>> is trying to cross compile or/and when host and target endianness
>> differ.
>
> Cross compilation or otherwise should have zero effect here.

When I build on native ppc64 system (***) with fc18, everything is ok.
When I build on x86_64 with headers/libraries simply copied from the (***),
I get these errors. gcc is 4.7.2 on ppc64 machine and 4.6.3..4.8.0 on
x86_64 (cross compiler).


> You may be hitting more issues because your target libc happens to be
> one that not very many other people are targetting.

Is standard glibc not popular any more? Wow. Neither RHEL (5.0+) nor Fedora
(16, 18) has that _ASSERT_H_DECLS in assert.h. So what is the target for
the QEMU people then?


-- 
Alexey

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

[Peter Maydell]

On 15 April 2013 12:08, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> On 04/15/2013 09:00 PM, Peter Maydell wrote:
>> You may be hitting more issues because your target libc happens to be
>> one that not very many other people are targetting.
>
>
> Is standard glibc not popular any more? Wow. Neither RHEL (5.0+) nor Fedora
> (16, 18) has that _ASSERT_H_DECLS in assert.h. So what is the target for
> the QEMU people then?

Well, compilation of QEMU against glibc for ppc64 probably
doesn't get much testing.

-- PMM

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

[Alexey Kardashevskiy]

On 04/15/2013 09:14 PM, Peter Maydell wrote:
> On 15 April 2013 12:08, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>> On 04/15/2013 09:00 PM, Peter Maydell wrote:
>>> You may be hitting more issues because your target libc happens to be
>>> one that not very many other people are targetting.
>>
>>
>> Is standard glibc not popular any more? Wow. Neither RHEL (5.0+) nor Fedora
>> (16, 18) has that _ASSERT_H_DECLS in assert.h. So what is the target for
>> the QEMU people then?
>
> Well, compilation of QEMU against glibc for ppc64 probably
> doesn't get much testing.

assert.h is exactly the same on my thinkpad (x86_64) with fedora18 as all 
(most?) standard headers. If I replace powerpc64-linux-gcc with the 
standard x86_64 gcc, I get the same error.


-- 
Alexey

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

[Markus Armbruster]

Alexey Kardashevskiy <aik@ozlabs.ru> writes:

> On 04/15/2013 08:01 PM, Peter Maydell wrote:
>> On 15 April 2013 10:50, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>> /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]
>>
>> This copy of assert.h seems to be broken. The declarations
>> should be guarded (by _ASSERT_H_DECLS in my system's copy).
>
> Debian? It uses eglibc which is fork (or clone?) of glibc.
>
>> If it's widespread we might have to work around this.
>
> It is in fedora 18 and glibc's git master branch. Why "if"?

It's in Fedora 17, too, but I *don't* get a warning.  Suspecting your
compiler.  --version?

[...]

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

[Alexey Kardashevskiy]

On 04/15/2013 10:57 PM, Markus Armbruster wrote:
> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>
>> On 04/15/2013 08:01 PM, Peter Maydell wrote:
>>> On 15 April 2013 10:50, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>> /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]
>>>
>>> This copy of assert.h seems to be broken. The declarations
>>> should be guarded (by _ASSERT_H_DECLS in my system's copy).
>>
>> Debian? It uses eglibc which is fork (or clone?) of glibc.
>>
>>> If it's widespread we might have to work around this.
>>
>> It is in fedora 18 and glibc's git master branch. Why "if"?
>
> It's in Fedora 17, too, but I *don't* get a warning.  Suspecting your
> compiler.  --version?


powerpc64-linux-gcc 4.6.3, 4.7.2, 4.8.0, all the same. I'll try to track it 
down tomorrow why it all works when host and target are the same (pretty 
sure this is the cse) but I just do not get it... It is just me who sees 
obvious error in assert.h which is caused by -Wno-redundant-decls? Even if 
you do not hit this now, you will get there eventually.



-- 
Alexey

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

[Markus Armbruster]

Alexey Kardashevskiy <aik@ozlabs.ru> writes:

> On 04/15/2013 10:57 PM, Markus Armbruster wrote:
>> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>>
>>> On 04/15/2013 08:01 PM, Peter Maydell wrote:
>>>> On 15 April 2013 10:50, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>>> /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]
>>>>
>>>> This copy of assert.h seems to be broken. The declarations
>>>> should be guarded (by _ASSERT_H_DECLS in my system's copy).
>>>
>>> Debian? It uses eglibc which is fork (or clone?) of glibc.
>>>
>>>> If it's widespread we might have to work around this.
>>>
>>> It is in fedora 18 and glibc's git master branch. Why "if"?
>>
>> It's in Fedora 17, too, but I *don't* get a warning.  Suspecting your
>> compiler.  --version?
>
>
> powerpc64-linux-gcc 4.6.3, 4.7.2, 4.8.0, all the same. I'll try to
> track it down tomorrow why it all works when host and target are the
> same (pretty sure this is the cse) but I just do not get it... It is
> just me who sees obvious error in assert.h which is caused by
> -Wno-redundant-decls? Even if you do not hit this now, you will get
> there eventually.

I don't doubt your gcc+libc is in error.  I just don't want to lose a
useful warning because of that.

Workaround: configure --disable-werror

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

[Alexey Kardashevskiy]

On 04/16/2013 01:55 AM, Markus Armbruster wrote:
> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>
>> On 04/15/2013 10:57 PM, Markus Armbruster wrote:
>>> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>>>
>>>> On 04/15/2013 08:01 PM, Peter Maydell wrote:
>>>>> On 15 April 2013 10:50, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>>>> /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]
>>>>>
>>>>> This copy of assert.h seems to be broken. The declarations
>>>>> should be guarded (by _ASSERT_H_DECLS in my system's copy).
>>>>
>>>> Debian? It uses eglibc which is fork (or clone?) of glibc.
>>>>
>>>>> If it's widespread we might have to work around this.
>>>>
>>>> It is in fedora 18 and glibc's git master branch. Why "if"?
>>>
>>> It's in Fedora 17, too, but I *don't* get a warning.  Suspecting your
>>> compiler.  --version?
>>
>>
>> powerpc64-linux-gcc 4.6.3, 4.7.2, 4.8.0, all the same. I'll try to
>> track it down tomorrow why it all works when host and target are the
>> same (pretty sure this is the cse) but I just do not get it... It is
>> just me who sees obvious error in assert.h which is caused by
>> -Wno-redundant-decls? Even if you do not hit this now, you will get
>> there eventually.
>
> I don't doubt your gcc+libc is in error.  I just don't want to lose a
> useful warning because of that.
 >
> Workaround: configure --disable-werror

This workaround does NOT work if pragmas used. "#pragma GCC diagnostic 
error "-Wredundant-decls"" re-enables warnings as errors.



-- 
Alexey

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

[Alexey Kardashevskiy]

On 04/16/2013 08:48 AM, Alexey Kardashevskiy wrote:
> On 04/16/2013 01:55 AM, Markus Armbruster wrote:
>> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>>
>>> On 04/15/2013 10:57 PM, Markus Armbruster wrote:
>>>> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>>>>
>>>>> On 04/15/2013 08:01 PM, Peter Maydell wrote:
>>>>>> On 15 April 2013 10:50, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>>>>> /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]
>>>>>>
>>>>>> This copy of assert.h seems to be broken. The declarations
>>>>>> should be guarded (by _ASSERT_H_DECLS in my system's copy).
>>>>>
>>>>> Debian? It uses eglibc which is fork (or clone?) of glibc.
>>>>>
>>>>>> If it's widespread we might have to work around this.
>>>>>
>>>>> It is in fedora 18 and glibc's git master branch. Why "if"?
>>>>
>>>> It's in Fedora 17, too, but I *don't* get a warning.  Suspecting your
>>>> compiler.  --version?
>>>
>>>
>>> powerpc64-linux-gcc 4.6.3, 4.7.2, 4.8.0, all the same. I'll try to
>>> track it down tomorrow why it all works when host and target are the
>>> same (pretty sure this is the cse) but I just do not get it... It is
>>> just me who sees obvious error in assert.h which is caused by
>>> -Wno-redundant-decls? Even if you do not hit this now, you will get
>>> there eventually.
>>
>> I don't doubt your gcc+libc is in error.  I just don't want to lose a
>> useful warning because of that.
>  >
>> Workaround: configure --disable-werror
>
> This workaround does NOT work if pragmas used. "#pragma GCC diagnostic
> error "-Wredundant-decls"" re-enables warnings as errors.


Kind of offtopic but still...

I think this is just beautiful. Fedora18, x86_64, NO cross compiler. gcc 
does not apply -Wredundant-decls to /usr/include/* but does it for all 
other headers and in the case of cross compilation I hit this case.

Does anyone know the way to tell gcc that libc headers are not at 
/usr/include but somewhere else?



[aik@aik ~]$ cp /usr/include/assert.h ./
[aik@aik ~]$
[aik@aik ~]$ cat a.c
#pragma GCC diagnostic error "-Wredundant-decls"

#ifdef USEMINE
#include "assert.h"
#include "assert.h"
#else
#include <assert.h>
#include <assert.h>
#endif

int main(int argc, char **argv){ return 0; }
[aik@aik ~]$
[aik@aik ~]$ gcc a.c -o a
[aik@aik ~]$ gcc a.c -o a -DUSEMINE
In file included from a.c:5:0:
assert.h:68:13: error: redundant redeclaration of ‘__assert_fail’ 
[-Werror=redundant-decls]
In file included from a.c:4:0:
assert.h:68:13: note: previous declaration of ‘__assert_fail’ was here
In file included from a.c:5:0:
assert.h:73:13: error: redundant redeclaration of ‘__assert_perror_fail’ 
[-Werror=redundant-decls]
In file included from a.c:4:0:
assert.h:73:13: note: previous declaration of ‘__assert_perror_fail’ was here
In file included from a.c:5:0:
assert.h:80:13: error: redundant redeclaration of ‘__assert’ 
[-Werror=redundant-decls]
In file included from a.c:4:0:
assert.h:80:13: note: previous declaration of ‘__assert’ was here
cc1: some warnings being treated as errors
[aik@aik ~]$




-- 
Alexey

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

[Markus Armbruster]

Gcc issue, perhaps Paolo [cc'ed] got an idea.

Alexey Kardashevskiy <aik@ozlabs.ru> writes:

> On 04/16/2013 08:48 AM, Alexey Kardashevskiy wrote:
>> On 04/16/2013 01:55 AM, Markus Armbruster wrote:
>>> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>>>
>>>> On 04/15/2013 10:57 PM, Markus Armbruster wrote:
>>>>> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>>>>>
>>>>>> On 04/15/2013 08:01 PM, Peter Maydell wrote:
>>>>>>> On 15 April 2013 10:50, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>>>>>> /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]
>>>>>>>
>>>>>>> This copy of assert.h seems to be broken. The declarations
>>>>>>> should be guarded (by _ASSERT_H_DECLS in my system's copy).
>>>>>>
>>>>>> Debian? It uses eglibc which is fork (or clone?) of glibc.
>>>>>>
>>>>>>> If it's widespread we might have to work around this.
>>>>>>
>>>>>> It is in fedora 18 and glibc's git master branch. Why "if"?
>>>>>
>>>>> It's in Fedora 17, too, but I *don't* get a warning.  Suspecting your
>>>>> compiler.  --version?
>>>>
>>>>
>>>> powerpc64-linux-gcc 4.6.3, 4.7.2, 4.8.0, all the same. I'll try to
>>>> track it down tomorrow why it all works when host and target are the
>>>> same (pretty sure this is the cse) but I just do not get it... It is
>>>> just me who sees obvious error in assert.h which is caused by
>>>> -Wno-redundant-decls? Even if you do not hit this now, you will get
>>>> there eventually.
>>>
>>> I don't doubt your gcc+libc is in error.  I just don't want to lose a
>>> useful warning because of that.
>>  >
>>> Workaround: configure --disable-werror
>>
>> This workaround does NOT work if pragmas used. "#pragma GCC diagnostic
>> error "-Wredundant-decls"" re-enables warnings as errors.
>
>
> Kind of offtopic but still...
>
> I think this is just beautiful. Fedora18, x86_64, NO cross
> compiler. gcc does not apply -Wredundant-decls to /usr/include/* but
> does it for all other headers and in the case of cross compilation I
> hit this case.
>
> Does anyone know the way to tell gcc that libc headers are not at
> /usr/include but somewhere else?
>
>
>
> [aik@aik ~]$ cp /usr/include/assert.h ./
> [aik@aik ~]$
> [aik@aik ~]$ cat a.c
> #pragma GCC diagnostic error "-Wredundant-decls"
>
> #ifdef USEMINE
> #include "assert.h"
> #include "assert.h"
> #else
> #include <assert.h>
> #include <assert.h>
> #endif
>
> int main(int argc, char **argv){ return 0; }
> [aik@aik ~]$
> [aik@aik ~]$ gcc a.c -o a
> [aik@aik ~]$ gcc a.c -o a -DUSEMINE
> In file included from a.c:5:0:
> assert.h:68:13: error: redundant redeclaration of ‘__assert_fail’
> [-Werror=redundant-decls]
> In file included from a.c:4:0:
> assert.h:68:13: note: previous declaration of ‘__assert_fail’ was here
> In file included from a.c:5:0:
> assert.h:73:13: error: redundant redeclaration of
> ‘__assert_perror_fail’ [-Werror=redundant-decls]
> In file included from a.c:4:0:
> assert.h:73:13: note: previous declaration of ‘__assert_perror_fail’ was here
> In file included from a.c:5:0:
> assert.h:80:13: error: redundant redeclaration of ‘__assert’
> [-Werror=redundant-decls]
> In file included from a.c:4:0:
> assert.h:80:13: note: previous declaration of ‘__assert’ was here
> cc1: some warnings being treated as errors
> [aik@aik ~]$

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

[Paolo Bonzini]

Il 16/04/2013 09:57, Markus Armbruster ha scritto:
>>
>> I think this is just beautiful. Fedora18, x86_64, NO cross
>> compiler. gcc does not apply -Wredundant-decls to /usr/include/* but
>> does it for all other headers and in the case of cross compilation I
>> hit this case.

'-Wsystem-headers'
     Print warning messages for constructs found in system header files.
     Warnings from system headers are normally suppressed, on the
     assumption that they usually do not indicate real problems and
     would only make the compiler output harder to read.

>> Does anyone know the way to tell gcc that libc headers are not at
>> /usr/include but somewhere else?

I think this helps:

'--sysroot=DIR'
     Use DIR as the logical root directory for headers and libraries.
     For example, if the compiler normally searches for headers in
     '/usr/include' and libraries in '/usr/lib', it instead searches
     'DIR/usr/include' and 'DIR/usr/lib'.

but you shouldn't need it.  Just configure your GCC with
--with-sysroot=/foo and it should just work.

Also:

> This workaround does NOT work if pragmas used. "#pragma GCC diagnostic
> error "-Wredundant-decls"" re-enables warnings as errors.

The solution is to use push/pop like this:

diff --git a/coroutine-ucontext.c b/coroutine-ucontext.c
index 867a662..4bf2cde 100644
--- a/coroutine-ucontext.c
+++ b/coroutine-ucontext.c
@@ -169,6 +169,7 @@ Coroutine *qemu_coroutine_new(void)
 #ifdef CONFIG_VALGRIND_H
 #ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE
 /* Work around an unused variable in the valgrind.h macro... */
+#pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wunused-but-set-variable"
 #endif
 static inline void valgrind_stack_deregister(CoroutineUContext *co)
@@ -176,7 +177,7 @@ static inline void
valgrind_stack_deregister(CoroutineUContext *co)
     VALGRIND_STACK_DEREGISTER(co->valgrind_stack_id);
 }
 #ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE
-#pragma GCC diagnostic error "-Wunused-but-set-variable"
+#pragma GCC diagnostic pop
 #endif
 #endif

diff --git a/include/ui/qemu-pixman.h b/include/ui/qemu-pixman.h
index b032f52..882e2a3 100644
--- a/include/ui/qemu-pixman.h
+++ b/include/ui/qemu-pixman.h
@@ -8,11 +8,12 @@

 /* pixman-0.16.0 headers have a redundant declaration */
 #ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE
+#pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wredundant-decls"
 #endif
 #include <pixman.h>
 #ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE
-#pragma GCC diagnostic error "-Wredundant-decls"
+#pragma GCC diagnostic pop
 #endif

 #include "qemu/typedefs.h"

Untested, feel free to resubmit with my Signed-off-by.

Paolo

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

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

[...]
>> This workaround does NOT work if pragmas used. "#pragma GCC diagnostic
>> error "-Wredundant-decls"" re-enables warnings as errors.
>
> The solution is to use push/pop like this:
>
> diff --git a/coroutine-ucontext.c b/coroutine-ucontext.c
> index 867a662..4bf2cde 100644
> --- a/coroutine-ucontext.c
> +++ b/coroutine-ucontext.c
> @@ -169,6 +169,7 @@ Coroutine *qemu_coroutine_new(void)
>  #ifdef CONFIG_VALGRIND_H
>  #ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE
>  /* Work around an unused variable in the valgrind.h macro... */
> +#pragma GCC diagnostic push
>  #pragma GCC diagnostic ignored "-Wunused-but-set-variable"
>  #endif
>  static inline void valgrind_stack_deregister(CoroutineUContext *co)
> @@ -176,7 +177,7 @@ static inline void
> valgrind_stack_deregister(CoroutineUContext *co)
>      VALGRIND_STACK_DEREGISTER(co->valgrind_stack_id);
>  }
>  #ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE
> -#pragma GCC diagnostic error "-Wunused-but-set-variable"
> +#pragma GCC diagnostic pop
>  #endif
>  #endif
>
> diff --git a/include/ui/qemu-pixman.h b/include/ui/qemu-pixman.h
> index b032f52..882e2a3 100644
> --- a/include/ui/qemu-pixman.h
> +++ b/include/ui/qemu-pixman.h
> @@ -8,11 +8,12 @@
>
>  /* pixman-0.16.0 headers have a redundant declaration */
>  #ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE
> +#pragma GCC diagnostic push
>  #pragma GCC diagnostic ignored "-Wredundant-decls"
>  #endif
>  #include <pixman.h>
>  #ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE
> -#pragma GCC diagnostic error "-Wredundant-decls"
> +#pragma GCC diagnostic pop
>  #endif
>
>  #include "qemu/typedefs.h"
>
> Untested, feel free to resubmit with my Signed-off-by.
>
> Paolo

You missed the one in ui/gtk.c.  My patch covers it, and also configure.

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

[Alexey Kardashevskiy]

On 04/16/2013 07:22 PM, Paolo Bonzini wrote:
> Il 16/04/2013 09:57, Markus Armbruster ha scritto:
>>>
>>> I think this is just beautiful. Fedora18, x86_64, NO cross
>>> compiler. gcc does not apply -Wredundant-decls to /usr/include/* but
>>> does it for all other headers and in the case of cross compilation I
>>> hit this case.
>
> '-Wsystem-headers'
>       Print warning messages for constructs found in system header files.
>       Warnings from system headers are normally suppressed, on the
>       assumption that they usually do not indicate real problems and
>       would only make the compiler output harder to read.
>
>>> Does anyone know the way to tell gcc that libc headers are not at
>>> /usr/include but somewhere else?
>
> I think this helps:
>
> '--sysroot=DIR'
>       Use DIR as the logical root directory for headers and libraries.
>       For example, if the compiler normally searches for headers in
>       '/usr/include' and libraries in '/usr/lib', it instead searches
>       'DIR/usr/include' and 'DIR/usr/lib'.
>
> but you shouldn't need it.  Just configure your GCC with
> --with-sysroot=/foo and it should just work.


--sysroot helps for native compiler but does not for the cross compiler.

Anyway, I tried Markus's patch, now assert.h generates just warnings which 
I can successfully suppress. Cool, thanks everyone :)



-- 
Alexey

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

[Paolo Bonzini]

Il 16/04/2013 12:54, Alexey Kardashevskiy ha scritto:
>>
>> but you shouldn't need it.  Just configure your GCC with
>> --with-sysroot=/foo and it should just work.
> 
> 
> --sysroot helps for native compiler but does not for the cross compiler.

That usually points at an incorrect configuration when you built GCC.

> Anyway, I tried Markus's patch, now assert.h generates just warnings
> which I can successfully suppress. Cool, thanks everyone :)

His patch looks good.  Markus, can you repost as a top-level message
with my Reviewed-by?

Paolo

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

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Il 16/04/2013 12:54, Alexey Kardashevskiy ha scritto:
>>>
>>> but you shouldn't need it.  Just configure your GCC with
>>> --with-sysroot=/foo and it should just work.
>> 
>> 
>> --sysroot helps for native compiler but does not for the cross compiler.
>
> That usually points at an incorrect configuration when you built GCC.
>
>> Anyway, I tried Markus's patch, now assert.h generates just warnings
>> which I can successfully suppress. Cool, thanks everyone :)
>
> His patch looks good.  Markus, can you repost as a top-level message
> with my Reviewed-by?

Done.  Thank you both for the testing and the review!

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

[Markus Armbruster]

Alexey Kardashevskiy <aik@ozlabs.ru> writes:

> On 04/16/2013 01:55 AM, Markus Armbruster wrote:
>> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>>
>>> On 04/15/2013 10:57 PM, Markus Armbruster wrote:
>>>> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>>>>
>>>>> On 04/15/2013 08:01 PM, Peter Maydell wrote:
>>>>>> On 15 April 2013 10:50, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>>>>> /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]
>>>>>>
>>>>>> This copy of assert.h seems to be broken. The declarations
>>>>>> should be guarded (by _ASSERT_H_DECLS in my system's copy).
>>>>>
>>>>> Debian? It uses eglibc which is fork (or clone?) of glibc.
>>>>>
>>>>>> If it's widespread we might have to work around this.
>>>>>
>>>>> It is in fedora 18 and glibc's git master branch. Why "if"?
>>>>
>>>> It's in Fedora 17, too, but I *don't* get a warning.  Suspecting your
>>>> compiler.  --version?
>>>
>>>
>>> powerpc64-linux-gcc 4.6.3, 4.7.2, 4.8.0, all the same. I'll try to
>>> track it down tomorrow why it all works when host and target are the
>>> same (pretty sure this is the cse) but I just do not get it... It is
>>> just me who sees obvious error in assert.h which is caused by
>>> -Wno-redundant-decls? Even if you do not hit this now, you will get
>>> there eventually.
>>
>> I don't doubt your gcc+libc is in error.  I just don't want to lose a
>> useful warning because of that.
>>
>> Workaround: configure --disable-werror
>
> This workaround does NOT work if pragmas used. "#pragma GCC diagnostic
> error "-Wredundant-decls"" re-enables warnings as errors.

Bummer.  Could you try the appended patch?

diff --git a/configure b/configure
index 0788e27..41097a2 100755
--- a/configure
+++ b/configure
@@ -3244,8 +3244,10 @@ fi
 
 pragma_disable_unused_but_set=no
 cat > $TMPC << EOF
+#pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wunused-but-set-variable"
 #pragma GCC diagnostic ignored "-Wstrict-prototypes"
+#pragma GCC diagnostic pop
 
 int main(void) {
     return 0;
diff --git a/coroutine-ucontext.c b/coroutine-ucontext.c
index 867a662..4bf2cde 100644
--- a/coroutine-ucontext.c
+++ b/coroutine-ucontext.c
@@ -169,6 +169,7 @@ Coroutine *qemu_coroutine_new(void)
 #ifdef CONFIG_VALGRIND_H
 #ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE
 /* Work around an unused variable in the valgrind.h macro... */
+#pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wunused-but-set-variable"
 #endif
 static inline void valgrind_stack_deregister(CoroutineUContext *co)
@@ -176,7 +177,7 @@ static inline void valgrind_stack_deregister(CoroutineUContext *co)
     VALGRIND_STACK_DEREGISTER(co->valgrind_stack_id);
 }
 #ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE
-#pragma GCC diagnostic error "-Wunused-but-set-variable"
+#pragma GCC diagnostic pop
 #endif
 #endif
 
diff --git a/include/ui/qemu-pixman.h b/include/ui/qemu-pixman.h
index b032f52..882e2a3 100644
--- a/include/ui/qemu-pixman.h
+++ b/include/ui/qemu-pixman.h
@@ -8,11 +8,12 @@
 
 /* pixman-0.16.0 headers have a redundant declaration */
 #ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE
+#pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wredundant-decls"
 #endif
 #include <pixman.h>
 #ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE
-#pragma GCC diagnostic error "-Wredundant-decls"
+#pragma GCC diagnostic pop
 #endif
 
 #include "qemu/typedefs.h"
diff --git a/ui/gtk.c b/ui/gtk.c
index 1e105e2..c2c6e38 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -38,11 +38,12 @@
 
 #ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE
 /* Work around an -Wstrict-prototypes warning in GTK headers */
+#pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wstrict-prototypes"
 #endif
 #include <gtk/gtk.h>
 #ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE
-#pragma GCC diagnostic error "-Wstrict-prototypes"
+#pragma GCC diagnostic pop
 #endif
 
 

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

[Alexey Kardashevskiy]

On 04/16/2013 07:00 PM, Markus Armbruster wrote:
> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>
>> On 04/16/2013 01:55 AM, Markus Armbruster wrote:
>>> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>>>
>>>> On 04/15/2013 10:57 PM, Markus Armbruster wrote:
>>>>> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>>>>>
>>>>>> On 04/15/2013 08:01 PM, Peter Maydell wrote:
>>>>>>> On 15 April 2013 10:50, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>>>>>> /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]
>>>>>>>
>>>>>>> This copy of assert.h seems to be broken. The declarations
>>>>>>> should be guarded (by _ASSERT_H_DECLS in my system's copy).
>>>>>>
>>>>>> Debian? It uses eglibc which is fork (or clone?) of glibc.
>>>>>>
>>>>>>> If it's widespread we might have to work around this.
>>>>>>
>>>>>> It is in fedora 18 and glibc's git master branch. Why "if"?
>>>>>
>>>>> It's in Fedora 17, too, but I *don't* get a warning.  Suspecting your
>>>>> compiler.  --version?
>>>>
>>>>
>>>> powerpc64-linux-gcc 4.6.3, 4.7.2, 4.8.0, all the same. I'll try to
>>>> track it down tomorrow why it all works when host and target are the
>>>> same (pretty sure this is the cse) but I just do not get it... It is
>>>> just me who sees obvious error in assert.h which is caused by
>>>> -Wno-redundant-decls? Even if you do not hit this now, you will get
>>>> there eventually.
>>>
>>> I don't doubt your gcc+libc is in error.  I just don't want to lose a
>>> useful warning because of that.
>>>
>>> Workaround: configure --disable-werror
>>
>> This workaround does NOT work if pragmas used. "#pragma GCC diagnostic
>> error "-Wredundant-decls"" re-enables warnings as errors.
>
> Bummer.  Could you try the appended patch?

Tried, works, thanks!



-- 
Alexey

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

[Kevin Wolf]

Am 16.04.2013 um 11:00 hat Markus Armbruster geschrieben:
> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
> 
> > On 04/16/2013 01:55 AM, Markus Armbruster wrote:
> >> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
> >>
> >>> On 04/15/2013 10:57 PM, Markus Armbruster wrote:
> >>>> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
> >>>>
> >>>>> On 04/15/2013 08:01 PM, Peter Maydell wrote:
> >>>>>> On 15 April 2013 10:50, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> >>>>>>> /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]
> >>>>>>
> >>>>>> This copy of assert.h seems to be broken. The declarations
> >>>>>> should be guarded (by _ASSERT_H_DECLS in my system's copy).
> >>>>>
> >>>>> Debian? It uses eglibc which is fork (or clone?) of glibc.
> >>>>>
> >>>>>> If it's widespread we might have to work around this.
> >>>>>
> >>>>> It is in fedora 18 and glibc's git master branch. Why "if"?
> >>>>
> >>>> It's in Fedora 17, too, but I *don't* get a warning.  Suspecting your
> >>>> compiler.  --version?
> >>>
> >>>
> >>> powerpc64-linux-gcc 4.6.3, 4.7.2, 4.8.0, all the same. I'll try to
> >>> track it down tomorrow why it all works when host and target are the
> >>> same (pretty sure this is the cse) but I just do not get it... It is
> >>> just me who sees obvious error in assert.h which is caused by
> >>> -Wno-redundant-decls? Even if you do not hit this now, you will get
> >>> there eventually.
> >>
> >> I don't doubt your gcc+libc is in error.  I just don't want to lose a
> >> useful warning because of that.
> >>
> >> Workaround: configure --disable-werror
> >
> > This workaround does NOT work if pragmas used. "#pragma GCC diagnostic
> > error "-Wredundant-decls"" re-enables warnings as errors.
> 
> Bummer.  Could you try the appended patch?
> 
> diff --git a/configure b/configure
> index 0788e27..41097a2 100755
> --- a/configure
> +++ b/configure
> @@ -3244,8 +3244,10 @@ fi
>  
>  pragma_disable_unused_but_set=no
>  cat > $TMPC << EOF
> +#pragma GCC diagnostic push
>  #pragma GCC diagnostic ignored "-Wunused-but-set-variable"
>  #pragma GCC diagnostic ignored "-Wstrict-prototypes"
> +#pragma GCC diagnostic pop

Breaks gcc < 4.6, which doesn't have push/pop yet.

Kevin

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

[Markus Armbruster]

Kevin Wolf <kwolf@redhat.com> writes:

> Am 16.04.2013 um 11:00 hat Markus Armbruster geschrieben:
>> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>> 
>> > On 04/16/2013 01:55 AM, Markus Armbruster wrote:
>> >> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>> >>
>> >>> On 04/15/2013 10:57 PM, Markus Armbruster wrote:
>> >>>> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>> >>>>
>> >>>>> On 04/15/2013 08:01 PM, Peter Maydell wrote:
>> >>>>>> On 15 April 2013 10:50, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>> >>>>>>> /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]
>> >>>>>>
>> >>>>>> This copy of assert.h seems to be broken. The declarations
>> >>>>>> should be guarded (by _ASSERT_H_DECLS in my system's copy).
>> >>>>>
>> >>>>> Debian? It uses eglibc which is fork (or clone?) of glibc.
>> >>>>>
>> >>>>>> If it's widespread we might have to work around this.
>> >>>>>
>> >>>>> It is in fedora 18 and glibc's git master branch. Why "if"?
>> >>>>
>> >>>> It's in Fedora 17, too, but I *don't* get a warning.  Suspecting your
>> >>>> compiler.  --version?
>> >>>
>> >>>
>> >>> powerpc64-linux-gcc 4.6.3, 4.7.2, 4.8.0, all the same. I'll try to
>> >>> track it down tomorrow why it all works when host and target are the
>> >>> same (pretty sure this is the cse) but I just do not get it... It is
>> >>> just me who sees obvious error in assert.h which is caused by
>> >>> -Wno-redundant-decls? Even if you do not hit this now, you will get
>> >>> there eventually.
>> >>
>> >> I don't doubt your gcc+libc is in error.  I just don't want to lose a
>> >> useful warning because of that.
>> >>
>> >> Workaround: configure --disable-werror
>> >
>> > This workaround does NOT work if pragmas used. "#pragma GCC diagnostic
>> > error "-Wredundant-decls"" re-enables warnings as errors.
>> 
>> Bummer.  Could you try the appended patch?
>> 
>> diff --git a/configure b/configure
>> index 0788e27..41097a2 100755
>> --- a/configure
>> +++ b/configure
>> @@ -3244,8 +3244,10 @@ fi
>>  
>>  pragma_disable_unused_but_set=no
>>  cat > $TMPC << EOF
>> +#pragma GCC diagnostic push
>>  #pragma GCC diagnostic ignored "-Wunused-but-set-variable"
>>  #pragma GCC diagnostic ignored "-Wstrict-prototypes"
>> +#pragma GCC diagnostic pop
>
> Breaks gcc < 4.6, which doesn't have push/pop yet.

The patch to configure makes sure push/pop works before we use it.  When
your gcc is too old, warnings aren't suppressed, and you may have to
--disable-werror.  I prefer that over breaking --disable-werror for
everyone.

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

[Peter Maydell]

On 15 April 2013 03:31, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> 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.

This commit message seems to be missing any mention of
which versions of pixman this change breaks and why
it's OK now to break compiling against them...

> 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 */

...and if it's a correct change it should be removing the
comment as well, since it would no longer apply.

> -#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"
>
> --
> 1.7.10.4

thanks
-- PMM

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

[Alexey Kardashevskiy]

On 04/15/2013 05:18 PM, Peter Maydell wrote:
> On 15 April 2013 03:31, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>> 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.
>
> This commit message seems to be missing any mention of
> which versions of pixman this change breaks and why
> it's OK now to break compiling against them...


The change does not _break_ anything. By default, it will just generate 
warnings, but only if "-Wredundant-decls" is set.

The second removed chunk in the patch is the problem as it:
1) enables -Wredundant-decls even if it was not enabled before;
2) makes -Wredundant-decls an error, not just a warning.

Default assert.h shipped with Fedora Core 18 (pretty recent and popular 
distribution, I would say) cannot compile with -Wredundant-decls as an 
error so we should either avoid -Wredundant-decls or watch where we include 
assert.h very carefully.



>> 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 */
>
> ...and if it's a correct change it should be removing the
> comment as well, since it would no longer apply.
>
>> -#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"
>>
>> --
>> 1.7.10.4
>
> thanks
> -- PMM
>


-- 
Alexey

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

[Peter Maydell]

On 15 April 2013 08:26, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> On 04/15/2013 05:18 PM, Peter Maydell wrote:
>>
>> On 15 April 2013 03:31, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>
>>> 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.
>>
>>
>> This commit message seems to be missing any mention of
>> which versions of pixman this change breaks and why
>> it's OK now to break compiling against them...
>
>
>
> The change does not _break_ anything.

The change breaks the fix that the code was put in to deal with,
ie that pixman-0.16's header files generate warnings, which the
default development qemu will turn into errors with -Werror, which
means we won't compile.

> The second removed chunk in the patch is the problem as it:
> 1) enables -Wredundant-decls even if it was not enabled before;
> 2) makes -Wredundant-decls an error, not just a warning.

This is because not all versions of gcc have the push/pop warning
pragma.

> Default assert.h shipped with Fedora Core 18 (pretty recent and popular
> distribution, I would say) cannot compile with -Wredundant-decls as an error

This appears to be a bug in Fedora.

I guess the correct workaround for that bug is going to be either:

 * wrap assert.h the same way we wrap pixman.h
 * drop -Wredundant-decls :-(

-- PMM

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

[Alexey Kardashevskiy]

On 04/15/2013 05:30 PM, Peter Maydell wrote:
> On 15 April 2013 08:26, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>> On 04/15/2013 05:18 PM, Peter Maydell wrote:
>>>
>>> On 15 April 2013 03:31, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>>
>>>> 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.
>>>
>>>
>>> This commit message seems to be missing any mention of
>>> which versions of pixman this change breaks and why
>>> it's OK now to break compiling against them...
>>
>>
>>
>> The change does not _break_ anything.
>
> The change breaks the fix that the code was put in to deal with,
> ie that pixman-0.16's header files generate warnings, which the
> default development qemu will turn into errors with -Werror, which
> means we won't compile.
>
>> The second removed chunk in the patch is the problem as it:
>> 1) enables -Wredundant-decls even if it was not enabled before;
>> 2) makes -Wredundant-decls an error, not just a warning.
>
> This is because not all versions of gcc have the push/pop warning
> pragma.
>
>> Default assert.h shipped with Fedora Core 18 (pretty recent and popular
>> distribution, I would say) cannot compile with -Wredundant-decls as an error
>
> This appears to be a bug in Fedora.


Rather glibc as it is still in glibc's master branch:
http://sourceware.org/git/?p=glibc.git;a=blob;f=assert/assert.h;h=1bb9b2d50f92da3b17cc0b937ecd6141a87dc196;hb=HEAD

Or am I looking at wrong git?

> I guess the correct workaround for that bug is going to be either:
>
>   * wrap assert.h the same way we wrap pixman.h
>   * drop -Wredundant-decls :-(


The second option seems less ugly.


-- 
Alexey