From 5a3a8bf22e4976c8fe9ff37daa1e221fd299da01 Mon Sep 17 00:00:00 2001
From: Alois Klink <alois@nquiringminds.com>
Date: Fri, 10 Feb 2023 13:15:14 +0000
Subject: [PATCH 1/6] style(common): add `__maybe_unused` attribute

Add the `__maybe_unused` attribute, which prevents warnings when a
variable is unused
(e.g. if we use the variable only if a given `#ifdef X` is set).

In the edgesec project, we normally use a cast to `(void)` to do the
same thing, e.g. `(void)my_var;`, but code taken from hostap
uses `__maybe_unused`.

This is equivalent to the C23 attribute [`[[maybe_unused]]`][1].

[1]: https://en.cppreference.com/w/c/language/attributes/maybe_unused
---
 src/radius/common.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/src/radius/common.h b/src/radius/common.h
index 4175006bd..1b849c39a 100644
--- a/src/radius/common.h
+++ b/src/radius/common.h
@@ -41,6 +41,24 @@ typedef int8_t s8;
 #define __bitwise
 #endif
 
+#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)`).
+ *
+ * @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 */
+
 typedef u16 __bitwise be16;
 typedef u16 __bitwise le16;
 typedef u32 __bitwise be32;

From 8147696cf95f503ff236bf802557a09b1af26854 Mon Sep 17 00:00:00 2001
From: Alois Klink <alois@nquiringminds.com>
Date: Mon, 20 Feb 2023 18:19:51 +0000
Subject: [PATCH 2/6] build(common): add CMake def for `common.h`

Add a CMake INTERFACE/header-only library for `src/radius/common.h`
---
 src/radius/CMakeLists.txt   | 10 ++++++++--
 tests/radius/CMakeLists.txt |  6 +++---
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/src/radius/CMakeLists.txt b/src/radius/CMakeLists.txt
index 571943869..bd6b82209 100644
--- a/src/radius/CMakeLists.txt
+++ b/src/radius/CMakeLists.txt
@@ -2,6 +2,10 @@ include_directories (
   "${PROJECT_SOURCE_DIR}/src"
 )
 
+add_library(common INTERFACE)
+set_target_properties(common PROPERTIES PUBLIC_HEADER "common.h")
+target_link_libraries(common INTERFACE allocs log)
+
 add_library(md5_internal md5_internal.c)
 target_link_libraries(md5_internal PRIVATE log os)
 
@@ -9,12 +13,14 @@ 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
+  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)
diff --git a/tests/radius/CMakeLists.txt b/tests/radius/CMakeLists.txt
index d1e2ea167..4abc285d8 100644
--- a/tests/radius/CMakeLists.txt
+++ b/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 ()

From 5f03e137ea84d9752a89c60784c66c30b0da0fea Mon Sep 17 00:00:00 2001
From: Alois Klink <alois@nquiringminds.com>
Date: Mon, 20 Feb 2023 18:08:27 +0000
Subject: [PATCH 3/6] refactor: add header file for compiler attributes

Add `./src/utils/attributes.h` header file for compiler attributes.

For example, `__attribute__((unused))` is supported on GCC and Clang,
but on other compilers, this is not supported.

Most of these are already in `src/radius/common.h`, so we just need to
move them into a new file to use them in the rest of the edgesec code.
---
 src/radius/CMakeLists.txt |  2 +-
 src/utils/CMakeLists.txt  |  3 +++
 src/utils/attributes.h    | 16 ++++++++++++++++
 3 files changed, 20 insertions(+), 1 deletion(-)
 create mode 100644 src/utils/attributes.h

diff --git a/src/radius/CMakeLists.txt b/src/radius/CMakeLists.txt
index bd6b82209..0dfc07b19 100644
--- a/src/radius/CMakeLists.txt
+++ b/src/radius/CMakeLists.txt
@@ -4,7 +4,7 @@ include_directories (
 
 add_library(common INTERFACE)
 set_target_properties(common PROPERTIES PUBLIC_HEADER "common.h")
-target_link_libraries(common INTERFACE allocs log)
+target_link_libraries(common INTERFACE attributes allocs log)
 
 add_library(md5_internal md5_internal.c)
 target_link_libraries(md5_internal PRIVATE log os)
diff --git a/src/utils/CMakeLists.txt b/src/utils/CMakeLists.txt
index b48f3f40e..cff068fbf 100644
--- a/src/utils/CMakeLists.txt
+++ b/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)
diff --git a/src/utils/attributes.h b/src/utils/attributes.h
new file mode 100644
index 000000000..6418bb43a
--- /dev/null
+++ b/src/utils/attributes.h
@@ -0,0 +1,16 @@
+/**
+ * @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
+
+#endif /* ATTRIBUTES_H */

From e07fed5eee363f73b32cb9078a37cec6f2a85d8c Mon Sep 17 00:00:00 2001
From: Alois Klink <alois@nquiringminds.com>
Date: Mon, 20 Feb 2023 18:10:41 +0000
Subject: [PATCH 4/6] refactor: move `__maybe_unused` to attributes.h

Move `__maybe_unused` from the `src/radius/common.h` file into
`src/utils/attributes.h`, so that we can use it in the rest of
the edgesec project.
---
 src/radius/common.h    | 19 +------------------
 src/utils/attributes.h | 19 +++++++++++++++++++
 2 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/src/radius/common.h b/src/radius/common.h
