Merge pull request #432 from nqminds/tests/fix-test-edgesec-codecov

Use `__maybe_unused` compiler attribute to fix flakey code-coverage in `test_edgesec`

src/CMakeLists.txt

@@ -67,6 +67,6 @@ set_target_properties(edgesec PROPERTIES
 
 if (USE_CAPTURE_SERVICE)
   add_executable(edgesec-recap edgesec-recap.c)
-  target_link_libraries(edgesec-recap PRIVATE capture_service protobuf_middleware packet_queue packet_decoder sqlite_header os log SQLite::SQLite3 eloop::eloop)
+  target_link_libraries(edgesec-recap PRIVATE capture_service protobuf_middleware packet_queue packet_decoder sqlite_header attributes os log SQLite::SQLite3 eloop::eloop)
   target_include_directories(edgesec-recap PRIVATE ${PROJECT_BINARY_DIR})
 endif()

src/capture/middlewares/header_middleware/CMakeLists.txt

@@ -12,7 +12,7 @@ target_link_libraries(dns_decoder PUBLIC PCAP::pcap SQLite::SQLite3 PRIVATE log
 
 # packet_decoder.h has an #include <pcap.h>, so need to make it PUBLIC include
 add_library(packet_decoder packet_decoder.c)
-target_link_libraries(packet_decoder PUBLIC PCAP::pcap LibUTHash::LibUTHash PRIVATE mdns_decoder dns_decoder hash net log os hashmap)
+target_link_libraries(packet_decoder PUBLIC PCAP::pcap LibUTHash::LibUTHash attributes PRIVATE mdns_decoder dns_decoder hash net log os hashmap)
 
 add_library(packet_queue packet_queue.c)
 target_link_libraries(packet_queue PUBLIC packet_decoder eloop::list PRIVATE log os)

src/capture/middlewares/header_middleware/packet_decoder.h

@@ -16,6 +16,7 @@
 
 #include <utarray.h>
 #include "../../../utils/allocs.h"
+#include "../../../utils/attributes.h"
 #include "../../../utils/net.h"
 #include "../../../utils/os.h"
 

src/edgesec-recap.c

@@ -30,6 +30,7 @@
 #include "capture/middlewares/header_middleware/packet_queue.h"
 #include "capture/middlewares/header_middleware/sqlite_header.h"
 #include "capture/middlewares/protobuf_middleware/protobuf_middleware.h"
+#include "utils/attributes.h"
 #include "utils/os.h"
 #include "utils/sqliteu.h"
 #include "version.h"

src/radius/CMakeLists.txt

@@ -2,19 +2,25 @@ include_directories (
   "${PROJECT_SOURCE_DIR}/src"
 )
 
+add_library(common INTERFACE)
+set_target_properties(common PROPERTIES PUBLIC_HEADER "common.h")
+target_link_libraries(common INTERFACE attributes allocs log)
+
 add_library(md5_internal md5_internal.c)
 target_link_libraries(md5_internal PRIVATE log os)
 
 add_library(md5 md5.c)
 target_link_libraries(md5 PRIVATE md5_internal os)
 
 add_library(wpabuf wpabuf.c)
-target_link_libraries(wpabuf PRIVATE log os)
+target_link_libraries(wpabuf PUBLIC common PRIVATE log os)
 # wpabuf.h has BSD functions like be16toh, see https://linux.die.net/man/3/be16toh
 target_compile_definitions(wpabuf PUBLIC _DEFAULT_SOURCE _BSD_SOURCE)
 
 add_library(radius radius.c)
-target_link_libraries(radius PRIVATE wpabuf md5 md5_internal log os)
+target_link_libraries(radius
+  PUBLIC common attributes
+  PRIVATE wpabuf md5 md5_internal log os)
 
 add_library(radius_server radius_server.c)
 target_link_libraries(radius_server PUBLIC os eloop::eloop PRIVATE radius wpabuf log net)

src/radius/common.h

@@ -15,6 +15,7 @@
 
 #include <stddef.h>
 
+#include "../utils/attributes.h"
 #include "utils/allocs.h"
 #include "utils/log.h"
 

src/radius/radius.h

@@ -16,6 +16,7 @@
 
 #include "common.h"
 
+#include "../utils/attributes.h"
 #include "utils/os.h"
 
 /* RFC 2865 - RADIUS */

src/utils/CMakeLists.txt

@@ -4,6 +4,9 @@ set_target_properties(log PROPERTIES C_EXTENSIONS ON POSITION_INDEPENDENT_CODE O
 
 add_library(allocs allocs.c)
 
+add_library(attributes INTERFACE) # #define's for compiler attributes
+set_target_properties(attributes PROPERTIES PUBLIC_HEADER "attributes.h")
+
 if (USE_CRYPTO_SERVICE)
   add_library(cryptou cryptou.c)
   target_link_libraries(cryptou PRIVATE base64 os log OpenSSL::Crypto)

src/utils/attributes.h

@@ -0,0 +1,41 @@
+/**
+ * @file
+ * @author Alois Klink <alois@nquiringminds.com>
+ * @date 2023
+ * @copyright
+ * SPDX-FileCopyrightText: © 2023 NQMCyber Ltd and edgesec contributors
+ * SPDX-License-Identifier: Expat
+ * @brief File containing macros for compiler attributes, if they are supported.
+ *
+ * In the future, once we support C23, we can remove this header and just
+ * use C23 attributes.
+ */
+#ifndef ATTRIBUTES_H
+#define ATTRIBUTES_H
+
+#ifndef __maybe_unused
+#if defined __has_attribute
+#if __has_attribute(unused)
+/**
+ * If used before a variable, tells the compiler that variable can be unused.
+ * (e.g. does the same thing as casting to `(void)`, or `[[maybe_unused]]` in
+ * C23).
+ *
+ * @see https://clang.llvm.org/docs/AttributeReference.html#maybe-unused-unused
+ */
+#define __maybe_unused __attribute__((unused))
+#else
+#define __maybe_unused
+#endif /* __has_attribute(unused) */
+#else
+#define __maybe_unused
+#endif /* defined __has_attribute */
+#endif /* __maybe_unused */
+
+#ifdef __GNUC__
+#define STRUCT_PACKED __attribute__((packed))
+#else
+#define STRUCT_PACKED
+#endif
+
+#endif /* ATTRIBUTES_H */

src/utils/os.h

@@ -32,12 +32,6 @@
 
 #define OS_HOST_NAME_MAX 64
 
-#ifdef __GNUC__
-#define STRUCT_PACKED __attribute__((packed))
-#else
-#define STRUCT_PACKED
-#endif
-
 #ifndef BIT
 #define BIT(x) (1U << (x))
 #endif

tests/CMakeLists.txt

@@ -47,7 +47,7 @@ target_compile_definitions(test_config PRIVATE TEST_CONFIG_INI_PATH="${CMAKE_BIN
 if (USE_RADIUS_SERVICE)
   add_cmocka_test(test_edgesec
     SOURCES test_edgesec.c
-    LINK_LIBRARIES radius radius_client sockctl runctl cmocka::cmocka log config
+    LINK_LIBRARIES radius radius_client attributes sockctl runctl cmocka::cmocka log config
   )
   target_compile_definitions(test_edgesec PRIVATE TEST_CONFIG_INI_PATH="${CMAKE_BINARY_DIR}/test-config.ini")
   set_tests_properties(test_edgesec PROPERTIES TIMEOUT 10)

tests/radius/CMakeLists.txt

@@ -7,16 +7,16 @@ if (TARGET hostapd::libeap)
   add_compile_definitions(EAP_TEST_DIR="${EAP_TEST_DIR}")
 
   add_library(eap_test_peer eap_test_peer.c)
-  target_link_libraries(eap_test_peer PRIVATE hostapd::libeap)
+  target_link_libraries(eap_test_peer PRIVATE common hostapd::libeap)
   target_compile_definitions(eap_test_peer PRIVATE _DEFAULT_SOURCE _BSD_SOURCE IEEE8021X_EAPOL)
 
   add_library(eap_test_server eap_test_server.c)
-  target_link_libraries(eap_test_server PRIVATE hostapd::libeap)
+  target_link_libraries(eap_test_server PRIVATE common hostapd::libeap)
   target_compile_definitions(eap_test_server PRIVATE _DEFAULT_SOURCE _BSD_SOURCE IEEE8021X_EAPOL)
 
   add_cmocka_test(test_libeap
     SOURCES test_libeap.c
-    LINK_LIBRARIES eap_test_peer eap_test_server hostapd::libeap cmocka::cmocka
+    LINK_LIBRARIES eap_test_peer eap_test_server hostapd::libeap common cmocka::cmocka
   )
   target_compile_definitions(test_libeap PRIVATE _DEFAULT_SOURCE _BSD_SOURCE IEEE8021X_EAPOL)
 endif ()

tests/test_edgesec.c

@@ -25,6 +25,7 @@
 #include "radius/radius_client.h"
 #include "supervisor/cmd_processor.h"
 #include "supervisor/system_commands.h"
+#include "utils/attributes.h"
 #include "utils/sockctl.h"
 
 #define AP_CTRL_IFACE_PATH "/tmp/wifi0"
@@ -40,10 +41,8 @@ void log_lock_fun(bool lock) {
   }
 }
 
-void ap_eloop(int sock, void *eloop_ctx, void *sock_ctx) {
-  (void)eloop_ctx;
-  (void)sock_ctx;
-
+void ap_eloop(int sock, __maybe_unused void *eloop_ctx,
+              __maybe_unused void *sock_ctx) {
   uint32_t bytes_available = 0;
 
   assert_int_not_equal(ioctl(sock, FIONREAD, &bytes_available), -1);
@@ -83,12 +82,10 @@ void *ap_server_thread(void *arg) {
 
 /* Process the RADIUS frames from Authentication Server */
 static RadiusRxResult receive_auth(struct radius_msg *msg,
-                                   struct radius_msg *req,
-                                   const uint8_t *shared_secret,
-                                   size_t shared_secret_len, void *data) {
-  (void)req;
-  (void)shared_secret;
-  (void)shared_secret_len;
+                                   __maybe_unused struct radius_msg *req,
+                                   __maybe_unused const uint8_t *shared_secret,
+                                   __maybe_unused size_t shared_secret_len,
+                                   void *data) {
   struct eloop_data *eloop = (struct eloop_data *)data;
 
   log_trace("Received RADIUS Authentication message; code=%d",
@@ -100,8 +97,7 @@ static RadiusRxResult receive_auth(struct radius_msg *msg,
   return RADIUS_RX_PROCESSED;
 }
 
-void *supervisor_client_thread(void *arg) {
-  (void)arg;
+void *supervisor_client_thread(__maybe_unused void *arg) {
   char socket_path[MAX_OS_PATH_LEN];
   char ping_reply[] = PING_REPLY;
   rtrim(ping_reply, NULL);
@@ -211,9 +207,7 @@ void *supervisor_client_thread(void *arg) {
 /**
  * @brief Performs an integration test on edgesec
  */
-static void test_edgesec(void **state) {
-  (void)state; /* unused */
-
+static void test_edgesec(__maybe_unused void **state) {
   struct app_config config = {0};
 
   assert_int_equal(load_app_config(TEST_CONFIG_INI_PATH, &config), 0);
@@ -245,10 +239,7 @@ static void test_edgesec(void **state) {
   pthread_mutex_destroy(&log_lock);
 }
 
-int main(int argc, char *argv[]) {
-  (void)argc;
-  (void)argv;
-
+int main(__maybe_unused int argc, __maybe_unused char *argv[]) {
   log_set_quiet(false);
   log_set_lock(log_lock_fun);