qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 00/12] Compiler warning fixes for libvhost-user,libvduse
@ 2022-12-22 20:36 Marcel Holtmann
  2022-12-22 20:36 ` [PATCH v4 01/12] libvhost-user: Provide _GNU_SOURCE when compiling outside of QEMU Marcel Holtmann
                   ` (13 more replies)
  0 siblings, 14 replies; 18+ messages in thread
From: Marcel Holtmann @ 2022-12-22 20:36 UTC (permalink / raw)
  To: qemu-devel, mst, xieyongji, pbonzini; +Cc: marcel

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.

Marcel Holtmann (12):
  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
  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
  libvhost-user: Fix assignment in vring_set_avail_event
  libvhost-user: Add extra compiler warnings
  libvduse: Add extra compiler warnings

 subprojects/libvduse/libvduse.c           |  9 ++++--
 subprojects/libvduse/meson.build          |  8 ++++-
 subprojects/libvhost-user/libvhost-user.c | 36 +++++++++++++----------
 subprojects/libvhost-user/meson.build     |  8 ++++-
 4 files changed, 42 insertions(+), 19 deletions(-)

-- 
2.38.1



^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v4 01/12] libvhost-user: Provide _GNU_SOURCE when compiling outside of QEMU
  2022-12-22 20:36 [PATCH v4 00/12] Compiler warning fixes for libvhost-user,libvduse Marcel Holtmann
