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

2013-04-16 Thread Markus Armbruster
Kevin Wolf  writes:

> Am 16.04.2013 um 11:00 hat Markus Armbruster geschrieben:
>> Alexey Kardashevskiy  writes:
>> 
>> > On 04/16/2013 01:55 AM, Markus Armbruster wrote:
>> >> Alexey Kardashevskiy  writes:
>> >>
>> >>> On 04/15/2013 10:57 PM, Markus Armbruster wrote:
>>  Alexey Kardashevskiy  writes:
>> 
>> > On 04/15/2013 08:01 PM, Peter Maydell wrote:
>> >> On 15 April 2013 10:50, Alexey Kardashevskiy  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

2013-04-16 Thread Markus Armbruster
Paolo Bonzini  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

2013-04-16 Thread Kevin Wolf
Am 16.04.2013 um 11:00 hat Markus Armbruster geschrieben:
> Alexey Kardashevskiy  writes:
> 
> > On 04/16/2013 01:55 AM, Markus Armbruster wrote:
> >> Alexey Kardashevskiy  writes:
> >>
> >>> On 04/15/2013 10:57 PM, Markus Armbruster wrote:
>  Alexey Kardashevskiy  writes:
> 
> > On 04/15/2013 08:01 PM, Peter Maydell wrote:
> >> On 15 April 2013 10:50, Alexey Kardashevskiy  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

2013-04-16 Thread 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

2013-04-16 Thread Alexey Kardashevskiy

On 04/16/2013 07:00 PM, Markus Armbruster wrote:

Alexey Kardashevskiy  writes:


On 04/16/2013 01:55 AM, Markus Armbruster wrote:

Alexey Kardashevskiy  writes:


On 04/15/2013 10:57 PM, Markus Armbruster wrote:

Alexey Kardashevskiy  writes:


On 04/15/2013 08:01 PM, Peter Maydell wrote:

On 15 April 2013 10:50, Alexey Kardashevskiy  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

2013-04-16 Thread 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

2013-04-16 Thread Markus Armbruster
Paolo Bonzini  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 
>  #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

2013-04-16 Thread 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 
 #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

2013-04-16 Thread Markus Armbruster
Alexey Kardashevskiy  writes:

> On 04/16/2013 01:55 AM, Markus Armbruster wrote:
>> Alexey Kardashevskiy  writes:
>>
>>> On 04/15/2013 10:57 PM, Markus Armbruster wrote:
 Alexey Kardashevskiy  writes:

> On 04/15/2013 08:01 PM, Peter Maydell wrote:
>> On 15 April 2013 10:50, Alexey Kardashevskiy  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 
 #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 
 #ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE
-#pragma GCC diagnostic error "-Wstrict-prototypes"
+#pragma GCC diagnostic pop
 #endif
 
 



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

