Adding g_strnjoin() with units tests

.cirrus.yml

@@ -6,7 +6,7 @@ FreeBSD_task:
     freebsd_instance:
       image_family: freebsd-12-2
   prepare_script:
-    - pkg install -y $SSL git autoconf automake libtool pkgconf opus jpeg-turbo fdk-aac pixman libX11 libXfixes libXrandr nasm fusefs-libs
+    - pkg install -y $SSL git autoconf automake libtool pkgconf opus jpeg-turbo fdk-aac pixman libX11 libXfixes libXrandr nasm fusefs-libs check
     - git submodule update --init --recursive
   configure_script:
     - ./bootstrap

.gitignore

@@ -42,7 +42,9 @@ sesman/xrdp-sesman
 sesman/sesman.ini
 *.so
 stamp-h1
+tap-driver.sh
 test-driver
+tests/common/test_common
 tests/memtest/memtest
 tools/devel/tcp_proxy/tcp_proxy
 *.trs

common/string_calls.c

@@ -30,6 +30,7 @@
 #include "log.h"
 #include "os_calls.h"
 #include "string_calls.h"
+#include "defines.h"
 
 unsigned int
 g_format_info_string(char *dest, unsigned int len,
@@ -806,3 +807,50 @@ g_strtrim(char *str, int trim_flags)
     free(text1);
     return 0;
 }
+
+/*****************************************************************************/
+char *
+g_strnjoin(char *dest, int dest_len, const char *joiner, const char *src[], int src_len)
+{
+    int len = 0;
+    int joiner_len;
+    int i = 0;
+    int dest_remaining;
+    char *dest_pos = dest;
+    char *dest_end;
+    
+    if (dest == NULL || dest_len < 1)
+    {
+        return dest;
+    }
+    if (src == NULL || src_len < 1)
+    {
+        dest[0] = '\0';
+        return dest;
+    }
+
+    dest[0] = '\0';
+    dest_end = dest + dest_len - 1;
+    joiner_len = g_strlen(joiner);
+    for (i = 0; i < src_len - 1 && dest_pos < dest_end; i++)
+    {
+        len = g_strlen(src[i]);
+        dest_remaining = dest_end - dest_pos;
+        g_strncat(dest_pos, src[i], dest_remaining);
+        dest_pos += MIN(len, dest_remaining);
+
+        if (dest_pos < dest_end)
+        {
+            dest_remaining = dest_end - dest_pos;
+            g_strncat(dest_pos, joiner, dest_remaining);
+            dest_pos += MIN(joiner_len, dest_remaining);
+        }
+    }
+
+    if (i == src_len - 1 && dest_pos < dest_end)
+    {
+        g_strncat(dest_pos, src[i], dest_end - dest_pos);
+    }
+
+    return dest;
+}

common/string_calls.h

@@ -80,6 +80,33 @@ g_bool2text(int value);
 int
 g_text2bool(const char *s);
 
+/**
+ * Joins an array of strings into a single string.
+ *
+ * Note: The joiner is placed between each source string. The joiner is not
+ *      placed after the last source string. If there is only one source string,
+ *      then the result string will be equal to the source string.
+ *
+ * Note: any content that is present in dest will be overwritten with the new
+ *      joined string.
+ *
+ * Note: If the destination array is not large enough to hold the entire
+ *      contents of the joined string, then the joined string will be truncated
+ *      to fit in the destination array.
+ *
+ * @param[out] dest The destination array to write the joined string into.
+ * @param[in] dest_len The max number of characters to write to the destination
+ *      array including the terminating null. Must be > 0
+ * @param[in] joiner The string to concatenate between each source string.
+ *      The joiner string may be NULL which is processed as a zero length string.
+ * @param[in] src An array of strings to join. The array must be non-null.
+ *      Array items may be NULL and are processed as zero length strings.
+ * @param[in] src_len The number of strings to join in the src array. Must be > 0
+ * @return A pointer to the begining of the joined string (ie. returns dest).
+ */
+char *
+g_strnjoin(char *dest, int dest_len, const char *joiner, const char *src[], int src_len);
+
 /**
  * Converts a binary array into a hux dump suitable for displaying to a user.
  *

configure.ac

@@ -357,6 +357,11 @@ AC_CHECK_HEADER([X11/extensions/Xrandr.h], [],
 
 CFLAGS="$save_CFLAGS"
 
+# checking for libcheck (used for unit testing)
+PKG_CHECK_MODULES([CHECK], [check >= 0.10.0])
+AC_CHECK_HEADER([check.h], [],
+  [AC_MSG_ERROR([please install check])])
+
 AC_SUBST([moduledir], '${libdir}/xrdp')
 
 AC_ARG_ENABLE([strict-locations],
@@ -398,6 +403,7 @@ AC_CONFIG_FILES([
   sesman/Makefile
   sesman/tools/Makefile
   tests/Makefile
+  tests/common/Makefile
   tests/memtest/Makefile
   tools/Makefile
   tools/devel/Makefile
@@ -409,6 +415,7 @@ AC_CONFIG_FILES([
   xup/Makefile
 ])
 
+AC_REQUIRE_AUX_FILE([tap-driver.sh])
 AC_OUTPUT
 
 echo ""

scripts/install_xrdp_build_dependencies_with_apt.sh

@@ -26,7 +26,8 @@ PACKAGES=" \
     libtool \
     make \
     nasm \
-    pkg-config\
+    pkg-config \
+    check \
     "
 
 case "$ARCH"

tests/Makefile.am

@@ -3,4 +3,5 @@ EXTRA_DIST = \
   readme.txt
 
 SUBDIRS = \
+  common \
   memtest 

tests/common/Makefile.am

@@ -0,0 +1,23 @@
+
+AM_CPPFLAGS = \
+  -I$(top_builddir) \
+  -I$(top_srcdir)/common
+
+if XRDP_DEBUG
+AM_CPPFLAGS += -DXRDP_DEBUG
+endif
+
+LOG_DRIVER = env AM_TAP_AWK='$(AWK)' $(SHELL) \
+                  $(top_srcdir)/tap-driver.sh
+
+TESTS = test_common
+check_PROGRAMS = test_common
+
+test_common_SOURCES = \
+    test_common.h \
+    test_common_main.c \
+    test_string_calls.c
+
+test_common_LDADD = \
+    $(top_builddir)/common/libcommon.la \
+    @CHECK_LIBS@

tests/common/test_common.h

@@ -0,0 +1,9 @@
+
+#ifndef TEST_COMMON_H
+#define TEST_COMMON_H
+
+#include <check.h>
+
+Suite *make_suite_test_string(void);
+
+#endif /* TEST_COMMON_H */