@ 2022-12-22 20:36 ` Marcel Holtmann
  2022-12-22 20:36 ` [PATCH v4 02/12] libvhost-user: Replace typeof with __typeof__ Marcel Holtmann
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 18+ messages in thread
From: Marcel Holtmann @ 2022-12-22 20:36 UTC (permalink / raw)
  To: qemu-devel, mst, xieyongji, pbonzini; +Cc: marcel

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



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 02/12] libvhost-user: Replace typeof with __typeof__
  2022-12-22 20:36 [PATCH v4 00/12] Compiler warning fixes for libvhost-user,libvduse Marcel Holtmann
  2022-12-22 20:36 ` [PATCH v4 01/12] libvhost-user: Provide _GNU_SOURCE when compiling outside of QEMU Marcel Holtmann
@ 2022-12-22 20:36 ` Marcel Holtmann
  2022-12-22 20:36 ` [PATCH v4 03/12] libvhost-user: Cast rc variable to avoid compiler warning Marcel Holtmann
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 18+ messages in thread
From: Marcel Holtmann @ 2022-12-22 20:36 UTC (permalink / raw)
  To: qemu-devel, mst, xieyongji, pbonzini; +Cc: marcel

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>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.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



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 03/12] libvhost-user: Cast rc variable to avoid compiler warning
  2022-12-22 20:36 [PATCH v4 00/12] Compiler warning fixes for libvhost-user,libvduse Marcel Holtmann
  2022-12-22 20:36 ` [PATCH v4 01/12] libvhost-user: Provide _GNU_SOURCE when compiling outside of QEMU Marcel Holtmann
  2022-12-22 20:36 ` [PATCH v4 02/12] libvhost-user: Replace typeof with __typeof__ Marcel Holtmann
@ 2022-12-22 20:36 ` Marcel Holtmann
  2022-12-22 20:36 ` [PATCH v4 04/12] libvhost-user: Use unsigned int i for some for-loop iterations Marcel Holtmann
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 18+ messages in thread
From: Marcel Holtmann @ 2022-12-22 20:36 UTC (permalink / raw)
  To: qemu-devel, mst, xieyongji, pbonzini; +Cc: marcel

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



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 04/12] libvhost-user: Use unsigned int i for some for-loop iterations
  2022-12-22 20:36 [PATCH v4 00/12] Compiler warning fixes for libvhost-user,libvduse Marcel Holtmann
                   ` (2 preceding siblings ...)
  2022-12-22 20:36 ` [PATCH v4 03/12] libvhost-user: Cast rc variable to avoid compiler warning Marcel Holtmann
@ 2022-12-22 20:36 ` Marcel Holtmann
  2022-12-22 20:36 ` [PATCH v4 05/12] libvhost-user: Declare uffdio_register early to make it C90 compliant Marcel Holtmann
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 18+ messages in thread
From: Marcel Holtmann @ 2022-12-22 20:36 UTC (permalink / raw)
  To: qemu-devel, mst, xieyongji, pbonzini; +Cc: marcel

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



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 05/12] libvhost-user: Declare uffdio_register early to make it C90 compliant
  2022-12-22 20:36 [PATCH v4 00/12] Compiler warning fixes for libvhost-user,libvduse Marcel Holtmann
                   ` (3 preceding siblings ...)
  2022-12-22 20:36 ` [PATCH v4 04/12] libvhost-user: Use unsigned int i for some for-loop iterations Marcel Holtmann
@ 2022-12-22 20:36 ` Marcel Holtmann
  2022-12-22 20:36 ` [PATCH v4 06/12] libvhost-user: Change dev->postcopy_ufd assignment " Marcel Holtmann
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 18+ messages in thread
From: Marcel Holtmann @ 2022-12-22 20:36 UTC (permalink / raw)
  To: qemu-devel, mst, xieyongji, pbonzini; +Cc: marcel

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>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.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



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 06/12] libvhost-user: Change dev->postcopy_ufd assignment to make it C90 compliant
  2022-12-22 20:36 [PATCH v4 00/12] Compiler warning fixes for libvhost-user,libvduse Marcel Holtmann
                   ` (4 preceding siblings ...)
  2022-12-22 20:36 ` [PATCH v4 05/12] libvhost-user: Declare uffdio_register early to make it C90 compliant Marcel Holtmann
@ 2022-12-22 20:36 ` Marcel Holtmann
  2022-12-22 20:36 ` [PATCH v4 07/12] libvduse: Provide _GNU_SOURCE when compiling outside of QEMU Marcel Holtmann
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 18+ messages in thread
From: Marcel Holtmann @ 2022-12-22 20:36 UTC (permalink / raw)
  To: qemu-devel, mst, xieyongji, pbonzini; +Cc: marcel

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>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.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



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 07/12] libvduse: Provide _GNU_SOURCE when compiling outside of QEMU
  2022-12-22 20:36 [PATCH v4 00/12] Compiler warning fixes for libvhost-user,libvduse Marcel Holtmann
                   ` (5 preceding siblings ...)
  2022-12-22 20:36 ` [PATCH v4 06/12] libvhost-user: Change dev->postcopy_ufd assignment " Marcel Holtmann
@ 2022-12-22 20:36 ` Marcel Holtmann
  2022-12-22 20:36 ` [PATCH v4 08/12] libvduse: Switch to unsigned int for inuse field in struct VduseVirtq Marcel Holtmann
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 18+ messages in thread
From: Marcel Holtmann @ 2022-12-22 20:36 UTC (permalink / raw)
  To: qemu-devel, mst, xieyongji, pbonzini; +Cc: marcel

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



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 08/12] libvduse: Switch to unsigned int for inuse field in struct VduseVirtq
  2022-12-22 20:36 [PATCH v4 00/12] Compiler warning fixes for libvhost-user,libvduse Marcel Holtmann
                   ` (6 preceding siblings ...)
  2022-12-22 20:36 ` [PATCH v4 07/12] libvduse: Provide _GNU_SOURCE when compiling outside of QEMU Marcel Holtmann
@ 2022-12-22 20:36 ` Marcel Holtmann
  2022-12-22 20:36 ` [PATCH v4 09/12] libvduse: Fix assignment in vring_set_avail_event Marcel Holtmann
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 18+ messages in thread
From: Marcel Holtmann @ 2022-12-22 20:36 UTC (permalink / raw)
  To: qemu-devel, mst, xieyongji, pbonzini; +Cc: marcel

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>
Reviewed-by: Xie Yongji <xieyongji@bytedance.com>
---
 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



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 09/12] libvduse: Fix assignment in vring_set_avail_event
  2022-12-22 20:36 [PATCH v4 00/12] Compiler warning fixes for libvhost-user,libvduse Marcel Holtmann
                   ` (7 preceding siblings ...)
  2022-12-22 20:36 ` [PATCH v4 08/12] libvduse: Switch to unsigned int for inuse field in struct VduseVirtq Marcel Holtmann
@ 2022-12-22 20:36 ` Marcel Holtmann
  2022-12-22 20:36 ` [PATCH v4 10/12] libvhost-user: " Marcel Holtmann
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 18+ messages in thread
From: Marcel Holtmann @ 2022-12-22 20:36 UTC (permalink / raw)
  To: qemu-devel, mst, xieyongji, pbonzini; +Cc: marcel

Since the assignment is causing a compiler warning, fix it by using
memcpy instead.

  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>
Suggested-by: Xie Yongji <xieyongji@bytedance.com>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
---
 subprojects/libvduse/libvduse.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/subprojects/libvduse/libvduse.c b/subprojects/libvduse/libvduse.c
index 338ad5e352e7..377959a0b4fb 100644
--- a/subprojects/libvduse/libvduse.c
+++ b/subprojects/libvduse/libvduse.c
@@ -582,7 +582,8 @@ 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);
+    uint16_t val_le = htole16(val);
+    memcpy(&vq->vring.used->ring[vq->vring.num], &val_le, sizeof(uint16_t));
 }
 
 static bool vduse_queue_map_single_desc(VduseVirtq *vq, unsigned int *p_num_sg,
-- 
2.38.1



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 10/12] libvhost-user: Fix assignment in vring_set_avail_event
  2022-12-22 20:36 [PATCH v4 00/12] Compiler warning fixes for libvhost-user,libvduse Marcel Holtmann
                   ` (8 preceding siblings ...)
  2022-12-22 20:36 ` [PATCH v4 09/12] libvduse: Fix assignment in vring_set_avail_event Marcel Holtmann
@ 2022-12-22 20:36 ` Marcel Holtmann
  2022-12-22 20:36 ` [PATCH v4 11/12] libvhost-user: Add extra compiler warnings Marcel Holtmann
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 18+ messages in thread
From: Marcel Holtmann @ 2022-12-22 20:36 UTC (permalink / raw)
  To: qemu-devel, mst, xieyongji, pbonzini; +Cc: marcel

Since it was proposed to change the code in libvduse.c to use memcpy
instead of an assignment, the code in libvhost-user.c should also be
changed to use memcpy.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
---
 subprojects/libvhost-user/libvhost-user.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
index b28b66cdb159..fc69783d2bf6 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -2478,14 +2478,13 @@ vring_used_flags_unset_bit(VuVirtq *vq, int mask)
 static inline void
 vring_set_avail_event(VuVirtq *vq, uint16_t val)
 {
-    uint16_t *avail;
+    uint16_t val_le = htole16(val);
 
     if (!vq->notification) {
         return;
     }
 
-    avail = (uint16_t *)&vq->vring.used->ring[vq->vring.num];
-    *avail = htole16(val);
+    memcpy(&vq->vring.used->ring[vq->vring.num], &val_le, sizeof(uint16_t));
 }
 
 void
-- 
2.38.1



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 11/12] libvhost-user: Add extra compiler warnings
  2022-12-22 20:36 [PATCH v4 00/12] Compiler warning fixes for libvhost-user,libvduse Marcel Holtmann
                   ` (9 preceding siblings ...)
  2022-12-22 20:36 ` [PATCH v4 10/12] libvhost-user: " Marcel Holtmann
@ 2022-12-22 20:36 ` Marcel Holtmann
  2022-12-22 20:36 ` [PATCH v4 12/12] libvduse: " Marcel Holtmann
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 18+ messages in thread
From: Marcel Holtmann @ 2022-12-22 20:36 UTC (permalink / raw)
  To: qemu-devel, mst, xieyongji, pbonzini; +Cc: marcel

In case libvhost-user is used externally, that projects compiler
warnings might be more strict. Enforce an extra set of compiler warnings
to catch issues early on.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
---
 subprojects/libvhost-user/meson.build | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/subprojects/libvhost-user/meson.build b/subprojects/libvhost-user/meson.build
index 39825d9404ae..a18014e7f26f 100644
--- a/subprojects/libvhost-user/meson.build
+++ b/subprojects/libvhost-user/meson.build
@@ -1,6 +1,12 @@
 project('libvhost-user', 'c',
         license: 'GPL-2.0-or-later',
-        default_options: ['c_std=gnu99'])
+        default_options: ['warning_level=1', 'c_std=gnu99'])
+
+cc = meson.get_compiler('c')
+add_project_arguments(cc.get_supported_arguments('-Wsign-compare',
+                                                 '-Wdeclaration-after-statement',
+                                                 '-Wstrict-aliasing'),
+                      native: false, language: 'c')
 
 threads = dependency('threads')
 glib = dependency('glib-2.0')
-- 
2.38.1



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 12/12] libvduse: Add extra compiler warnings
  2022-12-22 20:36 [PATCH v4 00/12] Compiler warning fixes for libvhost-user,libvduse Marcel Holtmann
                   ` (10 preceding siblings ...)
  2022-12-22 20:36 ` [PATCH v4 11/12] libvhost-user: Add extra compiler warnings Marcel Holtmann
@ 2022-12-22 20:36 ` Marcel Holtmann
  2022-12-23 15:21 ` [PATCH v4 00/12] Compiler warning fixes for libvhost-user,libvduse Paolo Bonzini
  2023-01-05  1:58 ` Michael S. Tsirkin
  13 siblings, 0 replies; 18+ messages in thread