index 1b849c39a..d1f313ebd 100644
--- a/src/radius/common.h
+++ b/src/radius/common.h
@@ -15,6 +15,7 @@
 
 #include <stddef.h>
 
+#include "../utils/attributes.h"
 #include "utils/allocs.h"
 #include "utils/log.h"
 
@@ -41,24 +42,6 @@ typedef int8_t s8;
 #define __bitwise
 #endif
 
-#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)`).
- *
- * @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 */
-
 typedef u16 __bitwise be16;
 typedef u16 __bitwise le16;
 typedef u32 __bitwise be32;
diff --git a/src/utils/attributes.h b/src/utils/attributes.h
index 6418bb43a..75c066037 100644
--- a/src/utils/attributes.h
+++ b/src/utils/attributes.h
@@ -13,4 +13,23 @@
 #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 */
+
 #endif /* ATTRIBUTES_H */

From 0be7a2bb2a29fc6a54ba1df030ffc63a07191852 Mon Sep 17 00:00:00 2001
From: Alois Klink <alois@nquiringminds.com>
Date: Mon, 20 Feb 2023 18:28:00 +0000
Subject: [PATCH 5/6] test(edgesec): use __maybe_unused, not (void) cast

Use the `__maybe_unused` compiler attribute to mark unused function
parameters in `tests/test_edgesec.c` as unused, instead of `(void)`.

For some reason, code coverage sometimes thinks that `(void)` lines
are not covered by tests. Using `__maybe_unused` instead should
fix this issue.
---
 tests/CMakeLists.txt |  2 +-
 tests/test_edgesec.c | 29 ++++++++++-------------------
 2 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index bf17fcb8c..b0a49017a 100644
--- a/tests/CMakeLists.txt
+++ b/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)
diff --git a/tests/test_edgesec.c b/tests/test_edgesec.c
index 4f7f7f95f..f630d63de 100644
--- a/tests/test_edgesec.c
+++ b/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);
 

From f770f44fc08594d57f6feb36b5fdfe09afb69049 Mon Sep 17 00:00:00 2001
From: Alois Klink <alois@nquiringminds.com>
Date: Tue, 21 Feb 2023 10:51:33 +0000
Subject: [PATCH 6/6] refactor: move STRUCT_PACKED into attributes

Move the `#define STRUCT_PACKED __attribute__((packed))` from
`src/utils/os.h` into `src/utils/attributes.h`.

Co-authored-by: Alexandru Mereacre <mereacre@gmail.com>
---
 src/CMakeLists.txt                                         | 2 +-
 src/capture/middlewares/header_middleware/CMakeLists.txt   | 2 +-
 src/capture/middlewares/header_middleware/packet_decoder.h | 1 +
 src/edgesec-recap.c                                        | 1 +
 src/radius/CMakeLists.txt                                  | 2 +-
 src/radius/radius.h                                        | 1 +
 src/utils/attributes.h                                     | 6 ++++++
 src/utils/os.h                                             | 6 ------
 8 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 10cba57a4..f2d4db9ce 100644
--- a/src/CMakeLists.txt
+++ b/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()
diff --git a/src/capture/middlewares/header_middleware/CMakeLists.txt b/src/capture/middlewares/header_middleware/CMakeLists.txt
index 2ea04d680..14c3a4d09 100644
--- a/src/capture/middlewares/header_middleware/CMakeLists.txt
+++ b/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)
diff --git a/src/capture/middlewares/header_middleware/packet_decoder.h b/src/capture/middlewares/header_middleware/packet_decoder.h
index ee3826bfa..f19bbb2b3 100644
--- a/src/capture/middlewares/header_middleware/packet_decoder.h
+++ b/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"
 
diff --git a/src/edgesec-recap.c b/src/edgesec-recap.c
index e93e0d99e..215239d47 100644
--- a/src/edgesec-recap.c
+++ b/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"
diff --git a/src/radius/CMakeLists.txt b/src/radius/CMakeLists.txt
index 0dfc07b19..e65b0e553 100644
--- a/src/radius/CMakeLists.txt
+++ b/src/radius/CMakeLists.txt
@@ -19,7 +19,7 @@ target_compile_definitions(wpabuf PUBLIC _DEFAULT_SOURCE _BSD_SOURCE)
 
 add_library(radius radius.c)
 target_link_libraries(radius
-  PUBLIC common
+  PUBLIC common attributes
   PRIVATE wpabuf md5 md5_internal log os)
 
 add_library(radius_server radius_server.c)
diff --git a/src/radius/radius.h b/src/radius/radius.h
index d8cf15f3f..364e89451 100644
--- a/src/radius/radius.h
+++ b/src/radius/radius.h
@@ -16,6 +16,7 @@
 
 #include "common.h"
 
+#include "../utils/attributes.h"
 #include "utils/os.h"
 
 /* RFC 2865 - RADIUS */
diff --git a/src/utils/attributes.h b/src/utils/attributes.h
index 75c066037..b9a7576de 100644
--- a/src/utils/attributes.h
+++ b/src/utils/attributes.h
@@ -32,4 +32,10 @@
 #endif /* defined __has_attribute */
 #endif /* __maybe_unused */
 
+#ifdef __GNUC__
+#define STRUCT_PACKED __attribute__((packed))
+#else
+#define STRUCT_PACKED
+#endif
+
 #endif /* ATTRIBUTES_H */
diff --git a/src/utils/os.h b/src/utils/os.h
index 6232d9e69..94524559c 100644
--- a/src/utils/os.h
+++ b/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