From 3b78bf3730eafb44235f64bdb6c34633889fbaf8 Mon Sep 17 00:00:00 2001 From: Francisco Date: Tue, 20 Apr 2021 11:14:11 +0100 Subject: [PATCH] bugfix - fix memcheck warnings. move instructions with side effects outside of asserts --- lib/include/srsran/common/network_utils.h | 7 +-- lib/src/common/network_utils.cc | 5 ++- lib/test/common/byte_buffer_queue_test.cc | 53 ++++++++++++----------- srsenb/test/upper/s1ap_test.cc | 3 +- 4 files changed, 37 insertions(+), 31 deletions(-) diff --git a/lib/include/srsran/common/network_utils.h b/lib/include/srsran/common/network_utils.h index d22cbc48a..d897dc9fb 100644 --- a/lib/include/srsran/common/network_utils.h +++ b/lib/include/srsran/common/network_utils.h @@ -144,9 +144,10 @@ private: // used to unlock select struct ctrl_cmd_t { enum class cmd_id_t { EXIT, NEW_FD, RM_FD }; - cmd_id_t cmd = cmd_id_t::EXIT; - int new_fd = -1; - bool signal_rm_complete = false; + cmd_id_t cmd; + int new_fd; + bool signal_rm_complete; + ctrl_cmd_t() { bzero(this, sizeof(ctrl_cmd_t)); } }; std::map::iterator remove_socket_unprotected(int fd, fd_set* total_fd_set, int* max_fd); diff --git a/lib/src/common/network_utils.cc b/lib/src/common/network_utils.cc index 1645da978..d9959fa39 100644 --- a/lib/src/common/network_utils.cc +++ b/lib/src/common/network_utils.cc @@ -329,7 +329,8 @@ bool sctp_init_server(unique_socket* socket, net_utils::socket_type socktype, co socket_manager::socket_manager() : thread("RXsockets"), socket_manager_itf(srslog::fetch_basic_logger("COMN")) { // register control pipe fd - srsran_assert(pipe(pipefd) != -1, "Failed to open control pipe"); + int fd = pipe(pipefd); + srsran_assert(fd != -1, "Failed to open control pipe"); start(thread_prio); } @@ -344,7 +345,7 @@ void socket_manager::stop() // close thread { std::lock_guard lock(socket_mutex); - ctrl_cmd_t msg{}; + ctrl_cmd_t msg; msg.cmd = ctrl_cmd_t::cmd_id_t::EXIT; if (write(pipefd[1], &msg, sizeof(msg)) != sizeof(msg)) { rxSockError("while writing to control pipe"); diff --git a/lib/test/common/byte_buffer_queue_test.cc b/lib/test/common/byte_buffer_queue_test.cc index dbd4db650..a9e1496b5 100644 --- a/lib/test/common/byte_buffer_queue_test.cc +++ b/lib/test/common/byte_buffer_queue_test.cc @@ -22,53 +22,56 @@ typedef struct { byte_buffer_queue* q; } args_t; -void* write_thread(void* a) +void write_thread(byte_buffer_queue* q) { - args_t* args = (args_t*)a; + unique_byte_buffer_t b; for (uint32_t i = 0; i < NMSGS; i++) { - unique_byte_buffer_t b = srsran::make_byte_buffer(); - if (b == nullptr) { - return nullptr; - } + do { + b = srsran::make_byte_buffer(); + if (b == nullptr) { + // wait until pool is not depleted + std::this_thread::yield(); + } + } while (b == nullptr); memcpy(b->msg, &i, 4); b->N_bytes = 4; - args->q->write(std::move(b)); + q->write(std::move(b)); } - return NULL; } -int main(int argc, char** argv) +int test_concurrent_writeread() { - bool result; byte_buffer_queue q; unique_byte_buffer_t b; - pthread_t thread; - args_t args; - u_int32_t r; + int result = 0; - result = true; - args.q = &q; - - pthread_create(&thread, NULL, &write_thread, &args); + std::thread t([&q]() { write_thread(&q); }); for (uint32_t i = 0; i < NMSGS; i++) { - b = q.read(); + b = q.read(); + uint32_t r = 0; memcpy(&r, b->msg, 4); - if (r != i) - result = false; + if (r != i) { + result = -1; + break; + } } - pthread_join(thread, NULL); + t.join(); if (q.size() != 0 || q.size_bytes() != 0) { - result = false; + result = -1; } - if (result) { + if (result == 0) { printf("Passed\n"); - exit(0); } else { printf("Failed\n;"); - exit(1); } + return result; +} + +int main() +{ + return test_concurrent_writeread(); } diff --git a/srsenb/test/upper/s1ap_test.cc b/srsenb/test/upper/s1ap_test.cc index 9e782696d..8791af8ae 100644 --- a/srsenb/test/upper/s1ap_test.cc +++ b/srsenb/test/upper/s1ap_test.cc @@ -28,7 +28,8 @@ struct mme_dummy { TESTASSERT(bind_addr(fd, mme_sockaddr)); } - srsran_assert(listen(fd, SOMAXCONN) == 0, "Failed to listen to incoming SCTP connections"); + int success = listen(fd, SOMAXCONN); + srsran_assert(success == 0, "Failed to listen to incoming SCTP connections"); } ~mme_dummy()