From: Marcel Holtmann @ 2022-12-22 20:36 UTC (permalink / raw)
  To: qemu-devel, mst, xieyongji, pbonzini; +Cc: marcel

In case libvhost-user is used externally, that projects compiler
warnings might be more strict. Enforce an extra set of compiler warnings
to catch issues early on.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
---
 subprojects/libvduse/meson.build | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/subprojects/libvduse/meson.build b/subprojects/libvduse/meson.build
index ba08f5ee1a03..3e3b53da33ae 100644
--- a/subprojects/libvduse/meson.build
+++ b/subprojects/libvduse/meson.build
@@ -1,6 +1,12 @@
 project('libvduse', 'c',
         license: 'GPL-2.0-or-later',
-        default_options: ['c_std=gnu99'])
+        default_options: ['warning_level=1', 'c_std=gnu99'])
+
+cc = meson.get_compiler('c')
+add_project_arguments(cc.get_supported_arguments('-Wsign-compare',
+                                                 '-Wdeclaration-after-statement',
+                                                 '-Wstrict-aliasing'),
+                      native: false, language: 'c')
 
 libvduse = static_library('vduse',
                           files('libvduse.c'),
-- 
2.38.1



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 00/12] Compiler warning fixes for libvhost-user,libvduse
  2022-12-22 20:36 [PATCH v4 00/12] Compiler warning fixes for libvhost-user,libvduse Marcel Holtmann
                   ` (11 preceding siblings ...)
  2022-12-22 20:36 ` [PATCH v4 12/12] libvduse: " Marcel Holtmann
