[Qemu-devel] [PATCH 1/1] virtio-ccw: clean up notify

[Qemu-devel] [PATCH 1/1] virtio-ccw: clean up notify

[Halil Pasic]

Coverity recently started complaining about virtio_ccw_notify().  Turns
out, there is a couple of things that can be cleaned up.  Let's clean!

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Fixes: CID 1390619
Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
---
 hw/s390x/virtio-ccw.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 22df33b509..09fa7eed4c 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -1003,10 +1003,15 @@ static void virtio_ccw_notify(DeviceState *d, uint16_t vector)
     SubchDev *sch = ccw_dev->sch;
     uint64_t indicators;
 
-    /* queue indicators + secondary indicators */
-    if (vector >= VIRTIO_QUEUE_MAX + 64) {
+    if (vector == VIRTIO_NO_VECTOR) {
         return;
     }
+    /*
+     * vector < VIRTIO_QUEUE_MAX: notification for a virtqueue
+     * vector == VIRTIO_QUEUE_MAX: configuration change notification
+     * bits beyond that are unused and should never be notified for
+     */
+    assert(vector <= VIRTIO_QUEUE_MAX);
 
     if (vector < VIRTIO_QUEUE_MAX) {
         if (!dev->indicators) {
@@ -1029,6 +1034,7 @@ static void virtio_ccw_notify(DeviceState *d, uint16_t vector)
                 css_adapter_interrupt(CSS_IO_ADAPTER_VIRTIO, dev->thinint_isc);
             }
         } else {
+            assert(vector < NR_CLASSIC_INDICATOR_BITS);
             indicators = address_space_ldq(&address_space_memory,
                                            dev->indicators->addr,
                                            MEMTXATTRS_UNSPECIFIED,
@@ -1042,12 +1048,11 @@ static void virtio_ccw_notify(DeviceState *d, uint16_t vector)
         if (!dev->indicators2) {
             return;
         }
-        vector = 0;
         indicators = address_space_ldq(&address_space_memory,
                                        dev->indicators2->addr,
                                        MEMTXATTRS_UNSPECIFIED,
                                        NULL);
-        indicators |= 1ULL << vector;
+        indicators |= 1ULL;
         address_space_stq(&address_space_memory, dev->indicators2->addr,
                           indicators, MEMTXATTRS_UNSPECIFIED, NULL);
         css_conditional_io_interrupt(sch);
-- 
2.16.3

Re: [Qemu-devel] [PATCH 1/1] virtio-ccw: clean up notify

[Cornelia Huck]

On Wed, 16 May 2018 15:27:57 +0200
Halil Pasic <pasic@linux.ibm.com> wrote:

> Coverity recently started complaining about virtio_ccw_notify().  Turns
> out, there is a couple of things that can be cleaned up.  Let's clean!

Changes look good to me.

I wanted to come up with a better patch description, but failed (well,
I did not try much.) So I'm inclined to merge this as-is.

> 
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Fixes: CID 1390619
> Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
> ---
>  hw/s390x/virtio-ccw.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)

Re: [Qemu-devel] [PATCH 1/1] virtio-ccw: clean up notify

[Halil Pasic]

On 05/17/2018 05:26 PM, Cornelia Huck wrote:
> On Wed, 16 May 2018 15:27:57 +0200
> Halil Pasic <pasic@linux.ibm.com> wrote:
> 
>> Coverity recently started complaining about virtio_ccw_notify().  Turns
>> out, there is a couple of things that can be cleaned up.  Let's clean!
> 
> Changes look good to me.
> 
> I wanted to come up with a better patch description, but failed (well,
> I did not try much.) So I'm inclined to merge this as-is.
> 

I also gave up on a good patch description real soon. The patch does not do
a single thing, but I don't think splitting it up would make things better.
So I was like: the changes are pretty straight-forward, and the commit message
should just hint refactoring.

Thanks!

Halil


>>
>> Reported-by: Peter Maydell <peter.maydell@linaro.org>
>> Fixes: CID 1390619
>> Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
>> ---
>>   hw/s390x/virtio-ccw.c | 13 +++++++++----
>>   1 file changed, 9 insertions(+), 4 deletions(-)
> 

Re: [Qemu-devel] [PATCH 1/1] virtio-ccw: clean up notify

[Cornelia Huck]

On Wed, 16 May 2018 15:27:57 +0200
Halil Pasic <pasic@linux.ibm.com> wrote:

> Coverity recently started complaining about virtio_ccw_notify().  Turns
> out, there is a couple of things that can be cleaned up.  Let's clean!
> 
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Fixes: CID 1390619
> Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
> ---
>  hw/s390x/virtio-ccw.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)

Thanks, applied.