[Qemu-devel] [PATCH 0/6] fix a few clang warnings

[Qemu-devel] [PATCH 0/6] fix a few clang warnings

[Paolo Bonzini]

All of these should be quite uncontroversial.  The vnc.c change is
not what you'd expect though (could be a real bug, so I leave the
clang warning).

I'll propose the second patch for 0.12 after a short while.

Paolo Bonzini (6):
  remove two dead assignments in target-i386/translate.c
  fix undefined shifts by >32
  exec.c: dead assignments
  vnc.c: warn about ignored option
  usb-linux.c: remove write-only variable
  fix audio_bug related failures

 audio/audio.c           |   44 ++++++++++++++++++++------------------------
 audio/audio_int.h       |    3 ++-
 exec.c                  |    4 ----
 target-i386/translate.c |    2 --
 usb-linux.c             |    2 --
 vl.c                    |    4 ++--
 vnc.c                   |    1 +
 7 files changed, 25 insertions(+), 35 deletions(-)

[Qemu-devel] [PATCH 1/6] remove two dead assignments in target-i386/translate.c

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target-i386/translate.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/target-i386/translate.c b/target-i386/translate.c
index 8078112..a597e80 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -4692,8 +4692,6 @@ static target_ulong disas_insn(DisasContext *s, target_ulong pc_start)
             ot = dflag + OT_WORD;
 
         modrm = ldub_code(s->pc++);
-        mod = (modrm >> 6) & 3;
-        rm = (modrm & 7) | REX_B(s);
         reg = ((modrm >> 3) & 7) | rex_r;
 
         gen_ldst_modrm(s, modrm, ot, OR_TMP0, 0);
-- 
1.6.6

[Qemu-devel] [PATCH 2/6] fix undefined shifts by >32

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 vl.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index 5e8c775..d9f1ccb 100644
--- a/vl.c
+++ b/vl.c
@@ -2373,9 +2373,9 @@ static void numa_add(const char *optarg)
                         fprintf(stderr,
                             "only 63 CPUs in NUMA mode supported.\n");
                     }
-                    value = (1 << (endvalue + 1)) - (1 << value);
+                    value = (2ULL << endvalue) - (1ULL << value);
                 } else {
-                    value = 1 << value;
+                    value = 1ULL << value;
                 }
             }
             node_cpumask[nodenr] = value;
-- 
1.6.6

[Qemu-devel] [PATCH 3/6] exec.c: dead assignments

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 exec.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/exec.c b/exec.c
index 1190591..64109a7 100644
--- a/exec.c
+++ b/exec.c
@@ -2489,17 +2489,13 @@ void *qemu_get_ram_ptr(ram_addr_t addr)
 ram_addr_t qemu_ram_addr_from_host(void *ptr)
 {
     RAMBlock *prev;
-    RAMBlock **prevp;
     RAMBlock *block;
     uint8_t *host = ptr;
 
     prev = NULL;
-    prevp = &ram_blocks;
     block = ram_blocks;
     while (block && (block->host > host
                      || block->host + block->length <= host)) {
-        if (prev)
-          prevp = &prev->next;
         prev = block;
         block = block->next;
     }
-- 
1.6.6

[Qemu-devel] [PATCH 4/6] vnc.c: warn about ignored option

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 vnc.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/vnc.c b/vnc.c
index cc2a26e..9ebee09 100644
--- a/vnc.c
+++ b/vnc.c
@@ -2563,6 +2563,7 @@ int vnc_display_open(DisplayState *ds, const char *display)
             reverse = 1;
         } else if (strncmp(options, "to=", 3) == 0) {
             to_port = atoi(options+3) + 5900;
+            fprintf(stderr, "qemu: warning: to= option for -vnc ignored\n");
 #ifdef CONFIG_VNC_SASL
         } else if (strncmp(options, "sasl", 4) == 0) {
             sasl = 1; /* Require SASL auth */
-- 
1.6.6

Re: [Qemu-devel] [PATCH 4/6] vnc.c: warn about ignored option

[Anthony Liguori]

On 01/26/2010 05:14 PM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> ---
>   vnc.c |    1 +
>   1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/vnc.c b/vnc.c
> index cc2a26e..9ebee09 100644
> --- a/vnc.c
> +++ b/vnc.c
> @@ -2563,6 +2563,7 @@ int vnc_display_open(DisplayState *ds, const char *display)
>               reverse = 1;
>           } else if (strncmp(options, "to=", 3) == 0) {
>               to_port = atoi(options+3) + 5900;
> +            fprintf(stderr, "qemu: warning: to= option for -vnc ignored\n");
>    

If we're ignoring it, why even have the code?  How did clang spot this?

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 4/6] vnc.c: warn about ignored option