@ 2022-12-23 15:21 ` Paolo Bonzini
  2022-12-23 15:28   ` Marcel Holtmann
  2022-12-23 17:55   ` Michael S. Tsirkin
  2023-01-05  1:58 ` Michael S. Tsirkin
  13 siblings, 2 replies; 18+ messages in thread
From: Paolo Bonzini @ 2022-12-23 15:21 UTC (permalink / raw)
  To: Marcel Holtmann, qemu-devel, mst, xieyongji

On 12/22/22 21:36, Marcel Holtmann wrote:
> 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.
> 
> Marcel Holtmann (12):
>    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
>    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
>    libvhost-user: Fix assignment in vring_set_avail_event
>    libvhost-user: Add extra compiler warnings
>    libvduse: Add extra compiler warnings
> 
>   subprojects/libvduse/libvduse.c           |  9 ++++--
>   subprojects/libvduse/meson.build          |  8 ++++-
>   subprojects/libvhost-user/libvhost-user.c | 36 +++++++++++++----------
>   subprojects/libvhost-user/meson.build     |  8 ++++-
>   4 files changed, 42 insertions(+), 19 deletions(-)
> 

Looks good, but what happened to "libvhost-user: Switch to unsigned int 
for inuse field in struct VuVirtq"?

(I can pick it up from v3, no need to respin).

