Repository: eclipse-bluechi/bluechi (language: python)

Issue:
Fedora rpminspect complains about the usage of inet_ntoa
Fedora rpminstpect complains, that using `inet_ntoa` function is forbidden:

```json
 "badfuncs": [
    {
      "result": "VERIFY",
      "waiver authorization": "Anyone",
      "message": "/usr/bin/hirte-agent may use forbidden functions on i686",
      "details": "Forbidden function symbols found:\n\tinet_ntoa\n",
      "remedy": "Forbidden symbols were found in an ELF file in the package.  The configuration settings for rpminspect indicate the named symbols are forbidden in packages.  If this is deliberate, you may want to disable the badfuncs inspection.  If it is not deliberate, check the man pages for the named symbols to see what API functions have replaced the forbidden symbols.  Usually a function is marked as deprecated but still provided in order to allow for backwards compatibility.  Whenever possible the deprecated functions should not be used."
    },
    {
      "result": "VERIFY",
      "waiver authorization": "Anyone",
      "message": "/usr/bin/hirte-agent may use forbidden functions on x86_64",
      "details": "Forbidden function symbols found:\n\tinet_ntoa\n",
      "remedy": "Forbidden symbols were found in an ELF file in the package.  The configuration settings for rpminspect indicate the named symbols are forbidden in packages.  If this is deliberate, you may want to disable the badfuncs inspection.  If it is not deliberate, check the man pages for the named symbols to see what API functions have replaced the forbidden symbols.  Usually a function is marked as deprecated but still provided in order to allow for backwards compatibility.  Whenever possible the deprecated functions should not be used."
    },
    {
      "result": "VERIFY",
      "waiver authorization": "Anyone",
      "message": "/usr/bin/hirte-agent may use forbidden functions on aarch64",
      "details": "Forbidden function symbols found:\n\tinet_ntoa\n",
      "remedy": "Forbidden symbols were found in an ELF file in the package.  The configuration settings for rpminspect indicate the named symbols are forbidden in packages.  If this is deliberate, you may want to disable the badfuncs inspection.  If it is not deliberate, check the man pages for the named symbols to see what API functions have replaced the forbidden symbols.  Usually a function is marked as deprecated but still provided in order to allow for backwards compatibility.  Whenever possible the deprecated functions should not be used."
    },
    {
      "result": "VERIFY",
      "waiver authorization": "Anyone",
      "message": "/usr/bin/hirte-agent may use forbidden functions on ppc64le",
      "details": "Forbidden function symbols found:\n\tinet_ntoa\n",
      "remedy": "Forbidden symbols were found in an ELF file in the package.  The configuration settings for rpminspect indicate the named symbols are forbidden in packages.  If this is deliberate, you may want to disable the badfuncs inspection.  If it is not deliberate, check the man pages for the named symbols to see what API functions have replaced the forbidden symbols.  Usually a function is marked as deprecated but still provided in order to allow for backwards compatibility.  Whenever possible the deprecated functions should not be used."
    },
    {
      "result": "VERIFY",
      "waiver authorization": "Anyone",
      "message": "/usr/bin/hirte-agent may use forbidden functions on s390x",
      "details": "Forbidden function symbols found:\n\tinet_ntoa\n",
      "remedy": "Forbidden symbols were found in an ELF file in the package.  The configuration settings for rpminspect indicate the named symbols are forbidden in packages.  If this is deliberate, you may want to disable the badfuncs inspection.  If it is not deliberate, check the man pages for the named symbols to see what API functions have replaced the forbidden symbols.  Usually a function is marked as deprecated but still provided in order to allow for backwards compatibility.  Whenever possible the deprecated functions should not be used."
    }
```


Discussion / hints:

Fix patch:
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index 7c40e8da16..684a91abbf 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -9,6 +9,7 @@ on:
 
 jobs:
   lint:
+    name: Lint and Test
     runs-on: ubuntu-latest
     container:
       image: quay.io/centos/centos:stream9
@@ -32,6 +33,7 @@ jobs:
                   meson \
                   systemd-devel \
                   codespell \
+                  valgrind \
               -y
 
       - name: Checkout sources
@@ -59,6 +61,20 @@ jobs:
 
       - name: Running markdown lint
         uses: DavidAnson/markdownlint-cli2-action@v9
+      
+      - name: Building hirte
+        run: |
+          make build
+
+      - name: Running unit tests
+        run: |
+          make test-with-valgrind
+
+      - name: Upload unit test logs
+        uses: actions/upload-artifact@v3
+        with:
+          name: unit-test-logs
+          path: ./builddir/meson-logs/testlog-valgrind.txt
 
   rpmbuild:
     needs: lint