[Gerd Hoffmann]

On 01/27/10 03:10, Anthony Liguori wrote:
> On 01/26/2010 05:14 PM, Paolo Bonzini wrote:
>> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
>> ---
>> vnc.c | 1 +
>> 1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/vnc.c b/vnc.c
>> index cc2a26e..9ebee09 100644
>> --- a/vnc.c
>> +++ b/vnc.c
>> @@ -2563,6 +2563,7 @@ int vnc_display_open(DisplayState *ds, const
>> char *display)
>> reverse = 1;
>> } else if (strncmp(options, "to=", 3) == 0) {
>> to_port = atoi(options+3) + 5900;
>> + fprintf(stderr, "qemu: warning: to= option for -vnc ignored\n");
>
> If we're ignoring it, why even have the code? How did clang spot this?

to= isn't ignored.  The to= handling has been moved to qemu-socket.c 
though a while ago.  So clang spotted correctly that to_port variable is 
unused.  The whole 'if (...) {}' is a leftover which can go away now.

cheers,
   Gerd

[Qemu-devel] [PATCH 5/6] usb-linux.c: remove write-only variable

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 usb-linux.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/usb-linux.c b/usb-linux.c
index 1aaa595..ba8facf 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -1007,11 +1007,9 @@ USBDevice *usb_host_device_open(const char *devname)
 {
     struct USBAutoFilter filter;
     USBDevice *dev;
-    USBHostDevice *s;
     char *p;
 
     dev = usb_create(NULL /* FIXME */, "usb-host");
-    s = DO_UPCAST(USBHostDevice, dev, dev);
 
     if (strstr(devname, "auto:")) {
         if (parse_filter(devname, &filter) < 0)
-- 
1.6.6

[Qemu-devel] [PATCH 6/6] fix audio_bug related failures

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 audio/audio.c     |   44 ++++++++++++++++++++------------------------
 audio/audio_int.h |    3 ++-
 2 files changed, 22 insertions(+), 25 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 2a20e5b..7fce46c 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -118,42 +118,38 @@ struct mixeng_volume nominal_volume = {
 static void audio_print_options (const char *prefix,
                                  struct audio_option *opt);
 
-int audio_bug (const char *funcname, int cond)
+void audio_bug_found (const char *funcname)
 {
-    if (cond) {
-        static int shown;
-
-        AUD_log (NULL, "A bug was just triggered in %s\n", funcname);
-        if (!shown) {
-            struct audio_driver *d;
-
-            shown = 1;
-            AUD_log (NULL, "Save all your work and restart without audio\n");
-            AUD_log (NULL, "Please send bug report to av1474@comtv.ru\n");
-            AUD_log (NULL, "I am sorry\n");
-            d = glob_audio_state.drv;
-            if (d) {
-                audio_print_options (d->name, d->options);
-            }
+    static int shown;
+
+    AUD_log (NULL, "A bug was just triggered in %s\n", funcname);
+    if (!shown) {
+        struct audio_driver *d;
+
+        shown = 1;
+        AUD_log (NULL, "Save all your work and restart without audio\n");
+        AUD_log (NULL, "Please send bug report to av1474@comtv.ru\n");
+        AUD_log (NULL, "I am sorry\n");
+        d = glob_audio_state.drv;
+        if (d) {
+            audio_print_options (d->name, d->options);
         }
-        AUD_log (NULL, "Context:\n");
+    }
+    AUD_log (NULL, "Context:\n");
 
 #if defined AUDIO_BREAKPOINT_ON_BUG
 #  if defined HOST_I386
 #    if defined __GNUC__
-        __asm__ ("int3");
+    __asm__ ("int3");
 #    elif defined _MSC_VER
-        _asm _emit 0xcc;
+    _asm _emit 0xcc;
 #    else
-        abort ();
+    abort ();
 #    endif
 #  else
-        abort ();
+    abort ();
 #  endif
 #endif
-    }
-
-    return cond;
 }
 #endif
 
diff --git a/audio/audio_int.h b/audio/audio_int.h
index 06e313f..4244615 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -223,7 +223,8 @@ int  audio_pcm_sw_read (SWVoiceIn *sw, void *buf, int len);
 int audio_pcm_hw_clip_out (HWVoiceOut *hw, void *pcm_buf,
                            int live, int pending);
 
-int audio_bug (const char *funcname, int cond);
+#define audio_bug(funcname, cond) ((cond) ? audio_bug_found (funcname), 1 : 0)
+void audio_bug_found (const char *funcname);
 void *audio_calloc (const char *funcname, int nmemb, size_t size);
 
 void audio_run (const char *msg);
-- 
1.6.6

Re: [Qemu-devel] [PATCH 6/6] fix audio_bug related failures

[Anthony Liguori]

What did clang complain about?  It's not obvious to me.

Regards,

Anthony Liguori

On 01/26/2010 05:14 PM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> ---
>   audio/audio.c     |   44 ++++++++++++++++++++------------------------
>   audio/audio_int.h |    3 ++-
>   2 files changed, 22 insertions(+), 25 deletions(-)
>
> diff --git a/audio/audio.c b/audio/audio.c
> index 2a20e5b..7fce46c 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -118,42 +118,38 @@ struct mixeng_volume nominal_volume = {
>   static void audio_print_options (const char *prefix,
>                                    struct audio_option *opt);
>
> -int audio_bug (const char *funcname, int cond)
> +void audio_bug_found (const char *funcname)
>   {
> -    if (cond) {
> -        static int shown;
> -
> -        AUD_log (NULL, "A bug was just triggered in %s\n", funcname);
> -        if (!shown) {
> -            struct audio_driver *d;
> -
> -            shown = 1;
> -            AUD_log (NULL, "Save all your work and restart without audio\n");
> -            AUD_log (NULL, "Please send bug report to av1474@comtv.ru\n");
> -            AUD_log (NULL, "I am sorry\n");
> -            d = glob_audio_state.drv;
> -            if (d) {
> -                audio_print_options (d->name, d->options);
> -            }
> +    static int shown;
> +
> +    AUD_log (NULL, "A bug was just triggered in %s\n", funcname);
> +    if (!shown) {
> +        struct audio_driver *d;
> +
> +        shown = 1;
> +        AUD_log (NULL, "Save all your work and restart without audio\n");
> +        AUD_log (NULL, "Please send bug report to av1474@comtv.ru\n");
> +        AUD_log (NULL, "I am sorry\n");
> +        d = glob_audio_state.drv;
> +        if (d) {
> +            audio_print_options (d->name, d->options);
>           }
> -        AUD_log (NULL, "Context:\n");
> +    }
> +    AUD_log (NULL, "Context:\n");
>
>   #if defined AUDIO_BREAKPOINT_ON_BUG
>   #  if defined HOST_I386
>   #    if defined __GNUC__
> -        __asm__ ("int3");
> +    __asm__ ("int3");
>   #    elif defined _MSC_VER
> -        _asm _emit 0xcc;
> +    _asm _emit 0xcc;
>   #    else
> -        abort ();
> +    abort ();
>   #    endif
>   #  else
> -        abort ();
> +    abort ();
>   #  endif
>   #endif
> -    }
> -
> -    return cond;
>   }
>   #endif
>
> diff --git a/audio/audio_int.h b/audio/audio_int.h
> index 06e313f..4244615 100644
> --- a/audio/audio_int.h
> +++ b/audio/audio_int.h
> @@ -223,7 +223,8 @@ int  audio_pcm_sw_read (SWVoiceIn *sw, void *buf, int len);
>   int audio_pcm_hw_clip_out (HWVoiceOut *hw, void *pcm_buf,
>                              int live, int pending);
>
> -int audio_bug (const char *funcname, int cond);
> +#define audio_bug(funcname, cond) ((cond) ? audio_bug_found (funcname), 1 : 0)
> +void audio_bug_found (const char *funcname);
>   void *audio_calloc (const char *funcname, int nmemb, size_t size);
>
>   void audio_run (const char *msg);
>    

[Qemu-devel] Re: [PATCH 6/6] fix audio_bug related failures

[Paolo Bonzini]

On 01/27/2010 03:10 AM, Anthony Liguori wrote:
> What did clang complain about?  It's not obvious to me.

It doesn't see that audio_bug returns cond, and gives quite a few false 
positive in its callers.

Paolo

Re: [Qemu-devel] Re: [PATCH 6/6] fix audio_bug related failures

[Anthony Liguori]

On 01/27/2010 05:56 AM, Paolo Bonzini wrote:
> On 01/27/2010 03:10 AM, Anthony Liguori wrote:
>> What did clang complain about?  It's not obvious to me.
>
> It doesn't see that audio_bug returns cond, and gives quite a few 
> false positive in its callers.

Ah, this is a clang issue.  I'll have to defer to malc on this one.

Regards,

Anthony Liguori

> Paolo
>
>
>
>

Re: [Qemu-devel] Re: [PATCH 6/6] fix audio_bug related failures

[malc]

On Wed, 27 Jan 2010, Anthony Liguori wrote:

> On 01/27/2010 05:56 AM, Paolo Bonzini wrote:
> > On 01/27/2010 03:10 AM, Anthony Liguori wrote:
> > > What did clang complain about?  It's not obvious to me.
> > 
> > It doesn't see that audio_bug returns cond, and gives quite a few false
> > positive in its callers.
> 
> Ah, this is a clang issue.  I'll have to defer to malc on this one.
> 

I'm against it.

-- 
mailto:av1474@comtv.ru

[Qemu-devel] Re: [PATCH 6/6] fix audio_bug related failures

[Paolo Bonzini]

On 01/31/2010 04:08 AM, malc wrote:
> On Wed, 27 Jan 2010, Anthony Liguori wrote:
>
>> On 01/27/2010 05:56 AM, Paolo Bonzini wrote:
>>> On 01/27/2010 03:10 AM, Anthony Liguori wrote:
>>>> What did clang complain about?  It's not obvious to me.
>>>
>>> It doesn't see that audio_bug returns cond, and gives quite a few false
>>> positive in its callers.
>>
>> Ah, this is a clang issue.  I'll have to defer to malc on this one.
>
> I'm against it.

Ok, I'll withdraw this patch.  I already posted v2.

Paolo

Re: [Qemu-devel] [PATCH 0/6] fix a few clang warnings

[Anthony Liguori]

On 01/26/2010 05:14 PM, Paolo Bonzini wrote:
> All of these should be quite uncontroversial.  The vnc.c change is
> not what you'd expect though (could be a real bug, so I leave the
> clang warning).
>    

I'm scratching my head quite a bit on these.  Can you resend the series 
with commit messages that actually explain what the problem clang is 
reporting and why it fixes it.

The vnc one is just whacky.

Regards,

Anthony Liguori