[PATCH v2 00/10] Compiler warning fixes for libvhost-user,libvduse

[PATCH v2 00/10] Compiler warning fixes for libvhost-user,libvduse

[Marcel Holtmann]

The libvhost-user and libvduse libraries are also useful for external
usage outside of QEMU and thus it would be nice if their files could
be just copied and used. However due to different compiler settings, a
lot of manual fixups are needed. This is the first attempt at some
obvious fixes that can be done without any harm to the code and its
readability.

Please note that 10/10 in this series is marked as RFC since I have no
idea what is correct here. From the structures point of view the
assignment that is done makes no sense to me. I might have to dig into
specification to figure it out or it better be commented accordingly
to tell the compiler that it got this part wrong.

Marcel Holtmann (10):
  libvhost-user: Provide _GNU_SOURCE when compiling outside of QEMU
  libvhost-user: Replace typeof with __typeof__
  libvhost-user: Cast rc variable to avoid compiler warning
  libvhost-user: Use unsigned int i for some for-loop iterations
  libvhost-user: Declare uffdio_register early to make it C90 compliant
  libvhost-user: Change dev->postcopy_ufd assignment to make it C90 compliant
  libvhost-user: Switch to unsigned int for inuse field in struct VuVirtq
  libvduse: Provide _GNU_SOURCE when compiling outside of QEMU
  libvduse: Switch to unsigned int for inuse field in struct VduseVirtq
  libvduse: Fix assignment in vring_set_avail_event

 subprojects/libvduse/libvduse.c           | 11 ++++++--
 subprojects/libvhost-user/libvhost-user.c | 31 ++++++++++++++---------
 subprojects/libvhost-user/libvhost-user.h |  2 +-
 3 files changed, 29 insertions(+), 15 deletions(-)

-- 
2.38.1

[PATCH v2 01/10] libvhost-user: Provide _GNU_SOURCE when compiling outside of QEMU

[Marcel Holtmann]

Then the libvhost-user sources are used by another project, it can not
be guaranteed that _GNU_SOURCE is set by the build system. If it is for
example not set, errors like this show up.

  CC       libvhost-user.o
libvhost-user.c: In function ‘vu_panic’:
libvhost-user.c:195:9: error: implicit declaration of function ‘vasprintf’; did you mean ‘vsprintf’? [-Werror=implicit-function-declaration]
  195 |     if (vasprintf(&buf, msg, ap) < 0) {
      |         ^~~~~~~~~
      |         vsprintf

The simplest way to allow external complication of libvhost-user.[ch] is
by setting _GNU_SOURCE if it is not already set by the build system.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
 subprojects/libvhost-user/libvhost-user.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
index d6ee6e7d9168..b55b9e244d9a 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -13,6 +13,10 @@
  * later.  See the COPYING file in the top-level directory.
  */
 
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE
+#endif
+
 /* this code avoids GLib dependency */
 #include <stdlib.h>
 #include <stdio.h>
-- 
2.38.1

[PATCH v2 02/10] libvhost-user: Replace typeof with __typeof__

[Marcel Holtmann]

Strictly speaking only -std=gnu99 support the usage of typeof and for
easier inclusion in external projects, it is better to use __typeof__.

  CC       libvhost-user.o
libvhost-user.c: In function ‘vu_log_queue_fill’:
libvhost-user.c:86:13: error: implicit declaration of function ‘typeof’ [-Werror=implicit-function-declaration]
   86 |             typeof(x) _min1 = (x);              \
      |             ^~~~~~

Changing these two users of typeof makes the compiler happy and no extra
flags or pragmas need to be provided.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
 subprojects/libvhost-user/libvhost-user.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
index b55b9e244d9a..67d75ece53b7 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -62,8 +62,8 @@
 #endif  /* !__GNUC__ */
 #ifndef MIN
 #define MIN(x, y) ({                            \
-            typeof(x) _min1 = (x);              \
-            typeof(y) _min2 = (y);              \
+            __typeof__(x) _min1 = (x);          \
+            __typeof__(y) _min2 = (y);          \
             (void) (&_min1 == &_min2);          \
             _min1 < _min2 ? _min1 : _min2; })
 #endif
-- 
2.38.1

Re: [PATCH v2 02/10] libvhost-user: Replace typeof with __typeof__

[Philippe Mathieu-Daudé]

