Patchwork [for,1.4] target-s390x: Fix wrong comparison in interrupt handling

login
register
mail settings
Submitter Stefan Weil
Date Feb. 3, 2013, 8:33 p.m.
Message ID <1359923596-26955-1-git-send-email-sw@weilnetz.de>
Download mbox | patch
Permalink /patch/217803/
State Accepted
Headers show

Comments

Stefan Weil - Feb. 3, 2013, 8:33 p.m.
gcc with -Wextra complains about an ordered pointer comparison:

target-s390x/helper.c:660:27: warning:
 ordered comparison of pointer with integer zero [-Wextra]

Obviously the index was missing in the code.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---

I hope my analysis and the fix is correct - please review.

For local builds, I always compile with -Wextra. This bug shows that -Wextra
would be good as a default compiler option for QEMU. Of course some extra
warnings must be explicitly disabled then. I'll send a patch for this after 1.4.

Regards,

Stefan W.


 target-s390x/helper.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Cornelia Huck - Feb. 4, 2013, 8:52 a.m.
On Sun,  3 Feb 2013 21:33:16 +0100
Stefan Weil <sw@weilnetz.de> wrote:

> gcc with -Wextra complains about an ordered pointer comparison:
> 
> target-s390x/helper.c:660:27: warning:
>  ordered comparison of pointer with integer zero [-Wextra]
> 
> Obviously the index was missing in the code.
> 
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> 
> I hope my analysis and the fix is correct - please review.
> 
> For local builds, I always compile with -Wextra. This bug shows that -Wextra
> would be good as a default compiler option for QEMU. Of course some extra
> warnings must be explicitly disabled then. I'll send a patch for this after 1.4.
> 
> Regards,
> 
> Stefan W.
> 
> 
>  target-s390x/helper.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target-s390x/helper.c b/target-s390x/helper.c
> index 3180b90..8bd84ef 100644
> --- a/target-s390x/helper.c
> +++ b/target-s390x/helper.c
> @@ -657,7 +657,7 @@ static void do_io_interrupt(CPUS390XState *env)
>          cpu_unmap_lowcore(lowcore);
> 
>          env->io_index[isc]--;
> -        if (env->io_index >= 0) {
> +        if (env->io_index[isc] >= 0) {
>              disable = 0;
>          }
>          break;

Oops, obvious bug when you look at it :)

Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Anthony Liguori - Feb. 4, 2013, 10:50 p.m.
Applied.  Thanks.

Regards,

Anthony Liguori

Patch

diff --git a/target-s390x/helper.c b/target-s390x/helper.c
index 3180b90..8bd84ef 100644
--- a/target-s390x/helper.c
+++ b/target-s390x/helper.c
@@ -657,7 +657,7 @@  static void do_io_interrupt(CPUS390XState *env)
         cpu_unmap_lowcore(lowcore);
 
         env->io_index[isc]--;
-        if (env->io_index >= 0) {
+        if (env->io_index[isc] >= 0) {
             disable = 0;
         }
         break;