Re: [PATCH] hw/virtio/vhost: simplify ring mapping verification with loop

["Michael S. Tsirkin" <mst@redhat.com>]

On Fri, Apr 03, 2026 at 05:16:21PM +0800, Bin Guo wrote:
> Replace the three nearly-identical vhost_verify_ring_part_mapping() calls
> with a simple loop over arrays. This eliminates code duplication while
> maintaining identical behavior and error reporting.
> 
> Signed-off-by: Bin Guo <guobin@linux.alibaba.com>
> ---
>  hw/virtio/vhost.c | 34 ++++++++++++----------------------
>  1 file changed, 12 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index b9dc4ed13b..7b5bfc86e6 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -525,31 +525,21 @@ static int vhost_verify_ring_mappings(struct vhost_dev *dev,
>              continue;
>          }
>  
> -        j = 0;
> -        r = vhost_verify_ring_part_mapping(
> -                vq->desc, vq->desc_phys, vq->desc_size,
> -                reg_hva, reg_gpa, reg_size);
> -        if (r) {
> -            break;
> -        }
> -
> -        j++;
> -        r = vhost_verify_ring_part_mapping(
> -                vq->avail, vq->avail_phys, vq->avail_size,
> -                reg_hva, reg_gpa, reg_size);
> -        if (r) {
> -            break;
> -        }
> -
> -        j++;
> -        r = vhost_verify_ring_part_mapping(
> -                vq->used, vq->used_phys, vq->used_size,
> -                reg_hva, reg_gpa, reg_size);
> -        if (r) {
> -            break;
> +        void *ring_addrs[] = {vq->desc, vq->avail, vq->used};
> +        uint64_t ring_phys[] = {vq->desc_phys, vq->avail_phys, vq->used_phys};
> +        uint64_t ring_sizes[] = {vq->desc_size, vq->avail_size, vq->used_size};
> +
> +        for (j = 0; j < 3; j++) {
> +            r = vhost_verify_ring_part_mapping(
> +                    ring_addrs[j], ring_phys[j], ring_sizes[j],
> +                    reg_hva, reg_gpa, reg_size);
> +            if (r) {
> +                goto out;
> +            }
>          }
>      }
>  
> +out:
>      if (r == -ENOMEM) {
>          error_report("Unable to map %s for ring %d", part_name[j], i);


I don't see the point, sorry. Maybe if it was a single array,
and we used ARRAY_SIZE ... Even then, I am not sure.

As it is, we are replacing straight forward linear code with
tricky arrays.

>      } else if (r == -EBUSY) {
> -- 
> 2.50.1 (Apple Git-155)