Hi,
On 1/3/23 23:03, Michael S. Tsirkin wrote:>> diff --git
a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index f35178f5fc..db70c4976e 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1071,6 +1071,7 @@ static void virtqueue_split_get_avail_bytes(VirtQueue *vq,
VirtIODevice *vdev = vq->vdev;
unsigned int max, idx;
unsigned int total_bufs, in_total, out_total;
+ MemoryRegionCache *desc_cache;
why are you moving desc_cache here?
Just to make virtqueue_split_get_avail_bytes() and
virtqueue_packed_get_avail_bytes() symmetrical, but I'll keep it out of v2.
MemoryRegionCache indirect_desc_cache = MEMORY_REGION_CACHE_INVALID;
int64_t len = 0;
int rc;
@@ -1078,15 +1079,13 @@ static void virtqueue_split_get_avail_bytes(VirtQueue
*vq,
idx = vq->last_avail_idx;
total_bufs = in_total = out_total = 0;
- max = vq->vring.num;
-
while ((rc = virtqueue_num_heads(vq, idx)) > 0) {
- MemoryRegionCache *desc_cache = &caches->desc;
- unsigned int num_bufs;
+ unsigned int num_bufs = total_bufs;
VRingDesc desc;
unsigned int i;
- num_bufs = total_bufs;
nice cleanup but not a bugfix. Keep cleanups separate from fixes pls.
Ok makes sense.
+ desc_cache = &caches->desc;
init as part of declaration seems cleaner.
+ max = vq->vring.num;
can we move declaration of max here within the loop?
will make sure the problem does not recur.
Yes, sounds good.
if (!virtqueue_get_head(vq, idx++, &i)) {
goto err;
@@ -1218,14 +1217,14 @@ static void virtqueue_packed_get_avail_bytes(VirtQueue
*vq,
wrap_counter = vq->last_avail_wrap_counter;
total_bufs = in_total = out_total = 0;
- max = vq->vring.num;
-
for (;;) {
unsigned int num_bufs = total_bufs;
unsigned int i = idx;
int rc;
desc_cache = &caches->desc;
+ max = vq->vring.num;
+
same question can we move declaration into the loop?
Same as above.
--
Carlos López
Security Engineer
SUSE Software Solutions