1. Patchew
  2. QEMU
  3. View series

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

Stefan Weil posted 1 patch 6 years, 7 months ago
Test docker-clang@ubuntu passed
Test asan passed
Test docker-mingw@fedora passed
Test FreeBSD passed
Test s390x passed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190712081910.29085-1-sw@weilnetz.de
There is a newer version of this series
contrib/libvhost-user/libvhost-user.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
[Qemu-devel] [PATCH] libvhost-user: Add missing GCC_FMT_ATTR and fix three format errors
Posted by Stefan Weil 6 years, 7 months ago 
Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 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;
     }
 
-- 
2.20.1
Re: [Qemu-devel] [PATCH] libvhost-user: Add missing GCC_FMT_ATTR and fix three format errors
Posted by Philippe Mathieu-Daudé 6 years, 7 months ago 
On 7/12/19 10:19 AM, Stefan Weil wrote:
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>  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é <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Thanks,

Phil.
Re: [Qemu-devel] [PATCH] libvhost-user: Add missing GCC_FMT_ATTR and fix three format errors
Posted by Marc-André Lureau 6 years, 7 months ago 
On Fri, Jul 12, 2019 at 12:25 PM Stefan Weil <sw@weilnetz.de> wrote:
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>  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);

The original version (hw/virtio/virtio.c) print *head. The message
isn't great, but I would rather stay consistent for now.

other than that,
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


>          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;
>      }
>
> --
> 2.20.1
>
>


-- 
Marc-André Lureau

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.