| Message ID | 20180516132757.68558-1-pasic@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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(-)
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(-) >
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.
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);
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(-)