Paolo



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 00/12] Compiler warning fixes for libvhost-user,libvduse
  2022-12-23 15:21 ` [PATCH v4 00/12] Compiler warning fixes for libvhost-user,libvduse Paolo Bonzini
@ 2022-12-23 15:28   ` Marcel Holtmann
  2022-12-23 17:55   ` Michael S. Tsirkin
  1 sibling, 0 replies; 18+ messages in thread
From: Marcel Holtmann @ 2022-12-23 15:28 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu devel list, Michael S. Tsirkin, xieyongji

Hi Paolo,

>> 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.
>> Marcel Holtmann (12):
>>   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
>>   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
>>   libvhost-user: Fix assignment in vring_set_avail_event
>>   libvhost-user: Add extra compiler warnings
>>   libvduse: Add extra compiler warnings
>>  subprojects/libvduse/libvduse.c           |  9 ++++--
>>  subprojects/libvduse/meson.build          |  8 ++++-
>>  subprojects/libvhost-user/libvhost-user.c | 36 +++++++++++++----------
>>  subprojects/libvhost-user/meson.build     |  8 ++++-
>>  4 files changed, 42 insertions(+), 19 deletions(-)
> 
> Looks good, but what happened to "libvhost-user: Switch to unsigned int for inuse field in struct VuVirtq"?
> 
> (I can pick it up from v3, no need to respin).

I found that it was already upstream and thus I removed it.

Regards

Marcel



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 00/12] Compiler warning fixes for libvhost-user,libvduse
  2022-12-23 15:21 ` [PATCH v4 00/12] Compiler warning fixes for libvhost-user,libvduse Paolo Bonzini
  2022-12-23 15:28   ` Marcel Holtmann
@ 2022-12-23 17:55   ` Michael S. Tsirkin
  1 sibling, 0 replies; 18+ messages in thread
From: Michael S. Tsirkin @ 2022-12-23 17:55 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Marcel Holtmann, qemu-devel, xieyongji

On Fri, Dec 23, 2022 at 04:21:33PM +0100, Paolo Bonzini wrote:
> On 12/22/22 21:36, Marcel Holtmann wrote:
> > 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.
> > 
> > Marcel Holtmann (12):
> >    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
> >    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
> >    libvhost-user: Fix assignment in vring_set_avail_event
> >    libvhost-user: Add extra compiler warnings
> >    libvduse: Add extra compiler warnings
> > 
> >   subprojects/libvduse/libvduse.c           |  9 ++++--
> >   subprojects/libvduse/meson.build          |  8 ++++-
> >   subprojects/libvhost-user/libvhost-user.c | 36 +++++++++++++----------
> >   subprojects/libvhost-user/meson.build     |  8 ++++-
> >   4 files changed, 42 insertions(+), 19 deletions(-)
> > 
> 
> Looks good, but what happened to "libvhost-user: Switch to unsigned int for
> inuse field in struct VuVirtq"?
> 
> (I can pick it up from v3, no need to respin).
> 
> Paolo

I merged that one IIRC.
Paolo I wandered whether if you are going to be merging patches in these
areas you wanted to be added to MAINTAINERS.

-- 
MST



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 00/12] Compiler warning fixes for libvhost-user,libvduse
  2022-12-22 20:36 [PATCH v4 00/12] Compiler warning fixes for libvhost-user,libvduse Marcel Holtmann
                   ` (12 preceding siblings ...)
  2022-12-23 15:21 ` [PATCH v4 00/12] Compiler warning fixes for libvhost-user,libvduse Paolo Bonzini
@ 2023-01-05  1:58 ` Michael S. Tsirkin
  2023-01-10  9:54   ` [PATCH v4 00/12] Compiler warning fixes for libvhost-user, libvduse Paolo Bonzini
  13 siblings, 1 reply; 18+ messages in thread
From: Michael S. Tsirkin @ 2023-01-05  1:58 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: qemu-devel, xieyongji, pbonzini

On Thu, Dec 22, 2022 at 09:36:39PM +0100, Marcel Holtmann wrote:
> 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.
> 
> Marcel Holtmann (12):
>   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
>   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
>   libvhost-user: Fix assignment in vring_set_avail_event
>   libvhost-user: Add extra compiler warnings
>   libvduse: Add extra compiler warnings


