Re: [Qemu-devel] [PATCH] clang: Disable warning about expansion to 'defined'

2016-08-09 Thread Pranith Kumar 
On Tue, Aug 9, 2016 at 4:17 AM, Peter Maydell  wrote:
> #if !defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06)
> #define SPICE_NEEDS_SET_MM_TIME 1
> #else
> #define SPICE_NEEDS_SET_MM_TIME 0
> #endif
>

This looks better. I will fix up a patch.

-- 
Pranith

Re: [Qemu-devel] [PATCH] clang: Disable warning about expansion to 'defined'

2016-08-09 Thread Paolo Bonzini 


On 09/08/2016 10:17, Peter Maydell wrote:
> On 9 August 2016 at 09:15, Paolo Bonzini  wrote:
>>
>>
>> On 09/08/2016 10:09, Peter Maydell wrote:
>>> Is this the only thing in our code which provokes the warning?
>>> If so, why don't we just fix it to not be undefined behaviour?
>>
>> Indeed that was my first suggestion yesterday.  However, we use -Wundef
>> (a good idea, I think), and fixing it would cause the -Wundef warning to
>> appear instead.
> 
> Why would
> #if !defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06)
> #define SPICE_NEEDS_SET_MM_TIME 1
> #else
> #define SPICE_NEEDS_SET_MM_TIME 0
> #endif
> 
> provoke a Wundef warning?

Oh, that would work indeed.  I was thinking of simplifying the
definition of SPICE_NEEDS_SET_MM_TIME like this:

#define SPICE_NEEDS_SET_MM_TIME (SPICE_SERVER_VERSION < 0xc06)

If we do this, it would make sense to add -Wexpansion-to-defined to GCC
and enable it (though I still believe it's wrong to add it to -Wall).

Thanks,

Paolo

Re: [Qemu-devel] [PATCH] clang: Disable warning about expansion to 'defined'

2016-08-09 Thread Peter Maydell 
On 9 August 2016 at 09:15, Paolo Bonzini  wrote:
>
>
> On 09/08/2016 10:09, Peter Maydell wrote:
>> Is this the only thing in our code which provokes the warning?
>> If so, why don't we just fix it to not be undefined behaviour?
>
> Indeed that was my first suggestion yesterday.  However, we use -Wundef
> (a good idea, I think), and fixing it would cause the -Wundef warning to
> appear instead.

Why would
#if !defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06)
#define SPICE_NEEDS_SET_MM_TIME 1
#else
#define SPICE_NEEDS_SET_MM_TIME 0
#endif

provoke a Wundef warning?

thanks
-- PMM

Re: [Qemu-devel] [PATCH] clang: Disable warning about expansion to 'defined'

2016-08-09 Thread Paolo Bonzini 


On 09/08/2016 10:09, Peter Maydell wrote:
> On 9 August 2016 at 03:35, Pranith Kumar  wrote:
>> Clang produces the following warning. The warning is detailed here:
>> https://reviews.llvm.org/D15866. Disable the warning.
>>
>> /home/pranith/devops/code/qemu/hw/display/qxl.c:507:5: warning: macro 
>> expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
>> #if SPICE_NEEDS_SET_MM_TIME
>> ^
>> /home/pranith/devops/code/qemu/include/ui/qemu-spice.h:46:5: note: expanded 
>> from macro 'SPICE_NEEDS_SET_MM_TIME'
>>   (!defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06))
>> ^
>> /home/pranith/devops/code/qemu/hw/display/qxl.c:1074:5: warning: macro 
>> expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
>> #if SPICE_NEEDS_SET_MM_TIME
>> ^
>> /home/pranith/devops/code/qemu/include/ui/qemu-spice.h:46:5: note: expanded 
>> from macro 'SPICE_NEEDS_SET_MM_TIME'
>>   (!defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06))
> 
> Is this the only thing in our code which provokes the warning?
> If so, why don't we just fix it to not be undefined behaviour?

Indeed that was my first suggestion yesterday.  However, we use -Wundef
(a good idea, I think), and fixing it would cause the -Wundef warning to
appear instead.

Paolo

Re: [Qemu-devel] [PATCH] clang: Disable warning about expansion to 'defined'

2016-08-09 Thread Daniel P. Berrange 
On Tue, Aug 09, 2016 at 09:09:13AM +0100, Peter Maydell wrote:
> On 9 August 2016 at 03:35, Pranith Kumar  wrote:
> > Clang produces the following warning. The warning is detailed here:
> > https://reviews.llvm.org/D15866. Disable the warning.
> >
> > /home/pranith/devops/code/qemu/hw/display/qxl.c:507:5: warning: macro 
> > expansion producing 'defined' has undefined behavior 
> > [-Wexpansion-to-defined]
> > #if SPICE_NEEDS_SET_MM_TIME
> > ^
> > /home/pranith/devops/code/qemu/include/ui/qemu-spice.h:46:5: note: expanded 
> > from macro 'SPICE_NEEDS_SET_MM_TIME'
> >   (!defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06))
> > ^
> > /home/pranith/devops/code/qemu/hw/display/qxl.c:1074:5: warning: macro 
> > expansion producing 'defined' has undefined behavior 
> > [-Wexpansion-to-defined]
> > #if SPICE_NEEDS_SET_MM_TIME
> > ^
> > /home/pranith/devops/code/qemu/include/ui/qemu-spice.h:46:5: note: expanded 
> > from macro 'SPICE_NEEDS_SET_MM_TIME'
> >   (!defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06))
> 
> Is this the only thing in our code which provokes the warning?
> If so, why don't we just fix it to not be undefined behaviour?