On 20/12/22 20:27, Marcel Holtmann wrote:
> Strictly speaking only -std=gnu99 support the usage of typeof and for
> easier inclusion in external projects, it is better to use __typeof__.
> 
>    CC       libvhost-user.o
> libvhost-user.c: In function ‘vu_log_queue_fill’:
> libvhost-user.c:86:13: error: implicit declaration of function ‘typeof’ [-Werror=implicit-function-declaration]
>     86 |             typeof(x) _min1 = (x);              \
>        |             ^~~~~~
> 
> Changing these two users of typeof makes the compiler happy and no extra
> flags or pragmas need to be provided.
> 
> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
> ---
>   subprojects/libvhost-user/libvhost-user.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH v2 03/10] libvhost-user: Cast rc variable to avoid compiler warning

[Marcel Holtmann]

The assert from recvmsg() return value against an uint32_t size field
from a protocol struct throws a compiler warning.

  CC       libvhost-user.o
In file included from libvhost-user.c:27:
libvhost-user.c: In function ‘vu_message_read_default’:
libvhost-user.c:363:19: error: comparison of integer expressions of different signedness: ‘int’ and ‘uint32_t’ {aka ‘unsigned int’} [-Werror=sign-compare]
  363 |         assert(rc == vmsg->size);
      |                   ^~

This is not critical, but annoying when the libvhost-user source are
used in an external project that has this compiler warning switched on.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
 subprojects/libvhost-user/libvhost-user.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
index 67d75ece53b7..bcdf32a24f60 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -339,7 +339,7 @@ vu_message_read_default(VuDev *dev, int conn_fd, VhostUserMsg *vmsg)
             goto fail;
         }
 
-        assert(rc == vmsg->size);
+        assert((uint32_t)rc == vmsg->size);
     }
 
     return true;
-- 
2.38.1

[PATCH v2 04/10] libvhost-user: Use unsigned int i for some for-loop iterations

[Marcel Holtmann]

The sign-compare warning also hits some of the for-loops, but it easy
fixed by just making the iterator variable unsigned int.

  CC       libvhost-user.o
