[Qemu-devel] [PATCH] vnc: Fix packed boolean struct members

[Qemu-devel] [PATCH] vnc: Fix packed boolean struct members

[Stefan Weil]

This patch fixes warnings reported by splint:

For variables which are packed in a single bit, a signed data type
like 'int' does not make much sense.

There is no obvious reason why the two values should be packed,
so I removed the packing and changed the data type to bool
because both are used as boolean values.

Cc: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 ui/vnc-auth-sasl.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ui/vnc-auth-sasl.h b/ui/vnc-auth-sasl.h
index fd9b18a..ee243a9 100644
--- a/ui/vnc-auth-sasl.h
+++ b/ui/vnc-auth-sasl.h
@@ -37,9 +37,9 @@ typedef struct VncDisplaySASL VncDisplaySASL;
 struct VncStateSASL {
     sasl_conn_t *conn;
     /* If we want to negotiate an SSF layer with client */
-    int wantSSF :1;
+    bool wantSSF;
     /* If we are now running the SSF layer */
-    int runSSF :1;
+    bool runSSF;
     /*
      * If this is non-zero, then wait for that many bytes
      * to be written plain, before switching to SSF encoding
-- 
1.7.9

Re: [Qemu-devel] [PATCH] vnc: Fix packed boolean struct members

[Stefan Hajnoczi]

On Sat, Feb 25, 2012 at 02:35:47PM +0100, Stefan Weil wrote:
> This patch fixes warnings reported by splint:
> 
> For variables which are packed in a single bit, a signed data type
> like 'int' does not make much sense.
> 
> There is no obvious reason why the two values should be packed,
> so I removed the packing and changed the data type to bool
> because both are used as boolean values.
> 
> Cc: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>  ui/vnc-auth-sasl.h |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)

Thanks, applied to the trivial patches tree:
https://github.com/stefanha/qemu/commits/trivial-patches

Stefan

Re: [Qemu-devel] [PATCH] vnc: Fix packed boolean struct members

[Stefan Hajnoczi]

On Sat, Feb 25, 2012 at 1:35 PM, Stefan Weil <sw@weilnetz.de> wrote:
> This patch fixes warnings reported by splint:
>
> For variables which are packed in a single bit, a signed data type
> like 'int' does not make much sense.
>
> There is no obvious reason why the two values should be packed,
> so I removed the packing and changed the data type to bool
> because both are used as boolean values.
>
> Cc: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>  ui/vnc-auth-sasl.h |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/ui/vnc-auth-sasl.h b/ui/vnc-auth-sasl.h
> index fd9b18a..ee243a9 100644
> --- a/ui/vnc-auth-sasl.h
> +++ b/ui/vnc-auth-sasl.h
> @@ -37,9 +37,9 @@ typedef struct VncDisplaySASL VncDisplaySASL;
>  struct VncStateSASL {
>     sasl_conn_t *conn;
>     /* If we want to negotiate an SSF layer with client */
> -    int wantSSF :1;
> +    bool wantSSF;
>     /* If we are now running the SSF layer */
> -    int runSSF :1;
> +    bool runSSF;

I had to drop this patch from the trivial patches tree, it tickles a
new gcc warning.  Please resend with the necessary change.

I cannot reproduce it on my build host here with gcc Debian 4.6.2-12
but Anthony reports the following Ubuntu/Linaro 4.5.2-8ubuntu4:

  CC    ui/vnc-auth-sasl.o
cc1: warnings being treated as errors
/home/anthony/git/qemu/ui/vnc-auth-sasl.c: In function
‘vnc_sasl_client_cleanup’:
/home/anthony/git/qemu/ui/vnc-auth-sasl.c:34:9: error: suggest
parentheses around assignment used as truth value
make: *** [ui/vnc-auth-sasl.o] Error 1

Re: [Qemu-devel] [PATCH] vnc: Fix packed boolean struct members

[Paolo Bonzini]

Il 08/03/2012 16:26, Stefan Hajnoczi ha scritto:
> I had to drop this patch from the trivial patches tree, it tickles a
> new gcc warning.  Please resend with the necessary change.
> 
> I cannot reproduce it on my build host here with gcc Debian 4.6.2-12
> but Anthony reports the following Ubuntu/Linaro 4.5.2-8ubuntu4:
> 
>   CC    ui/vnc-auth-sasl.o
> cc1: warnings being treated as errors
> /home/anthony/git/qemu/ui/vnc-auth-sasl.c: In function
> ‘vnc_sasl_client_cleanup’:
> /home/anthony/git/qemu/ui/vnc-auth-sasl.c:34:9: error: suggest
> parentheses around assignment used as truth value
> make: *** [ui/vnc-auth-sasl.o] Error 1

Looks like a GCC bug where

   x = y = 0;

is converted to

   x = (y = 0) != 0;

and the magic that does this inside the compiler causes the warning.

We should add a configure check to only add -Werror on compilers newer
than X (for example X could leave out GCC 4.5), so that this patch can
be reapplied.

Paolo

Re: [Qemu-devel] [PATCH] vnc: Fix packed boolean struct members

[Anthony Liguori]

On 03/08/2012 10:08 AM, Paolo Bonzini wrote:
> Il 08/03/2012 16:26, Stefan Hajnoczi ha scritto:
>> I had to drop this patch from the trivial patches tree, it tickles a
>> new gcc warning.  Please resend with the necessary change.
>>
>> I cannot reproduce it on my build host here with gcc Debian 4.6.2-12
>> but Anthony reports the following Ubuntu/Linaro 4.5.2-8ubuntu4:
>>
>>    CC    ui/vnc-auth-sasl.o
>> cc1: warnings being treated as errors
>> /home/anthony/git/qemu/ui/vnc-auth-sasl.c: In function
>> ‘vnc_sasl_client_cleanup’:
>> /home/anthony/git/qemu/ui/vnc-auth-sasl.c:34:9: error: suggest
>> parentheses around assignment used as truth value
>> make: *** [ui/vnc-auth-sasl.o] Error 1
>
> Looks like a GCC bug where
>
>     x = y = 0;
>
> is converted to
>
>     x = (y = 0) != 0;

Curious, why convert like this?  What does this optimization do?

Regards,

Anthony Liguori

>
> and the magic that does this inside the compiler causes the warning.
>
> We should add a configure check to only add -Werror on compilers newer
> than X (for example X could leave out GCC 4.5), so that this patch can
> be reapplied.
>
> Paolo
>

Re: [Qemu-devel] [PATCH] vnc: Fix packed boolean struct members

[Paolo Bonzini]

Il 08/03/2012 17:25, Anthony Liguori ha scritto:
>>
>> Looks like a GCC bug where
>>
>>     x = y = 0;
>>
>> is converted to
>>
>>     x = (y = 0) != 0;
> 
> Curious, why convert like this?  What does this optimization do?

Nothing, it's just that if "y = 0" is an int (for some reason, I didn't
check if it's implicit promotion or just that y is already an int), then
it has to be converted back to a truth value before assigning to bool.
If you do

  bool x;
  int y;

  x = y = 2;

x is actually set to 1.

Paolo