fix clang build (#210)

Also add clang to pull request build checks.

Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@nutanix.com>

5 files changed, 61 insertions, 46 deletions

diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml
index 4717bbe..1f661a5 100644
--- a/.github/workflows/pull_request.yml
+++ b/.github/workflows/pull_request.yml
@@ -7,7 +7,7 @@ jobs:
       - uses: actions/checkout@v2
       - name: pre-push
         run: |
-          sudo apt-get -y install libjson-c-dev libcmocka-dev
+          sudo apt-get -y install libjson-c-dev libcmocka-dev clang
           make pre-push VERBOSE=1
   ubuntu-18:
     runs-on: ubuntu-18.04
@@ -15,7 +15,7 @@ jobs:
       - uses: actions/checkout@v2
       - name: pre-push
         run: |
-          sudo apt-get -y install libjson-c-dev libcmocka-dev
+          sudo apt-get -y install libjson-c-dev libcmocka-dev clang
           make pre-push VERBOSE=1
   centos-7:
     runs-on: ubuntu-latest
@@ -25,5 +25,5 @@ jobs:
       - name: pre-push
         run: |
           yum -y install make gcc-4.8.5 epel-release
-          yum -y install cmake json-c-devel libcmocka-devel openssl-devel
+          yum -y install clang cmake json-c-devel libcmocka-devel openssl-devel
           make pre-push VERBOSE=1
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 4b1b0b0..29a40cc 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -50,7 +50,8 @@ set(CMAKE_C_FLAGS_DEBUG "-O0 -ggdb")
 #
 set(CMAKE_C_FLAGS_RELEASE "-O3")
 
-set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Werror -Wextra")
+set(CMAKE_C_FLAGS
+    "${CMAKE_C_FLAGS} -Wall -Werror -Wextra -Wno-missing-field-initializers")
 
 # public headers
 add_subdirectory(include)
diff --git a/Makefile b/Makefile
index a140ccd..f45d824 100644
--- a/Makefile
+++ b/Makefile
@@ -58,8 +58,11 @@ test: all
 	cd $(BUILD_DIR)/test; ctest --verbose
 
 pre-push: realclean
-	make test
-	make test BUILD_TYPE=rel
+	make test CC=gcc
+	make test CC=gcc BUILD_TYPE=rel
+	make realclean
+	make test CC=clang
+	make test CC=clang BUILD_TYPE=rel
 
 realclean:
 	rm -rf $(BUILD_DIR_BASE)
diff --git a/samples/client.c b/samples/client.c
index 576815d..3baaa18 100644
--- a/samples/client.c
+++ b/samples/client.c
@@ -155,7 +155,7 @@ recv_version(int sock, int *server_max_fds, size_t *pgsize)
     }
 
     if (sversion->major != LIB_VFIO_USER_MAJOR) {
-        errx(EXIT_FAILURE, "unsupported server major %hu (must be %hu)",
+        errx(EXIT_FAILURE, "unsupported server major %hu (must be %u)",
                sversion->major, LIB_VFIO_USER_MAJOR);
     }
 
@@ -163,7 +163,7 @@ recv_version(int sock, int *server_max_fds, size_t *pgsize)
      * The server is supposed to tell us the minimum agreed version.
      */
     if (sversion->minor > LIB_VFIO_USER_MINOR) {
-        errx(EXIT_FAILURE, "unsupported server minor %hu (must be %hu)",
+        errx(EXIT_FAILURE, "unsupported server minor %hu (must be <= %u)",
                sversion->minor, LIB_VFIO_USER_MINOR);
     }
 
@@ -437,21 +437,25 @@ access_region(int sock, int region, bool is_write, uint64_t offset,
             .iov_len = data_len
         }
     };
-    struct {
-        struct vfio_user_region_access region_access;
-        char data[data_len];
-    } __attribute__((packed)) recv_data;
-    int op, ret;
+
+    struct vfio_user_region_access *recv_data;
     size_t nr_send_iovecs, recv_data_len;
+    int op, ret;
 
     if (is_write) {
         op = VFIO_USER_REGION_WRITE;
         nr_send_iovecs = 3;
-        recv_data_len = sizeof(recv_data.region_access);
+        recv_data_len = sizeof(*recv_data);
     } else {
         op = VFIO_USER_REGION_READ;
         nr_send_iovecs = 2;
-        recv_data_len = sizeof(recv_data);
+        recv_data_len = sizeof (*recv_data) + data_len;
+    }
+
+    recv_data = calloc(1, recv_data_len);
+
+    if (recv_data == NULL) {
+        err(EXIT_FAILURE, "failed to alloc recv_data");
     }
 
     ret = vfu_msg_iovec(sock, 0, op,
@@ -462,12 +466,14 @@ access_region(int sock, int region, bool is_write, uint64_t offset,
         warnx("failed to %s region %d %#lx-%#lx: %s",
              is_write ? "write to" : "read from", region, offset,
              offset + data_len - 1, strerror(-ret));
+        free(recv_data);
         return ret;
     }
-    if (recv_data.region_access.count != data_len) {
+    if (recv_data->count != data_len) {
         warnx("bad %s data count, expected=%lu, actual=%d",
              is_write ? "write" : "read", data_len,
-             recv_data.region_access.count);
+             recv_data->count);
+        free(recv_data);
         return -EINVAL;
     }
 
@@ -476,8 +482,9 @@ access_region(int sock, int region, bool is_write, uint64_t offset,
      * response into an iovec, but it's some work to implement it.
      */
     if (!is_write) {
-        memcpy(data, recv_data.data, data_len);
+        memcpy(data, ((char *)recv_data) + sizeof (*recv_data), data_len);
     }
+    free(recv_data);
     return 0;
 }
 
diff --git a/test/unit-tests.c b/test/unit-tests.c
index c3de8e9..7de11ac 100644
--- a/test/unit-tests.c
+++ b/test/unit-tests.c
@@ -278,6 +278,30 @@ test_dma_controller_remove_region_no_fd(void **state __attribute__((unused)))
     assert_int_equal(0, dma_controller_remove_region(dma, r.dma_addr, r.size, NULL, NULL));
 }
 
+static int fds[] = { 0xab, 0xcd };
+
+static int
+set_fds(const long unsigned int value, const long unsigned int data)
+{
+    assert(value != 0);
+    if ((void*)data == &get_next_command) {
+        memcpy((int*)value, fds, ARRAY_SIZE(fds) * sizeof(int));
+    } else if ((void*)data == &exec_command) {
+        ((int*)value)[0] = -1;
+    }
+    return 1;
+}
+
+static int
+set_nr_fds(const long unsigned int value,
+           const long unsigned int data __attribute__((unused)))
+{
+    int *nr_fds = (int*)value;
+    assert(nr_fds != NULL);
+    *nr_fds = ARRAY_SIZE(fds);
+    return 1;
+}
+
 /*
  * Tests that if if exec_command fails then process_request frees passed file
  * descriptors.
@@ -285,27 +309,6 @@ test_dma_controller_remove_region_no_fd(void **state __attribute__((unused)))
 static void
 test_process_command_free_passed_fds(void **state __attribute__((unused)))
 {
-    int fds[] = {0xab, 0xcd};
-    int set_fds(const long unsigned int value,
-                const long unsigned int data)
-    {
-        assert(value != 0);
-        if ((void*)data == &get_next_command) {
-            memcpy((int*)value, fds, ARRAY_SIZE(fds) * sizeof(int));
-        } else if ((void*)data == &exec_command) {
-            ((int*)value)[0] = -1;
-        }
-        return 1;
-    }
-    int set_nr_fds(const long unsigned int value,
-                   const long unsigned int data __attribute__((unused)))
-    {
-        int *nr_fds = (int*)value;
-        assert(nr_fds != NULL);
-        *nr_fds = ARRAY_SIZE(fds);
-        return 1;
-    }
-
     vfu_ctx_t vfu_ctx = {
         .conn_fd = 0xcafebabe,
         .migration = (struct migration*)0x8badf00d
@@ -375,16 +378,17 @@ test_realize_ctx(void **state __attribute__((unused)))
     assert_null(vfu_ctx.pci.caps);
 }
 
-static void
-test_attach_ctx(void **state __attribute__((unused)))
+static int
+dummy_attach(vfu_ctx_t *vfu_ctx)
 {
-    int dummy_attach(vfu_ctx_t *vfu_ctx)
-    {
-        assert(vfu_ctx != NULL);
+    assert(vfu_ctx != NULL);
 
-        return 222;
-    }
+    return 222;
+}
 
+static void
+test_attach_ctx(void **state __attribute__((unused)))
+{
     struct transport_ops transport_ops = {
         .attach = &dummy_attach,
     };