tests/common/test_common_main.c

@@ -0,0 +1,23 @@
+
+#if defined(HAVE_CONFIG_H)
+#include "config_ac.h"
+#endif
+
+#include <stdlib.h>
+#include <check.h>
+#include "test_common.h"
+
+int main (void)
+{
+    int number_failed;
+    SRunner *sr;
+
+    sr = srunner_create (make_suite_test_string());
+    //   srunner_add_suite(sr, make_list_suite());
+
+    srunner_set_tap(sr, "-");
+    srunner_run_all (sr, CK_ENV);
+    number_failed = srunner_ntests_failed(sr);
+    srunner_free(sr);
+    return (number_failed == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
+}

tests/common/test_string_calls.c

@@ -0,0 +1,240 @@
+
+#if defined(HAVE_CONFIG_H)
+#include "config_ac.h"
+#endif
+
+#include "test_common.h"
+#include "string_calls.h"
+
+#define RESULT_LEN 1024
+
+START_TEST(test_strnjoin__when_src_is_null__returns_empty_string)
+{
+    /* setup */
+
+    const char **src = NULL;
+    char result[RESULT_LEN];
+    result[0] = '\0';
+
+    /* test */
+
+    g_strnjoin(result, RESULT_LEN, " ", src, 0);
+
+    /* verify */
+
+    ck_assert_str_eq(result, "");
+}
+END_TEST
+
+START_TEST(test_strnjoin__when_src_has_null_item__returns_joined_string)
+{
+    /* setup */
+
+    const char *test_value = "test_value";
+    const char *src[] = { test_value, NULL };
+    char result[RESULT_LEN];
+    result[0] = '\0';
+
+    /* test */
+
+    g_strnjoin(result, RESULT_LEN, " ", src, 2);
+
+    /* verify */
+
+    ck_assert_str_eq(result, "test_value ");
+}
+END_TEST
+
+START_TEST(test_strnjoin__when_src_has_one_item__returns_copied_source_string)
+{
+    /* setup */
+
+    const char *expected_value = "test_value";
+    const char *src[] = { expected_value };
+    char result[RESULT_LEN];
+    result[0] = '\0';
+
+    /* test */
+
+    g_strnjoin(result, RESULT_LEN, " ", src, 1);
+
+    /* verify */
+
+    ck_assert_str_eq(result, expected_value);
+}
+END_TEST
+
+START_TEST(test_strnjoin__when_src_has_two_items__returns_joined_string)
+{
+    /* setup */
+
+    const char *test_value = "test_value";
+    const char *src[] = { test_value, test_value };
+    char result[RESULT_LEN];
+    result[0] = '\0';
+
+    /* test */
+
+    g_strnjoin(result, RESULT_LEN, " ", src, 2);
+
+    /* verify */
+
+    ck_assert_str_eq(result, "test_value test_value");
+}
+END_TEST
+
+START_TEST(test_strnjoin__when_joiner_is_empty_string__returns_joined_string)
+{
+    /* setup */
+
+    const char *test_value = "test_value";
+    const char *src[] = { test_value, test_value };
+    char result[RESULT_LEN];
+    result[0] = '\0';
+
+    /* test */
+
+    g_strnjoin(result, RESULT_LEN, "", src, 2);
+
+    /* verify */
+
+    ck_assert_str_eq(result, "test_valuetest_value");
+}
+END_TEST
+
+START_TEST(test_strnjoin__when_joiner_is_NULL__returns_joined_string)
+{
+    /* setup */
+
+    const char *test_value = "test_value";
+    const char *src[] = { test_value, test_value };
+    char result[RESULT_LEN];
+    result[0] = '\0';
+
+    /* test */
+
+    g_strnjoin(result, RESULT_LEN, NULL, src, 2);
+
+    /* verify */
+
+    ck_assert_str_eq(result, "test_valuetest_value");
+}
+END_TEST
+
+START_TEST(test_strnjoin__when_destination_is_NULL__returns_NULL)
+{
+    /* setup */
+
+    const char *test_value = "test_value";
+    const char *src[] = { test_value };
+    char *result = NULL;
+
+    /* test */
+
+    g_strnjoin(result, 0, " ", src, 1);
+
+    /* verify */
+
+    ck_assert_ptr_eq(result, NULL);
+}
+END_TEST
+
+START_TEST(test_strnjoin__when_destination_is_shorter_than_src__returns_partial_src_string)
+{
+    /* setup */
+
+    const char *test_value = "test_value";
+    const char *src[] = { test_value };
+    char result[5];
+    result[0] = '\0';
+
+    /* test */
+
+    g_strnjoin(result, 5, " ", src, 1);
+
+    /* verify */
+
+    ck_assert_str_eq(result, "test");
+}
+END_TEST
+
+START_TEST(test_strnjoin__when_destination_is_shorter_than_src_plus_joiner__returns_partial_joined_string)
+{
+    /* setup */
+
+    const char *test_value = "test_value";
+    const char *src[] = { test_value, test_value};
+    char result[16];
+    result[0] = '\0';
+
+    /* test */
+
+    g_strnjoin(result, 16, " joiner ", src, 2);
+
+    /* verify */
+
+    ck_assert_str_eq(result, "test_value join");
+}
+END_TEST
+
+START_TEST(test_strnjoin__when_destination_has_contents__returns_joined_string_with_content_overwritten)
+{
+    /* setup */
+
+    const char *test_value = "test_value";
+    const char *src[] = { test_value, test_value};
+    char result[RESULT_LEN] = "1234567890";
+
+    /* test */
+
+    g_strnjoin(result, RESULT_LEN, " ", src, 2);
+
+    /* verify */
+
+    ck_assert_str_eq(result, "test_value test_value");
+}
+END_TEST
+
+START_TEST(test_strnjoin__when_always__then_doesnt_write_beyond_end_of_destination)
+{
+    /* setup */
+
+    const char *src[] = { "a","b","c"};
+    char result[5+1+1]; /* a-b-c + term null + guard value */
+
+    /* test */
+
+    result[6] = '\x7f';
+    g_strnjoin(result, 5 + 1, "-", src, 3);
+
+    /* verify */
+
+    ck_assert_int_eq(result[6], '\x7f');
+}
+END_TEST
+
+/******************************************************************************/
+Suite *
+make_suite_test_string(void)
+{
+    Suite *s;
+    TCase *tc_strnjoin;
+
+    s = suite_create("String");
+
+    tc_strnjoin = tcase_create("strnjoin");
+    suite_add_tcase(s, tc_strnjoin);
+    tcase_add_test(tc_strnjoin, test_strnjoin__when_src_is_null__returns_empty_string);
+    tcase_add_test(tc_strnjoin, test_strnjoin__when_src_has_null_item__returns_joined_string);
+    tcase_add_test(tc_strnjoin, test_strnjoin__when_src_has_one_item__returns_copied_source_string);
+    tcase_add_test(tc_strnjoin, test_strnjoin__when_src_has_two_items__returns_joined_string);
+    tcase_add_test(tc_strnjoin, test_strnjoin__when_joiner_is_empty_string__returns_joined_string);
+    tcase_add_test(tc_strnjoin, test_strnjoin__when_joiner_is_NULL__returns_joined_string);
+    tcase_add_test(tc_strnjoin, test_strnjoin__when_destination_is_NULL__returns_NULL);
+    tcase_add_test(tc_strnjoin, test_strnjoin__when_destination_is_shorter_than_src__returns_partial_src_string);
+    tcase_add_test(tc_strnjoin, test_strnjoin__when_destination_is_shorter_than_src_plus_joiner__returns_partial_joined_string);
+    tcase_add_test(tc_strnjoin, test_strnjoin__when_destination_has_contents__returns_joined_string_with_content_overwritten);
+    tcase_add_test(tc_strnjoin, test_strnjoin__when_always__then_doesnt_write_beyond_end_of_destination);
+
+    return s;
+}

tests/memtest/Makefile.am

@@ -6,7 +6,7 @@ if XRDP_DEBUG
 AM_CPPFLAGS += -DXRDP_DEBUG
 endif
 
-sbin_PROGRAMS = \
+check_PROGRAMS = \
   memtest
 
 memtest_SOURCES = \