2013-04-16 Thread Markus Armbruster
Gcc issue, perhaps Paolo [cc'ed] got an idea.

Alexey Kardashevskiy  writes:

> On 04/16/2013 08:48 AM, Alexey Kardashevskiy wrote:
>> On 04/16/2013 01:55 AM, Markus Armbruster wrote:
>>> Alexey Kardashevskiy  writes:
>>>
 On 04/15/2013 10:57 PM, Markus Armbruster wrote:
> Alexey Kardashevskiy  writes:
>
>> On 04/15/2013 08:01 PM, Peter Maydell wrote:
>>> On 15 April 2013 10:50, Alexey Kardashevskiy  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 
> #include 
> #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

2013-04-15 Thread Alexey Kardashevskiy

On 04/16/2013 08:48 AM, Alexey Kardashevskiy wrote:

On 04/16/2013 01:55 AM, Markus Armbruster wrote:

Alexey Kardashevskiy  writes:


On 04/15/2013 10:57 PM, Markus Armbruster wrote:

Alexey Kardashevskiy  writes:


On 04/15/2013 08:01 PM, Peter Maydell wrote:

On 15 April 2013 10:50, Alexey Kardashevskiy  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 
#include 
#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

2013-04-15 Thread Alexey Kardashevskiy

On 04/16/2013 01:55 AM, Markus Armbruster wrote:

Alexey Kardashevskiy  writes:


On 04/15/2013 10:57 PM, Markus Armbruster wrote:

Alexey Kardashevskiy  writes:


On 04/15/2013 08:01 PM, Peter Maydell wrote:

On 15 April 2013 10:50, Alexey Kardashevskiy  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

2013-04-15 Thread Markus Armbruster
Alexey Kardashevskiy  writes:

> On 04/15/2013 10:57 PM, Markus Armbruster wrote:
>> Alexey Kardashevskiy  writes:
>>
>>> On 04/15/2013 08:01 PM, Peter Maydell wrote:
 On 15 April 2013 10:50, Alexey Kardashevskiy  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

2013-04-15 Thread Alexey Kardashevskiy

On 04/15/2013 10:57 PM, Markus Armbruster wrote:

Alexey Kardashevskiy  writes:


On 04/15/2013 08:01 PM, Peter Maydell wrote:

On 15 April 2013 10:50, Alexey Kardashevskiy  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

2013-04-15 Thread Markus Armbruster
Alexey Kardashevskiy  writes:

> On 04/15/2013 08:01 PM, Peter Maydell wrote:
>> On 15 April 2013 10:50, Alexey Kardashevskiy  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

2013-04-15 Thread Alexey Kardashevskiy

On 04/15/2013 09:14 PM, Peter Maydell wrote:

On 15 April 2013 12:08, Alexey Kardashevskiy  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

2013-04-15 Thread Peter Maydell
On 15 April 2013 12:08, Alexey Kardashevskiy  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

2013-04-15 Thread Alexey Kardashevskiy

On 04/15/2013 09:00 PM, Peter Maydell wrote:

On 15 April 2013 11:52, Alexey Kardashevskiy  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

2013-04-15 Thread Alexey Kardashevskiy

On 04/15/2013 08:01 PM, Peter Maydell wrote:

On 15 April 2013 10:50, Alexey Kardashevskiy  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

2013-04-15 Thread Peter Maydell
On 15 April 2013 11:52, Alexey Kardashevskiy  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

2013-04-15 Thread Peter Maydell
On 15 April 2013 11:01, Peter Maydell  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

2013-04-15 Thread Peter Maydell
On 15 April 2013 10:50, Alexey Kardashevskiy  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

2013-04-15 Thread Alexey Kardashevskiy

On 04/15/2013 06:23 PM, Markus Armbruster wrote:

Alexey Kardashevskiy  writes:


On 04/15/2013 05:08 PM, Markus Armbruster wrote:

Alexey Kardashevskiy  writes:


The assert.h header file from Fedora18 does not have #ifdef-#endif
brackets around __assert function so it cannot compile with
the -Wredundant-decls switch on.

Some Linux distributions (such as Debian Wheezy) still do have those
brackets arounb __assert 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 
---
   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 
-#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):


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

2013-04-15 Thread Markus Armbruster
Alexey Kardashevskiy  writes:

> On 04/15/2013 05:08 PM, Markus Armbruster wrote:
>> Alexey Kardashevskiy  writes:
>>
>>> The assert.h header file from Fedora18 does not have #ifdef-#endif
>>> brackets around __assert function so it cannot compile with
>>> the -Wredundant-decls switch on.
>>>
>>> Some Linux distributions (such as Debian Wheezy) still do have those
>>> brackets arounb __assert 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 
>>> ---
>>>   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 
>>> -#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

2013-04-15 Thread Alexey Kardashevskiy

On 04/15/2013 05:30 PM, Peter Maydell wrote:

On 15 April 2013 08:26, Alexey Kardashevskiy  wrote:

On 04/15/2013 05:18 PM, Peter Maydell wrote:


On 15 April 2013 03:31, Alexey Kardashevskiy  wrote:


The assert.h header file from Fedora18 does not have #ifdef-#endif
brackets around __assert function so it cannot compile with
the -Wredundant-decls switch on.

Some Linux distributions (such as Debian Wheezy) still do have those
brackets arounb __assert 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



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

2013-04-15 Thread Peter Maydell
On 15 April 2013 08:26, Alexey Kardashevskiy  wrote:
> On 04/15/2013 05:18 PM, Peter Maydell wrote:
>>
>> On 15 April 2013 03:31, Alexey Kardashevskiy  wrote:
>>>
>>> The assert.h header file from Fedora18 does not have #ifdef-#endif
>>> brackets around __assert function so it cannot compile with
>>> the -Wredundant-decls switch on.
>>>
>>> Some Linux distributions (such as Debian Wheezy) still do have those
>>> brackets arounb __assert 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

2013-04-15 Thread Alexey Kardashevskiy

On 04/15/2013 05:18 PM, Peter Maydell wrote:

On 15 April 2013 03:31, Alexey Kardashevskiy  wrote:

The assert.h header file from Fedora18 does not have #ifdef-#endif
brackets around __assert function so it cannot compile with
the -Wredundant-decls switch on.

Some Linux distributions (such as Debian Wheezy) still do have those
brackets arounb __assert 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 
---
  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 
-#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

2013-04-15 Thread Peter Maydell
On 15 April 2013 03:31, Alexey Kardashevskiy  wrote:
> The assert.h header file from Fedora18 does not have #ifdef-#endif
> brackets around __assert function so it cannot compile with
> the -Wredundant-decls switch on.
>
> Some Linux distributions (such as Debian Wheezy) still do have those
> brackets arounb __assert 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 
> ---
>  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 
> -#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

2013-04-15 Thread Alexey Kardashevskiy

On 04/15/2013 05:08 PM, Markus Armbruster wrote:

Alexey Kardashevskiy  writes:


The assert.h header file from Fedora18 does not have #ifdef-#endif
brackets around __assert function so it cannot compile with
the -Wredundant-decls switch on.

Some Linux distributions (such as Debian Wheezy) still do have those
brackets arounb __assert 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 
---
  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 
-#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

2013-04-15 Thread Markus Armbruster
Alexey Kardashevskiy  writes:

> The assert.h header file from Fedora18 does not have #ifdef-#endif
> brackets around __assert function so it cannot compile with
> the -Wredundant-decls switch on.
>
> Some Linux distributions (such as Debian Wheezy) still do have those
> brackets arounb __assert 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 
> ---
>  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 
> -#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 :)



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

2013-04-14 Thread Alexey Kardashevskiy
The assert.h header file from Fedora18 does not have #ifdef-#endif
brackets around __assert function so it cannot compile with
the -Wredundant-decls switch on.

Some Linux distributions (such as Debian Wheezy) still do have those
brackets arounb __assert 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 
---
 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 
-#ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE
-#pragma GCC diagnostic error "-Wredundant-decls"
-#endif
 
 #include "qemu/typedefs.h"
 
-- 
1.7.10.4