diff --git a/Makefile b/Makefile
index 2521224522..6096a53da7 100644
--- a/Makefile
+++ b/Makefile
@@ -13,9 +13,11 @@ build:
 	meson compile -C builddir
 
 test: build
-	meson compile -C builddir
 	meson test -C builddir
 
+test-with-valgrind: build
+	meson test --wrap='valgrind --leak-check=full --error-exitcode=1 --track-origins=yes' -C builddir
+
 install: build
 	meson install -C builddir --destdir "$(DESTDIR)"
 
diff --git a/src/libhirte/bus/bus.c b/src/libhirte/bus/bus.c
index ad98b03a17..efe2c69a3f 100644
--- a/src/libhirte/bus/bus.c
+++ b/src/libhirte/bus/bus.c
@@ -12,14 +12,21 @@
  *** peer dbus ***
  *****************/
 
+static char *typesafe_inet_ntop4(const struct sockaddr_in *addr) {
+        char *dst = malloc0_array(0, sizeof(char), INET_ADDRSTRLEN);
+        inet_ntop(AF_INET, &addr->sin_addr, dst, INET_ADDRSTRLEN);
+        return dst;
+}
+
 char *assemble_tcp_address(const struct sockaddr_in *addr) {
         if (addr == NULL) {
                 hirte_log_error("Can not assemble an empty address");
                 return NULL;
         }
 
+        _cleanup_free_ char *host = typesafe_inet_ntop4(addr);
         char *dbus_addr = NULL;
-        int r = asprintf(&dbus_addr, "tcp:host=%s,port=%d", inet_ntoa(addr->sin_addr), ntohs(addr->sin_port));
+        int r = asprintf(&dbus_addr, "tcp:host=%s,port=%d", host, ntohs(addr->sin_port));
         if (r < 0) {
                 hirte_log_error("Out of memory");
                 return NULL;
diff --git a/src/libhirte/bus/bus.h b/src/libhirte/bus/bus.h
index a27fb23787..82e3a28fbc 100644
--- a/src/libhirte/bus/bus.h
+++ b/src/libhirte/bus/bus.h
@@ -4,6 +4,7 @@
 #include <netinet/in.h>
 #include <systemd/sd-bus.h>
 
+
 char *assemble_tcp_address(const struct sockaddr_in *addr);
 sd_bus *peer_bus_open(sd_event *event, const char *dbus_description, const char *dbus_server_addr);
 int peer_bus_close(sd_bus *peer_dbus);


Test patch:
diff --git a/src/libhirte/test/bus/bus_test.c b/src/libhirte/test/bus/bus_test.c
new file mode 100644
index 0000000000..79c7539b5e
--- /dev/null
+++ b/src/libhirte/test/bus/bus_test.c
@@ -0,0 +1,80 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#include <arpa/inet.h>
+#include <assert.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "libhirte/bus/bus.h"
+#include "libhirte/common/common.h"
+
+
+#ifndef __FUNCTION_NAME__
+#        define __FUNCTION_NAME__ __func__
+#endif
+
+int build_sockaddr_in(const char *host_in, uint16_t port_in, struct sockaddr_in *ret) {
+        int r = 0;
+
+        memset(ret, 0, sizeof(*ret));
+        ret->sin_family = AF_INET;
+        ret->sin_port = htons(port_in);
+
+        r = inet_pton(AF_INET, host_in, &ret->sin_addr);
+        if (r < 1) {
+                fprintf(stdout, "Invalid host option '%s'\n", host_in);
+                return r;
+        }
+
+        return 0;
+}
+
+
+bool test_assemble_tcp_address(const struct sockaddr_in *input, const char *expectedResult) {
+        _cleanup_free_ char *result = assemble_tcp_address(input);
+        if ((result == NULL && expectedResult != NULL) || (result != NULL && !streq(result, expectedResult))) {
+                _cleanup_free_ char *msg = NULL;
+                int r = asprintf(
+                                &msg,
+                                "FAILED: %s\n\tExpected: %s\n\tGot: %s\n",
+                                __FUNCTION_NAME__,
+                                expectedResult,
+                                result);
+                if (r < 0) {
+                        fprintf(stderr, "OOM: %s failed", __FUNCTION_NAME__);
+                        return false;
+                }
+                if (msg != NULL) {
+                        fprintf(stderr, "%s", msg);
+                        return false;
+                }
+        }
+        return true;
+}
+
+int main() {
+        bool result = true;
+        struct sockaddr_in host;
+
+        result = result && test_assemble_tcp_address(NULL, NULL);
+
+        build_sockaddr_in("127.0.0.1", 808, &host);
+        result = result && test_assemble_tcp_address(&host, "tcp:host=127.0.0.1,port=808");
+
+        build_sockaddr_in("192.168.178.1", 808, &host);
+        result = result && test_assemble_tcp_address(&host, "tcp:host=192.168.178.1,port=808");
+
+        build_sockaddr_in("", 808, &host);
+        result = result && test_assemble_tcp_address(&host, "tcp:host=0.0.0.0,port=808");
+
+        build_sockaddr_in("I am invalid", 808, &host);
+        result = result && test_assemble_tcp_address(&host, "tcp:host=0.0.0.0,port=808");
+
+        build_sockaddr_in("I am invalid", -1, &host);
+        result = result && test_assemble_tcp_address(&host, "tcp:host=0.0.0.0,port=65535");
+
+        if (!result) {
+                return EXIT_FAILURE;
+        }
+        return EXIT_SUCCESS;
+}
diff --git a/src/libhirte/test/bus/meson.build b/src/libhirte/test/bus/meson.build
new file mode 100644
index 0000000000..7aa85cbd3b
--- /dev/null
+++ b/src/libhirte/test/bus/meson.build
@@ -0,0 +1,15 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+bus_src = [
+  'bus_test'
+]
+
+foreach src : bus_src
+  exec_test = executable(src, src + '.c',
+    link_with: [
+      hirte_lib,
+    ],
+    include_directories: include_directories('../../..'),
+  )
+  test(src, exec_test)
+endforeach
diff --git a/src/libhirte/test/config_test.c b/src/libhirte/test/common/cfg_test.c
similarity index 88%
rename from src/libhirte/test/config_test.c
rename to src/libhirte/test/common/cfg_test.c
index d2a2cedb4d..dd73920390 100644
--- a/src/libhirte/test/config_test.c
+++ b/src/libhirte/test/common/cfg_test.c
@@ -28,22 +28,18 @@ void _config_set_and_get(
 }
 
 void test_config_set_get() {
-    struct config *config = NULL;
-    cfg_initialize(&config);
+        struct config *config = NULL;
+        cfg_initialize(&config);
 
-    _config_set_and_get(config, "key1", "value1", "value1", false);
-    _config_set_and_get(config, "key2", NULL, NULL, true);
-    _config_set_and_get(config, "key1", "value3", "value3", false);
-    _config_set_and_get(config, "key2", "value4", "value4", false);
+        _config_set_and_get(config, "key1", "value1", "value1", false);
+        _config_set_and_get(config, "key2", NULL, NULL, true);
+        _config_set_and_get(config, "key1", "value3", "value3", false);
+        _config_set_and_get(config, "key2", "value4", "value4", false);
 
-    cfg_dispose(config);
+        cfg_dispose(config);
 }
 
-void _config_set_get_bool(
-                    struct config *config,
-                    const char *option,
-                    const char *value,
-                    bool expected_value) {
+void _config_set_get_bool(struct config *config, const char *option, const char *value, bool expected_value) {
         assert(option != NULL);
 
         assert(cfg_set_value(config, option, value) == 0);
@@ -139,11 +135,11 @@ void test_non_existent_option() {
 
 void test_parse_config_from_file() {
         const char *cfg_file_content =
-                "key1 = value1\n"
-                "key2 = value 2\n"
-                "# key3 = ignored\n"
-                "key4 = value 4\n"
-                "key5=\n";
+                        "key1 = value1\n"
+                        "key2 = value 2\n"
+                        "# key3 = ignored\n"
+                        "key4 = value 4\n"
+                        "key5=\n";
 
 
         // check if tmp file was created and filled-in successfully
@@ -176,13 +172,13 @@ void test_parse_config_from_file() {
 
 void test_parse_sectioned_config_from_file() {
         const char *cfg_file_content =
-                "key = value1\n"
-                "[section2]\n"
-                "key = value 2\n"
-                "# key = ignored\n"
-                "[section 3]\n"
-                "# [ignored section]\n"
-                "key = value 3\n";
+                        "key = value1\n"
+                        "[section2]\n"
+                        "key = value 2\n"
+                        "# key = ignored\n"
+                        "[section 3]\n"
+                        "# [ignored section]\n"
+                        "key = value 3\n";
 
 
         // check if tmp file was created and filled-in successfully
@@ -245,7 +241,8 @@ void test_default_section() {
         assert(strcmp(cfg_s_get_value(config, old_default_section, KEY), OLD_SECTION_VALUE1) == 0);
 
         // Change value in old_default_section and check the value didn't change in NEW_DEFAULT_SECTION
-        _config_section_set_and_get(config, old_default_section, KEY, OLD_SECTION_VALUE2, OLD_SECTION_VALUE2, false);
+        _config_section_set_and_get(
+                        config, old_default_section, KEY, OLD_SECTION_VALUE2, OLD_SECTION_VALUE2, false);
         assert(strcmp(cfg_s_get_value(config, NEW_DEFAULT_SECTION, KEY), NEW_SECTION_VALUE) == 0);
 
         free(old_default_section);
@@ -253,9 +250,9 @@ void test_default_section() {
 }
 
 void test_parse_config_from_env() {
-        const char* LOG_LEVEL_DEBUG = "DEBUG";
-        const char* LOG_TARGET_STDERR = "stderr";
-        const char* LOG_IS_QUIET_OFF = "false";
+        const char *LOG_LEVEL_DEBUG = "DEBUG";
+        const char *LOG_TARGET_STDERR = "stderr";
+        const char *LOG_IS_QUIET_OFF = "false";
 
         struct config *config = NULL;
         cfg_initialize(&config);
@@ -287,10 +284,10 @@ void test_parse_config_from_env() {
 }
 
 void test_env_override_config() {
-        const char* LOG_LEVEL_DEBUG = "DEBUG";
-        const char* LOG_LEVEL_INFO = "INFO";
-        const char* LOG_TARGET_STDERR = "stderr";
-        const char* LOG_IS_QUIET_ON = "on";
+        const char *LOG_LEVEL_DEBUG = "DEBUG";
+        const char *LOG_LEVEL_INFO = "INFO";
+        const char *LOG_TARGET_STDERR = "stderr";
+        const char *LOG_IS_QUIET_ON = "on";
 
         struct config *config = NULL;
         cfg_initialize(&config);
diff --git a/src/libhirte/test/list_test.c b/src/libhirte/test/common/list_test.c
similarity index 100%
rename from src/libhirte/test/list_test.c
rename to src/libhirte/test/common/list_test.c
diff --git a/src/libhirte/test/common/meson.build b/src/libhirte/test/common/meson.build
new file mode 100644
index 0000000000..adbfd938ec
--- /dev/null
+++ b/src/libhirte/test/common/meson.build
@@ -0,0 +1,17 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+common_src = [
+  'cfg_test',
+  'list_test',
+  'parse-util_test'
+]
+
+foreach src : common_src
+  exec_test = executable(src, src + '.c',
+    link_with: [
+      hirte_lib,
+    ],
+    include_directories: include_directories('../../..'),
+  )
+  test(src, exec_test)
+endforeach
diff --git a/src/libhirte/test/parse-util_test.c b/src/libhirte/test/common/parse-util_test.c
similarity index 96%
rename from src/libhirte/test/parse-util_test.c
rename to src/libhirte/test/common/parse-util_test.c
index b7afc4b67a..1c9a3b13bc 100644
--- a/src/libhirte/test/parse-util_test.c
+++ b/src/libhirte/test/common/parse-util_test.c
@@ -4,6 +4,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 
+#include "libhirte/common/common.h"
 #include "libhirte/common/parse-util.h"
 
 
@@ -12,7 +13,7 @@
 #endif
 
 bool test_parse_long(const char *input, bool expectedReturn, long expectedResult) {
-        char *msg = NULL;
+        _cleanup_free_ char *msg = NULL;
 
         long res = 0;
         int ret = parse_long(input, &res);
@@ -35,7 +36,7 @@ bool test_parse_long(const char *input, bool expectedReturn, long expectedResult
 }
 
 bool test_parse_port(const char *input, bool expected_return, uint16_t expected_result) {
-        char *msg = NULL;
+        _cleanup_free_ char *msg = NULL;
 
         uint16_t res = 0;
         bool ret = parse_port(input, &res);
diff --git a/src/libhirte/test/meson.build b/src/libhirte/test/meson.build
index e04951cc35..9a8af3ddbd 100644
--- a/src/libhirte/test/meson.build
+++ b/src/libhirte/test/meson.build
@@ -1,37 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0-or-later
 
-# common test configuration
-
-config_src = [
-  'config_test.c'
-]
-exec_config_test = executable('config_test', config_src,
-  link_with: [
-    hirte_lib,
-  ],
-  include_directories: include_directories('../..'),
-)
-test('config_test', exec_config_test)
-
-list_src = [
-  'list_test.c'
-]
-exec_list_test = executable('list_test', list_src,
-  link_with: [
-    hirte_lib,
-  ],
-  include_directories: include_directories('../..'),
-)
-test('list_test', exec_list_test)
-
-parse_util_src = [
-  'parse-util_test.c'
-]
-
-e = executable('parse-util_test', parse_util_src,
-  link_with: [
-    hirte_lib,
-  ],
-  include_directories: include_directories('../..'),
-)
-test('parse-util_test', e)
+subdir('bus')
+subdir('common')
\ No newline at end of file