Re: [PATCH v1] virtio/virtio.c: include virtio prefix in error message

[Alex Bennée]

Moteen Shah <moteenshah.02@gmail.com> writes:

> From: Moteen Shah <moteenshah.02@gmail.com>
>
> The error message in virtio_init_region_cache()
> is given a prefix virtio.

It seems a shame considering we have a common virtio_error function that
we couldn't do it in one place. One option would be to make virtio_error
a macro that could insert __func__ in place:

  #define virtio_error(vdev, fmt, ...)                            \
      virtio_error_impl(vdev, "%s: " fmt, __func__, ## __VA_ARGS__);

  void virtio_error_impl(VirtIODevice *vdev, const char *fmt, ...) 
G_GNUC_PRINTF(2, 3);

and make rename the function in virtio.c. As the functions calling this
all tend to have virtio in their names anyway. What do you think?

>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/230
> Buglink: https://bugs.launchpad.net/qemu/+bug/1919021``
>
> Signed-off-by: Moteen Shah <moteenshah.02@gmail.com>
> ---
>  hw/virtio/virtio.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 9d637e043e..f31427bd41 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -174,7 +174,7 @@ static void virtio_init_region_cache(VirtIODevice *vdev, 
> int n)
>      len = address_space_cache_init(&new->desc, vdev->dma_as,
>                                     addr, size, packed);
>      if (len < size) {
> -        virtio_error(vdev, "Cannot map desc");
> +        virtio_error(vdev, "Virtio cannot map desc");
>          goto err_desc;
>      }
>  
> @@ -182,7 +182,7 @@ static void virtio_init_region_cache(VirtIODevice *vdev, 
> int n)
>      len = address_space_cache_init(&new->used, vdev->dma_as,
>                                     vq->vring.used, size, true);
>      if (len < size) {
> -        virtio_error(vdev, "Cannot map used");
> +        virtio_error(vdev, "Virtio cannot map used");
>          goto err_used;
>      }
>  
> @@ -190,7 +190,7 @@ static void virtio_init_region_cache(VirtIODevice *vdev, 
> int n)
>      len = address_space_cache_init(&new->avail, vdev->dma_as,
>                                     vq->vring.avail, size, false);
>      if (len < size) {
> -        virtio_error(vdev, "Cannot map avail");
> +        virtio_error(vdev, "Virtio cannot map avail");
>          goto err_avail;
>      }


-- 
Alex Bennée