libvhost-user.c: In function ‘vu_gpa_to_va’:
libvhost-user.c:223:19: error: comparison of integer expressions of different signedness: ‘int’ and ‘uint32_t’ {aka ‘unsigned int’} [-Werror=sign-compare]
  223 |     for (i = 0; i < dev->nregions; i++) {
      |                   ^

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
 subprojects/libvhost-user/libvhost-user.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
index bcdf32a24f60..211d31a4cc88 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -192,7 +192,7 @@ vu_panic(VuDev *dev, const char *msg, ...)
 void *
 vu_gpa_to_va(VuDev *dev, uint64_t *plen, uint64_t guest_addr)
 {
-    int i;
+    unsigned int i;
 
     if (*plen == 0) {
         return NULL;
@@ -218,7 +218,7 @@ vu_gpa_to_va(VuDev *dev, uint64_t *plen, uint64_t guest_addr)
 static void *
 qva_to_va(VuDev *dev, uint64_t qemu_addr)
 {
-    int i;
+    unsigned int i;
 
     /* Find matching memory region.  */
     for (i = 0; i < dev->nregions; i++) {
@@ -621,7 +621,7 @@ map_ring(VuDev *dev, VuVirtq *vq)
 
 static bool
 generate_faults(VuDev *dev) {
-    int i;
+    unsigned int i;
     for (i = 0; i < dev->nregions; i++) {
         VuDevRegion *dev_region = &dev->regions[i];
         int ret;
@@ -829,7 +829,7 @@ static inline bool reg_equal(VuDevRegion *vudev_reg,
 static bool
 vu_rem_mem_reg(VuDev *dev, VhostUserMsg *vmsg) {
     VhostUserMemoryRegion m = vmsg->payload.memreg.region, *msg_region = &m;
-    int i;
+    unsigned int i;
     bool found = false;
 
     if (vmsg->fd_num > 1) {
@@ -895,7 +895,7 @@ vu_rem_mem_reg(VuDev *dev, VhostUserMsg *vmsg) {
 static bool
 vu_set_mem_table_exec_postcopy(VuDev *dev, VhostUserMsg *vmsg)
 {
-    int i;
+    unsigned int i;
     VhostUserMemory m = vmsg->payload.memory, *memory = &m;
     dev->nregions = memory->nregions;
 
@@ -972,7 +972,7 @@ vu_set_mem_table_exec_postcopy(VuDev *dev, VhostUserMsg *vmsg)
 static bool
 vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg)
 {
-    int i;
+    unsigned int i;
     VhostUserMemory m = vmsg->payload.memory, *memory = &m;
 
     for (i = 0; i < dev->nregions; i++) {
@@ -1980,7 +1980,7 @@ end:
 void
 vu_deinit(VuDev *dev)
 {
-    int i;
+    unsigned int i;
 
     for (i = 0; i < dev->nregions; i++) {
         VuDevRegion *r = &dev->regions[i];
-- 
2.38.1

[PATCH v2 05/10] libvhost-user: Declare uffdio_register early to make it C90 compliant

[Marcel Holtmann]

When using libvhost-user source in an external project that wants to
comply with the C90 standard, it is best to declare variables before
code.

  CC       libvhost-user.o
libvhost-user.c: In function ‘generate_faults’:
libvhost-user.c:683:9: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
  683 |         struct uffdio_register reg_struct;
      |         ^~~~~~

In this case, it is also simple enough and doesn't cause any extra
ifdef additions.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
 subprojects/libvhost-user/libvhost-user.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
index 211d31a4cc88..bf92cc85c086 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -626,6 +626,8 @@ generate_faults(VuDev *dev) {
         VuDevRegion *dev_region = &dev->regions[i];
         int ret;
 #ifdef UFFDIO_REGISTER
+        struct uffdio_register reg_struct;
+
         /*
          * We should already have an open ufd. Mark each memory
          * range as ufd.
@@ -659,7 +661,7 @@ generate_faults(VuDev *dev) {
                     "%s: Failed to madvise(NOHUGEPAGE) region %d: %s\n",
                     __func__, i, strerror(errno));
         }
-        struct uffdio_register reg_struct;
+
         reg_struct.range.start = (uintptr_t)dev_region->mmap_addr;
         reg_struct.range.len = dev_region->size + dev_region->mmap_offset;
         reg_struct.mode = UFFDIO_REGISTER_MODE_MISSING;
-- 
2.38.1

Re: [PATCH v2 05/10] libvhost-user: Declare uffdio_register early to make it C90 compliant

[Philippe Mathieu-Daudé]

On 20/12/22 20:27, Marcel Holtmann wrote:
> When using libvhost-user source in an external project that wants to
> comply with the C90 standard, it is best to declare variables before
> code.
> 
>    CC       libvhost-user.o
> libvhost-user.c: In function ‘generate_faults’:
> libvhost-user.c:683:9: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
>    683 |         struct uffdio_register reg_struct;
>        |         ^~~~~~
> 
> In this case, it is also simple enough and doesn't cause any extra
> ifdef additions.
> 
> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
> ---
>   subprojects/libvhost-user/libvhost-user.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH v2 06/10] libvhost-user: Change dev->postcopy_ufd assignment to make it C90 compliant

[Marcel Holtmann]

The assignment of dev->postcopy_ufd can be moved into an else clause and
then the code becomes C90 compliant.

  CC       libvhost-user.o
libvhost-user.c: In function ‘vu_set_postcopy_advise’:
libvhost-user.c:1625:5: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
 1625 |     struct uffdio_api api_struct;
      |     ^~~~~~

Understandable, it might be desired to avoid else clauses, but in this
case it seems clear enough and frankly the dev->postcopy_ufd is only
assigned once.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
 subprojects/libvhost-user/libvhost-user.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
index bf92cc85c086..b28b66cdb159 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -1599,12 +1599,13 @@ vu_set_config(VuDev *dev, VhostUserMsg *vmsg)
 static bool
 vu_set_postcopy_advise(VuDev *dev, VhostUserMsg *vmsg)
 {
-    dev->postcopy_ufd = -1;
 #ifdef UFFDIO_API
     struct uffdio_api api_struct;
 
     dev->postcopy_ufd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK);
     vmsg->size = 0;
+#else
+    dev->postcopy_ufd = -1;
 #endif
 
     if (dev->postcopy_ufd == -1) {
-- 
2.38.1

Re: [PATCH v2 06/10] libvhost-user: Change dev->postcopy_ufd assignment to make it C90 compliant

[Philippe Mathieu-Daudé]

On 20/12/22 20:27, Marcel Holtmann wrote:
> The assignment of dev->postcopy_ufd can be moved into an else clause and
> then the code becomes C90 compliant.
> 
>    CC       libvhost-user.o
> libvhost-user.c: In function ‘vu_set_postcopy_advise’:
> libvhost-user.c:1625:5: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
>   1625 |     struct uffdio_api api_struct;
>        |     ^~~~~~
> 
> Understandable, it might be desired to avoid else clauses, but in this
> case it seems clear enough and frankly the dev->postcopy_ufd is only
> assigned once.
> 
> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
> ---
>   subprojects/libvhost-user/libvhost-user.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH v2 07/10] libvhost-user: Switch to unsigned int for inuse field in struct VuVirtq

[Marcel Holtmann]

It seems there is no need to keep the inuse field signed and end up with
compiler warnings for sign-compare.

  CC       libvhost-user.o
libvhost-user.c: In function ‘vu_queue_pop’:
libvhost-user.c:2763:19: error: comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
 2763 |     if (vq->inuse >= vq->vring.num) {
      |                   ^~
libvhost-user.c: In function ‘vu_queue_rewind’:
libvhost-user.c:2808:13: error: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Werror=sign-compare]
 2808 |     if (num > vq->inuse) {
      |             ^

Instead of casting the comparison to unsigned int, just make the inuse
field unsigned int in the fist place.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 subprojects/libvhost-user/libvhost-user.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/subprojects/libvhost-user/libvhost-user.h b/subprojects/libvhost-user/libvhost-user.h
index aea7ec5061d5..8cda9b8f577a 100644
--- a/subprojects/libvhost-user/libvhost-user.h
+++ b/subprojects/libvhost-user/libvhost-user.h
@@ -343,7 +343,7 @@ typedef struct VuVirtq {
     /* Notification enabled? */
     bool notification;
 
-    int inuse;
+    unsigned int inuse;
 
     vu_queue_handler_cb handler;
 
-- 
2.38.1

[PATCH v2 08/10] libvduse: Provide _GNU_SOURCE when compiling outside of QEMU

[Marcel Holtmann]

When the libvduse sources are used by another project, it can not be
guaranteed that _GNU_SOURCE is set by the build system. If it is for
example not set, errors like this show up.

  CC       libvduse.o
libvduse.c: In function ‘vduse_log_get’:
libvduse.c:172:9: error: implicit declaration of function ‘ftruncate’; did you mean ‘strncat’? [-Werror=implicit-function-declaration]
  172 |     if (ftruncate(fd, size) == -1) {
      |         ^~~~~~~~~
      |         strncat

The simplest way to allow external complication of libvduse.[ch] by
setting _GNU_SOURCE if it is not already set by the build system.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
 subprojects/libvduse/libvduse.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/subprojects/libvduse/libvduse.c b/subprojects/libvduse/libvduse.c
index e089d4d546cf..c871bd331a6b 100644
--- a/subprojects/libvduse/libvduse.c
+++ b/subprojects/libvduse/libvduse.c
@@ -16,6 +16,10 @@
  * later.  See the COPYING file in the top-level directory.
  */
 
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE
+#endif
+
 #include <stdlib.h>
 #include <stdio.h>
 #include <stdbool.h>
-- 
2.38.1

[PATCH v2 09/10] libvduse: Switch to unsigned int for inuse field in struct VduseVirtq

[Marcel Holtmann]

It seems there is no need to keep the inuse field signed and end up with
compiler warnings for sign-compare.

  CC       libvduse.o
libvduse.c: In function ‘vduse_queue_pop’:
libvduse.c:789:19: error: comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
  789 |     if (vq->inuse >= vq->vring.num) {
      |                   ^~

Instead of casting the comparison to unsigned int, just make the inuse
field unsigned int in the fist place.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
 subprojects/libvduse/libvduse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/subprojects/libvduse/libvduse.c b/subprojects/libvduse/libvduse.c
index c871bd331a6b..338ad5e352e7 100644
--- a/subprojects/libvduse/libvduse.c
+++ b/subprojects/libvduse/libvduse.c
@@ -101,7 +101,7 @@ struct VduseVirtq {
     uint16_t signalled_used;
     bool signalled_used_valid;
     int index;
-    int inuse;
+    unsigned int inuse;
     bool ready;
     int fd;
     VduseDev *dev;
-- 
2.38.1

Re: [PATCH v2 09/10] libvduse: Switch to unsigned int for inuse field in struct VduseVirtq

[Yongji Xie]

On Wed, Dec 21, 2022 at 3:27 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> It seems there is no need to keep the inuse field signed and end up with
> compiler warnings for sign-compare.
>
>   CC       libvduse.o
> libvduse.c: In function ‘vduse_queue_pop’:
> libvduse.c:789:19: error: comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
>   789 |     if (vq->inuse >= vq->vring.num) {
>       |                   ^~
>
> Instead of casting the comparison to unsigned int, just make the inuse
> field unsigned int in the fist place.
>
> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
> ---
>  subprojects/libvduse/libvduse.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Reviewed-by: Xie Yongji <xieyongji@bytedance.com>

Thanks,
Yongji

[RFC v2 10/10] libvduse: Fix assignment in vring_set_avail_event

[Marcel Holtmann]

  CC       libvduse.o
libvduse.c: In function ‘vring_set_avail_event’:
libvduse.c:603:7: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasin]
  603 |     *((uint16_t *)&vq->vring.used->ring[vq->vring.num]) = htole16(val);
      |      ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
 subprojects/libvduse/libvduse.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/subprojects/libvduse/libvduse.c b/subprojects/libvduse/libvduse.c
index 338ad5e352e7..51a4ba1b6878 100644
--- a/subprojects/libvduse/libvduse.c
+++ b/subprojects/libvduse/libvduse.c
@@ -582,7 +582,10 @@ void vduse_queue_notify(VduseVirtq *vq)
 
 static inline void vring_set_avail_event(VduseVirtq *vq, uint16_t val)
 {
-    *((uint16_t *)&vq->vring.used->ring[vq->vring.num]) = htole16(val);
+    vring_used_elem_t *ring = &vq->vring.used->ring[vq->vring.num];
+
+    /* FIXME: Is this actually correct since this is __virtio32 id; */
+    ring->id = htole16(val);
 }
 
 static bool vduse_queue_map_single_desc(VduseVirtq *vq, unsigned int *p_num_sg,
-- 
2.38.1

Re: [RFC v2 10/10] libvduse: Fix assignment in vring_set_avail_event

[Yongji Xie]

On Wed, Dec 21, 2022 at 3:27 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
>   CC       libvduse.o
> libvduse.c: In function ‘vring_set_avail_event’:
> libvduse.c:603:7: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasin]
>   603 |     *((uint16_t *)&vq->vring.used->ring[vq->vring.num]) = htole16(val);
>       |      ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
> ---
>  subprojects/libvduse/libvduse.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/subprojects/libvduse/libvduse.c b/subprojects/libvduse/libvduse.c
> index 338ad5e352e7..51a4ba1b6878 100644
> --- a/subprojects/libvduse/libvduse.c
> +++ b/subprojects/libvduse/libvduse.c
> @@ -582,7 +582,10 @@ void vduse_queue_notify(VduseVirtq *vq)
>
>  static inline void vring_set_avail_event(VduseVirtq *vq, uint16_t val)
>  {
> -    *((uint16_t *)&vq->vring.used->ring[vq->vring.num]) = htole16(val);
> +    vring_used_elem_t *ring = &vq->vring.used->ring[vq->vring.num];
> +
> +    /* FIXME: Is this actually correct since this is __virtio32 id; */
> +    ring->id = htole16(val);
>  }

Can we do it as libvhost-user does？

static inline void
vring_set_avail_event(VuVirtq *vq, uint16_t val)
{
    uint16_t *avail;

    avail = (uint16_t *)&vq->vring.used->ring[vq->vring.num];
    *avail = htole16(val);
}

Thanks,
Yongji

Re: [RFC v2 10/10] libvduse: Fix assignment in vring_set_avail_event

[Marcel Holtmann]

Hi Yongji,

>>  CC       libvduse.o
>> libvduse.c: In function ‘vring_set_avail_event’:
>> libvduse.c:603:7: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasin]
>>  603 |     *((uint16_t *)&vq->vring.used->ring[vq->vring.num]) = htole16(val);
>>      |      ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> 
>> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
>> ---
>> subprojects/libvduse/libvduse.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>> 
>> diff --git a/subprojects/libvduse/libvduse.c b/subprojects/libvduse/libvduse.c
>> index 338ad5e352e7..51a4ba1b6878 100644
>> --- a/subprojects/libvduse/libvduse.c
>> +++ b/subprojects/libvduse/libvduse.c
>> @@ -582,7 +582,10 @@ void vduse_queue_notify(VduseVirtq *vq)
>> 
>> static inline void vring_set_avail_event(VduseVirtq *vq, uint16_t val)
>> {
>> -    *((uint16_t *)&vq->vring.used->ring[vq->vring.num]) = htole16(val);
>> +    vring_used_elem_t *ring = &vq->vring.used->ring[vq->vring.num];
>> +
>> +    /* FIXME: Is this actually correct since this is __virtio32 id; */
>> +    ring->id = htole16(val);
>> }
> 
> Can we do it as libvhost-user does？
> 
> static inline void
> vring_set_avail_event(VuVirtq *vq, uint16_t val)
> {
>    uint16_t *avail;
> 
>    avail = (uint16_t *)&vq->vring.used->ring[vq->vring.num];
>    *avail = htole16(val);
> }

that will also work. Sending a v3 in a few moments.

Regards

Marcel