series looks good to me.

Paolo I understand you are merging this?

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

>  subprojects/libvduse/libvduse.c           |  9 ++++--
>  subprojects/libvduse/meson.build          |  8 ++++-
>  subprojects/libvhost-user/libvhost-user.c | 36 +++++++++++++----------
>  subprojects/libvhost-user/meson.build     |  8 ++++-
>  4 files changed, 42 insertions(+), 19 deletions(-)
> 
> -- 
> 2.38.1
> 
> 
> 



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 00/12] Compiler warning fixes for libvhost-user, libvduse
  2023-01-05  1:58 ` Michael S. Tsirkin
@ 2023-01-10  9:54   ` Paolo Bonzini
  0 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2023-01-10  9:54 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Marcel Holtmann, qemu-devel, Xie Yongji

[-- Attachment #1: Type: text/plain, Size: 865 bytes --]

Il gio 5 gen 2023, 02:58 Michael S. Tsirkin <mst@redhat.com> ha scritto:

> Paolo I understand you are merging this?
>

It would be fine either way; when I find issues in a series I tend to queue
it just in case the other maintainer prefers to leave further handling to
me. In this case it caught my eye due to the meson.build implications.

Anyhow I have already passed it through CI (a bit slow due to the holidays)
and will send the PR today or tomorrow.

Paolo


> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>
> >  subprojects/libvduse/libvduse.c           |  9 ++++--
> >  subprojects/libvduse/meson.build          |  8 ++++-
> >  subprojects/libvhost-user/libvhost-user.c | 36 +++++++++++++----------
> >  subprojects/libvhost-user/meson.build     |  8 ++++-
> >  4 files changed, 42 insertions(+), 19 deletions(-)
> >
> > --
> > 2.38.1
> >
> >
> >
>
>

[-- Attachment #2: Type: text/html, Size: 1652 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2023-01-10 10:59 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-22 20:36 [PATCH v4 00/12] Compiler warning fixes for libvhost-user,libvduse Marcel Holtmann
2022-12-22 20:36 ` [PATCH v4 01/12] libvhost-user: Provide _GNU_SOURCE when compiling outside of QEMU Marcel Holtmann
2022-12-22 20:36 ` [PATCH v4 02/12] libvhost-user: Replace typeof with __typeof__ Marcel Holtmann
2022-12-22 20:36 ` [PATCH v4 03/12] libvhost-user: Cast rc variable to avoid compiler warning Marcel Holtmann
2022-12-22 20:36 ` [PATCH v4 04/12] libvhost-user: Use unsigned int i for some for-loop iterations Marcel Holtmann
2022-12-22 20:36 ` [PATCH v4 05/12] libvhost-user: Declare uffdio_register early to make it C90 compliant Marcel Holtmann
2022-12-22 20:36 ` [PATCH v4 06/12] libvhost-user: Change dev->postcopy_ufd assignment " Marcel Holtmann
2022-12-22 20:36 ` [PATCH v4 07/12] libvduse: Provide _GNU_SOURCE when compiling outside of QEMU Marcel Holtmann
2022-12-22 20:36 ` [PATCH v4 08/12] libvduse: Switch to unsigned int for inuse field in struct VduseVirtq Marcel Holtmann
2022-12-22 20:36 ` [PATCH v4 09/12] libvduse: Fix assignment in vring_set_avail_event Marcel Holtmann
2022-12-22 20:36 ` [PATCH v4 10/12] libvhost-user: " Marcel Holtmann
2022-12-22 20:36 ` [PATCH v4 11/12] libvhost-user: Add extra compiler warnings Marcel Holtmann
2022-12-22 20:36 ` [PATCH v4 12/12] libvduse: " Marcel Holtmann
2022-12-23 15:21 ` [PATCH v4 00/12] Compiler warning fixes for libvhost-user,libvduse Paolo Bonzini
2022-12-23 15:28   ` Marcel Holtmann
2022-12-23 17:55   ` Michael S. Tsirkin
2023-01-05  1:58 ` Michael S. Tsirkin
2023-01-10  9:54   ` [PATCH v4 00/12] Compiler warning fixes for libvhost-user, libvduse Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).