Merge branch 'maint-account-cpp' into maint #1107

gnucash/gnome-utils/dialog-preferences.c

@@ -132,7 +132,7 @@ static gchar *gnc_account_separator_is_valid (const gchar *separator,
         message = gnc_account_name_violations_errmsg (*normalized_separator,
                                                       conflict_accts);
 
-    g_list_free (conflict_accts);
+    g_list_free_full (conflict_accts, g_free);
     return message;
 }
 

gnucash/gnome-utils/gnc-file.c

@@ -1101,6 +1101,7 @@ RESTART:
                          invalid_account_names );
         gnc_warning_dialog(parent, "%s", message);
         g_free ( message );
+        g_list_free_full (invalid_account_names, g_free);
     }
 
     // Fix account color slots being set to 'Not Set', should run once on a book

libgnucash/core-utils/gnc-glib-utils.c

@@ -327,3 +327,35 @@ void gnc_gpid_kill(GPid pid)
     }
 #endif /* G_OS_WIN32 */
 }
+
+static inline char*
+gnc_strcat (char* dest, const char* src)
+{
+    while (*dest) dest++;
+    while ((*dest++ = *src++));
+    return --dest;
+}
+
+gchar *
+gnc_g_list_stringjoin (GList *list_of_strings, const gchar *sep)
+{
+    gint seplen = sep ? strlen(sep) : 0;
+    gint length = -seplen;
+    gchar *retval, *p;
+
+    if (!list_of_strings)
+        return NULL;
+
+    for (GList *n = list_of_strings; n; n = n->next)
+        length += strlen ((gchar*)n->data) + seplen;
+
+    p = retval = (gchar*) g_malloc0 (length * sizeof (gchar) + 1);
+    for (GList *n = list_of_strings; n; n = n->next)
+    {
+        p = gnc_strcat (p, (gchar*)n->data);
+        if (n->next && sep)
+            p = gnc_strcat (p, sep);
+    }
+
+    return retval;
+}

libgnucash/core-utils/gnc-glib-utils.h

@@ -183,6 +183,22 @@ void gnc_scm_log_debug(const gchar *msg);
  @{
 */
 
+
+/**
+ * @brief Return a string joining a GList whose elements are gchar*
+ * strings. Returns NULL if an empty GList* is passed through. The
+ * optional sep string will be used as separator. Must be g_freed when
+ * not needed anymore.
+ *
+ * @param list_of_strings A GList of chars*
+ *
+ * @param sep a separator or NULL
+ *
+ * @return A newly allocated string that has to be g_free'd by the
+ * caller.
+ **/
+gchar * gnc_g_list_stringjoin (GList *list_of_strings, const gchar *sep);
+
 /** Kill a process.  On UNIX send a SIGKILL, on Windows call TerminateProcess.
  *
  *  @param pid The process ID. */

libgnucash/core-utils/test/test-gnc-glib-utils.c

@@ -53,6 +53,62 @@ test_gnc_utf8_strip_invalid_and_controls (gconstpointer data)
     g_free (msg1);
 }
 
+static void
+test_g_list_stringjoin (gconstpointer data)
+{
+    GList *test = NULL;
+    gchar *ret;
+
+    ret = gnc_g_list_stringjoin (NULL, NULL);
+    g_assert (ret == NULL);
+
+    ret = gnc_g_list_stringjoin (NULL, ":");
+    g_assert (ret == NULL);
+
+    test = g_list_prepend (test, "one");
+
+    ret = gnc_g_list_stringjoin (test, NULL);
+    g_assert_cmpstr (ret, ==, "one");
+    g_free (ret);
+
+    ret = gnc_g_list_stringjoin (test, "");
+    g_assert_cmpstr (ret, ==, "one");
+    g_free (ret);
+
+    ret = gnc_g_list_stringjoin (test, ":");
+    g_assert_cmpstr (ret, ==, "one");
+    g_free (ret);
+
+    test = g_list_prepend (test, "two");
+
+    ret = gnc_g_list_stringjoin (test, NULL);
+    g_assert_cmpstr (ret, ==, "twoone");
+    g_free (ret);
+
+    ret = gnc_g_list_stringjoin (test, "");
+    g_assert_cmpstr (ret, ==, "twoone");
+    g_free (ret);
+
+    ret = gnc_g_list_stringjoin (test, ":");
+    g_assert_cmpstr (ret, ==, "two:one");
+    g_free (ret);
+
+    test = g_list_prepend (test, "three");
+
+    ret = gnc_g_list_stringjoin (test, NULL);
+    g_assert_cmpstr (ret, ==, "threetwoone");
+    g_free (ret);
+
+    ret = gnc_g_list_stringjoin (test, "");
+    g_assert_cmpstr (ret, ==, "threetwoone");
+    g_free (ret);
+
+    ret = gnc_g_list_stringjoin (test, ":");
+    g_assert_cmpstr (ret, ==, "three:two:one");
+    g_free (ret);
+
+    g_list_free (test);
+}
 
 int
 main (int argc, char *argv[])
