Re: [Qemu-devel] [PATCH] libvhost-user: Add missing GCC_FMT_ATTR and fix three format errors

[Philippe Mathieu-Daudé]

On 7/12/19 10:19 AM, Stefan Weil wrote:
> Signed-off-by: Stefan Weil <address@hidden>
> ---
>  contrib/libvhost-user/libvhost-user.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/contrib/libvhost-user/libvhost-user.c 
> b/contrib/libvhost-user/libvhost-user.c
> index 4b36e35a82..59b3202979 100644
> --- a/contrib/libvhost-user/libvhost-user.c
> +++ b/contrib/libvhost-user/libvhost-user.c
> @@ -142,7 +142,7 @@ vu_request_to_string(unsigned int req)
>      }
>  }
>  
> -static void
> +static void GCC_FMT_ATTR(2, 3)
>  vu_panic(VuDev *dev, const char *msg, ...)
>  {
>      char *buf = NULL;
> @@ -661,7 +661,7 @@ vu_set_mem_table_exec_postcopy(VuDev *dev, VhostUserMsg 
> *vmsg)
>  
>          if (ioctl(dev->postcopy_ufd, UFFDIO_REGISTER, &reg_struct)) {
>              vu_panic(dev, "%s: Failed to userfault region %d "
> -                          "@%p + size:%zx offset: %zx: (ufd=%d)%s\n",
> +                          "@%" PRIx64 " + size:%zx offset: %zx: 
> (ufd=%d)%s\n",
>                       __func__, i,
>                       dev_region->mmap_addr,
>                       dev_region->size, dev_region->mmap_offset,
> @@ -1753,7 +1753,7 @@ virtqueue_get_head(VuDev *dev, VuVirtq *vq,
>  
>      /* If their number is silly, that's a fatal mistake. */
>      if (*head >= vq->vring.num) {
> -        vu_panic(dev, "Guest says index %u is available", head);
> +        vu_panic(dev, "Guest says index %u is available", idx);
>          return false;
>      }
>  
> @@ -1812,7 +1812,7 @@ virtqueue_read_next_desc(VuDev *dev, struct vring_desc 
> *desc,
>      smp_wmb();
>  
>      if (*next >= max) {
> -        vu_panic(dev, "Desc next is %u", next);
> +        vu_panic(dev, "Desc next is %u", *next);
>          return VIRTQUEUE_READ_DESC_ERROR;
>      }
>  
> 

This fixes:

      CC      contrib/libvhost-user/libvhost-user.o
    contrib/libvhost-user/libvhost-user.c: In function
'vu_set_mem_table_exec_postcopy':
    contrib/libvhost-user/libvhost-user.c:663:27: error: format '%p'
expects argument of type 'void *', but argument 5 has type 'uint64_t
{aka long long unsigned int}' [-Werror=format=]
                 vu_panic(dev, "%s: Failed to userfault region %d "
                               ^
    contrib/libvhost-user/libvhost-user.c: In function 'virtqueue_get_head':
    contrib/libvhost-user/libvhost-user.c:1756:42: error: format '%u'
expects argument of type 'unsigned int', but argument 3 has type
'unsigned int *' [-Werror=format=]
             vu_panic(dev, "Guest says index %u is available", head);
                                              ^
    contrib/libvhost-user/libvhost-user.c: In function
'virtqueue_read_next_desc':
    contrib/libvhost-user/libvhost-user.c:1815:38: error: format '%u'
expects argument of type 'unsigned int', but argument 3 has type
'unsigned int *' [-Werror=format=]
             vu_panic(dev, "Desc next is %u", next);
                                          ^

However with your patch applied I still have:

    contrib/libvhost-user/libvhost-user.c: In function
'vu_set_mem_table_exec_postcopy':
    contrib/libvhost-user/libvhost-user.c:663:27: error: format '%zx'
expects argument of type 'size_t', but argument 6 has type 'uint64_t
{aka long long unsigned int}' [-Werror=format=]
                 vu_panic(dev, "%s: Failed to userfault region %d "
                               ^
    contrib/libvhost-user/libvhost-user.c:663:27: error: format '%zx'
expects argument of type 'size_t', but argument 7 has type 'uint64_t
{aka long long unsigned int}' [-Werror=format=]
    cc1: all warnings being treated as errors

Which is right:

typedef struct VuDevRegion {
    /* Guest Physical address. */
    uint64_t gpa;
    /* Memory region size. */
    uint64_t size;
    /* QEMU virtual address (userspace). */
    uint64_t qva;
    /* Starting offset in our mmaped space. */
    uint64_t mmap_offset;
    /* Start address of mmaped space. */
    uint64_t mmap_addr;
} VuDevRegion;

Build succeed applying this on top of your patch:

-- >8 --
--- a/contrib/libvhost-user/libvhost-user.c
+++ b/contrib/libvhost-user/libvhost-user.c
@@ -661,7 +661,8 @@ vu_set_mem_table_exec_postcopy(VuDev *dev,
VhostUserMsg *vmsg)

         if (ioctl(dev->postcopy_ufd, UFFDIO_REGISTER, &reg_struct)) {
             vu_panic(dev, "%s: Failed to userfault region %d "
-                          "@%" PRIx64 " + size:%zx offset: %zx:
(ufd=%d)%s\n",
+                          "@%" PRIx64 " + size:%" PRIx64 " offset: %"
PRIx64
+                          ": (ufd=%d)%s\n",
                      __func__, i,
                      dev_region->mmap_addr,
                      dev_region->size, dev_region->mmap_offset,
---

With this snippet amended:
Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
Tested-by: Philippe Mathieu-Daudé <address@hidden>

Thanks,

Phil.