From 0502943fb1ce6f24aff1f3086e55f7ceff86bd68 Mon Sep 17 00:00:00 2001 From: John Levon Date: Wed, 16 Dec 2020 11:47:10 +0000 Subject: fix clang build (#210) Also add clang to pull request build checks. Signed-off-by: John Levon Reviewed-by: Thanos Makatos --- .github/workflows/pull_request.yml | 6 ++-- CMakeLists.txt | 3 +- Makefile | 7 +++-- samples/client.c | 31 ++++++++++++-------- test/unit-tests.c | 60 ++++++++++++++++++++------------------ 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, }; -- cgit v1.1