Indeed, it looks like it could be fixed by changing include/ui/qemu-spice.h


#define SPICE_NEEDS_SET_MM_TIME   \
  (!defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06))


to be

#if !defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06)
#define SPICE_NEEDS_SET_MM_TIME
#endif


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH] clang: Disable warning about expansion to 'defined'

2016-08-09 Thread Peter Maydell 
On 9 August 2016 at 03:35, Pranith Kumar  wrote:
> Clang produces the following warning. The warning is detailed here:
> https://reviews.llvm.org/D15866. Disable the warning.
>
> /home/pranith/devops/code/qemu/hw/display/qxl.c:507:5: warning: macro 
> expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
> #if SPICE_NEEDS_SET_MM_TIME
> ^
> /home/pranith/devops/code/qemu/include/ui/qemu-spice.h:46:5: note: expanded 
> from macro 'SPICE_NEEDS_SET_MM_TIME'
>   (!defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06))
> ^
> /home/pranith/devops/code/qemu/hw/display/qxl.c:1074:5: warning: macro 
> expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
> #if SPICE_NEEDS_SET_MM_TIME
> ^
> /home/pranith/devops/code/qemu/include/ui/qemu-spice.h:46:5: note: expanded 
> from macro 'SPICE_NEEDS_SET_MM_TIME'
>   (!defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06))

Is this the only thing in our code which provokes the warning?
If so, why don't we just fix it to not be undefined behaviour?

thanks
-- PMM

Re: [Qemu-devel] [PATCH] clang: Disable warning about expansion to 'defined'

2016-08-09 Thread Paolo Bonzini 


On 09/08/2016 04:35, Pranith Kumar wrote:
> Clang produces the following warning. The warning is detailed here:
> https://reviews.llvm.org/D15866. Disable the warning.
> 
> /home/pranith/devops/code/qemu/hw/display/qxl.c:507:5: warning: macro 
> expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
> #if SPICE_NEEDS_SET_MM_TIME
> ^
> /home/pranith/devops/code/qemu/include/ui/qemu-spice.h:46:5: note: expanded 
> from macro 'SPICE_NEEDS_SET_MM_TIME'
>   (!defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06))
> ^
> /home/pranith/devops/code/qemu/hw/display/qxl.c:1074:5: warning: macro 
> expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
> #if SPICE_NEEDS_SET_MM_TIME
> ^
> /home/pranith/devops/code/qemu/include/ui/qemu-spice.h:46:5: note: expanded 
> from macro 'SPICE_NEEDS_SET_MM_TIME'
>   (!defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06))
> 
> Signed-off-by: Pranith Kumar 
> ---
>  configure | 13 +
>  1 file changed, 13 insertions(+)
> 
> diff --git a/configure b/configure
> index f57fcc6..a1a6e3f 100755
> --- a/configure
> +++ b/configure
> @@ -3025,6 +3025,19 @@ if ! compile_prog "$glib_cflags -Werror" "$glib_libs" 
> ; then
>  fi
>  fi
>  
> +# Silence clang warnings about expansion to 'defined' in macro
> +cat > $TMPC << EOF
> +#define TEST (defined(DUMMY))
> +#if TEST
> +int main(void) { return 0; }
> +#endif
> +EOF
> +if ! compile_prog "-Werror" ; then
> +if cc_has_warning_flag "-Wno-expansion-to-defined"; then
> + CFLAGS="-Wno-expansion-to-defined $CFLAGS"
> +fi
> +fi
> +
>  ##
>  # SHA command probe for modules
>  if test "$modules" = yes; then
> 

Can you just add it to gcc_flags instead?

Thanks,

Paolo

[Qemu-devel] [PATCH] clang: Disable warning about expansion to 'defined'

2016-08-08 Thread Pranith Kumar 
Clang produces the following warning. The warning is detailed here:
https://reviews.llvm.org/D15866. Disable the warning.

/home/pranith/devops/code/qemu/hw/display/qxl.c:507:5: warning: macro expansion 
producing 'defined' has undefined behavior [-Wexpansion-to-defined]
#if SPICE_NEEDS_SET_MM_TIME
^
/home/pranith/devops/code/qemu/include/ui/qemu-spice.h:46:5: note: expanded 
from macro 'SPICE_NEEDS_SET_MM_TIME'
  (!defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06))
^
/home/pranith/devops/code/qemu/hw/display/qxl.c:1074:5: warning: macro 
expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
#if SPICE_NEEDS_SET_MM_TIME
^
/home/pranith/devops/code/qemu/include/ui/qemu-spice.h:46:5: note: expanded 
from macro 'SPICE_NEEDS_SET_MM_TIME'
  (!defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06))

Signed-off-by: Pranith Kumar 
---
 configure | 13 +
 1 file changed, 13 insertions(+)

diff --git a/configure b/configure
index f57fcc6..a1a6e3f 100755
--- a/configure
+++ b/configure
@@ -3025,6 +3025,19 @@ if ! compile_prog "$glib_cflags -Werror" "$glib_libs" ; 
then
 fi
 fi
 
+# Silence clang warnings about expansion to 'defined' in macro
+cat > $TMPC << EOF
+#define TEST (defined(DUMMY))
+#if TEST
+int main(void) { return 0; }
+#endif
+EOF
+if ! compile_prog "-Werror" ; then
+if cc_has_warning_flag "-Wno-expansion-to-defined"; then
+   CFLAGS="-Wno-expansion-to-defined $CFLAGS"
+fi
+fi
+
 ##
 # SHA command probe for modules
 if test "$modules" = yes; then
-- 
2.9.2