diff mbox

spice: Fix warning reg. 'defined' in macro

Message ID 20160807012545.7145-1-bobby.prani@gmail.com
State New
Headers show

Commit Message

Pranith Kumar Aug. 7, 2016, 1:25 a.m. UTC
Clang produces the following warning. The warning is detailed here:
https://reviews.llvm.org/D15866

/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 <bobby.prani@gmail.com>
---
 include/ui/qemu-spice.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Paolo Bonzini Aug. 8, 2016, 9:08 a.m. UTC | #1
On 07/08/2016 03:25, Pranith Kumar wrote:
> Clang produces the following warning. The warning is detailed here:
> https://reviews.llvm.org/D15866
> 
> /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 <bobby.prani@gmail.com>
> ---
>  include/ui/qemu-spice.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h
> index edad5e7..3979a1e 100644
> --- a/include/ui/qemu-spice.h
> +++ b/include/ui/qemu-spice.h
> @@ -42,8 +42,7 @@ int qemu_spice_set_pw_expire(time_t expires);
>  int qemu_spice_migrate_info(const char *hostname, int port, int tls_port,
>                              const char *subject);
>  
> -#define SPICE_NEEDS_SET_MM_TIME                                       \
> -  (!defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06))
> +#define SPICE_NEEDS_SET_MM_TIME (SPICE_SERVER_VERSION < 0xc06)
>  
>  #if SPICE_SERVER_VERSION >= 0x000c02
>  void qemu_spice_register_ports(void);
> 

If SPICE_SERVER_VERSION is not defined, the preprocessor will give it
value 0 so there is no semantic change.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo
Paolo Bonzini Aug. 8, 2016, 9:10 a.m. UTC | #2
On 07/08/2016 03:25, Pranith Kumar wrote:
> Clang produces the following warning. The warning is detailed here:
> https://reviews.llvm.org/D15866
> 
> /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 <bobby.prani@gmail.com>
> ---
>  include/ui/qemu-spice.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h
> index edad5e7..3979a1e 100644
> --- a/include/ui/qemu-spice.h
> +++ b/include/ui/qemu-spice.h
> @@ -42,8 +42,7 @@ int qemu_spice_set_pw_expire(time_t expires);
>  int qemu_spice_migrate_info(const char *hostname, int port, int tls_port,
>                              const char *subject);
>  
> -#define SPICE_NEEDS_SET_MM_TIME                                       \
> -  (!defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06))
> +#define SPICE_NEEDS_SET_MM_TIME (SPICE_SERVER_VERSION < 0xc06)
>  
>  #if SPICE_SERVER_VERSION >= 0x000c02
>  void qemu_spice_register_ports(void);
> 

Hmm, no, the patch is not okay because we use -Wundef.  The GCC manual says:

   If the 'defined' operator appears as a result of a macro expansion,
   the C standard says the behavior is undefined.  GNU cpp treats it as
   a genuine 'defined' operator and evaluates it normally.  It will warn
   wherever your code uses this feature if you use the command-line
   option '-pedantic', since other compilers may handle it differently.

So this is another instance of clang putting warnings in the wrong
category.  Please add -Wno-expansion-to-defined instead.

Paolo
diff mbox

Patch

diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h
index edad5e7..3979a1e 100644
--- a/include/ui/qemu-spice.h
+++ b/include/ui/qemu-spice.h
@@ -42,8 +42,7 @@  int qemu_spice_set_pw_expire(time_t expires);
 int qemu_spice_migrate_info(const char *hostname, int port, int tls_port,
                             const char *subject);
 
-#define SPICE_NEEDS_SET_MM_TIME                                       \
-  (!defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06))
+#define SPICE_NEEDS_SET_MM_TIME (SPICE_SERVER_VERSION < 0xc06)
 
 #if SPICE_SERVER_VERSION >= 0x000c02
 void qemu_spice_register_ports(void);