@@ -62,6 +118,7 @@ main (int argc, char *argv[])
     g_test_init (&argc, &argv, NULL); // initialize test program
     g_test_add_data_func ("/core-utils/gnc_utf8_strip_invalid_and_controls invalid utf8", (gconstpointer)invalid_utf8, test_gnc_utf8_strip_invalid_and_controls);
     g_test_add_data_func ("/core-utils/gnc_utf8_strip_invalid_and_controls control chars", (gconstpointer)controls, test_gnc_utf8_strip_invalid_and_controls);
+    g_test_add_data_func ("/core-utils/gnc_g_list_stringjoin", NULL, test_g_list_stringjoin);
 
     return g_test_run();
 }

libgnucash/engine/Account.cpp

@@ -227,26 +227,12 @@ gnc_set_account_separator (const gchar *separator)
 
 gchar *gnc_account_name_violations_errmsg (const gchar *separator, GList* invalid_account_names)
 {
-    GList *node;
     gchar *message = NULL;
-    gchar *account_list = NULL;
 
     if ( !invalid_account_names )
         return NULL;
 
-    for ( node = invalid_account_names;  node; node = g_list_next(node))
+    auto account_list {gnc_g_list_stringjoin (invalid_account_names, "\n")};
-    {
-        if ( !account_list )
-            account_list = static_cast<gchar *>(node->data);
-        else
-        {
-            gchar *tmp_list = NULL;
-
-            tmp_list = g_strconcat (account_list, "\n", node->data, nullptr);
-            g_free ( account_list );
-            account_list = tmp_list;
-        }
-    }
 
     /* Translators: The first %s will be the account separator character,
        the second %s is a list of account names.

libgnucash/engine/Account.h

@@ -283,8 +283,8 @@ gchar *gnc_account_name_violations_errmsg (const gchar *separator, GList* invali
  *  @param book Pointer to the book with accounts to verify
  *  @param separator The separator character to verify against
  *
- *  @return A GList of invalid account names. Should be freed with g_list_free
+ *  @return A GList of invalid account names. Should be freed with
- *          if no longer needed.
+ *          g_list_free_full (value, g_free) when no longer needed.
  */
 GList *gnc_account_list_name_violations (QofBook *book, const gchar *separator);
 

libgnucash/engine/test/utest-Account.cpp

@@ -29,6 +29,7 @@ extern "C"
 #include <gnc-event.h>
 #include <gnc-date.h>
 /* Add specific headers for this class */
+#include "gnc-glib-utils.h"
 #include "../Account.h"
 #include "../AccountP.h"
 #include "../Split.h"
@@ -429,24 +430,12 @@ test_gnc_account_name_violations_errmsg ()
 {
     GList *badnames = NULL, *nonames = NULL, *node = NULL;
     auto separator = ":";
-    char *account_list = NULL;
     /* FUT wants to free the strings, so we alloc them */
     badnames = g_list_prepend (badnames, g_strdup ("Foo:bar"));
     badnames = g_list_prepend (badnames, g_strdup ("baz"));
     badnames = g_list_prepend (badnames, g_strdup ("waldo:pepper"));
     auto message = gnc_account_name_violations_errmsg (separator, nonames);
-    for (node = badnames; node; node = g_list_next (node))
+    auto account_list = gnc_g_list_stringjoin (badnames, "\n");
-    {
-        if (!account_list)
-            account_list = g_strdup (static_cast<char*>(node->data));
-        else
-        {
-            auto tmp_list = g_strconcat ( account_list, "\n",
-                                          static_cast<char*>(node->data), NULL);
-            g_free (account_list);
-            account_list = tmp_list;
-        }
-    }
     message = gnc_account_name_violations_errmsg (separator, nonames);
     g_assert (message == NULL);
     auto validation_message = g_strdup_printf (
@@ -455,6 +444,7 @@ test_gnc_account_name_violations_errmsg ()
         "Either change the account names or choose another separator "
         "character.\n\nBelow you will find the list of invalid account names:\n"
         "%s", separator, account_list);
+    g_free (account_list);
     message = gnc_account_name_violations_errmsg (separator, badnames);
     g_assert_cmpstr ( message, == , validation_message);
     g_free (validation_message);