Merge branch 'maint-g-value-leaks' into maint #1124

libgnucash/engine/Account.cpp

@@ -325,12 +325,12 @@ gnc_account_init(Account* acc)
     priv->starting_reconciled_balance = gnc_numeric_zero();
     priv->balance_dirty = FALSE;
 
-    priv->color == is_unset;
-    priv->sort_order == is_unset;
-    priv->notes == is_unset;
-    priv->filter == is_unset;
-    priv->equity_type == TriState::Unset;
-    priv->sort_reversed == TriState::Unset;
+    priv->color = (char*) is_unset;
+    priv->sort_order = (char*) is_unset;
+    priv->notes = (char*) is_unset;
+    priv->filter = (char*) is_unset;
+    priv->equity_type = TriState::Unset;
+    priv->sort_reversed = TriState::Unset;
 
     priv->splits = NULL;
     priv->sort_dirty = FALSE;
@@ -2470,6 +2470,21 @@ xaccAccountSetDescription (Account *acc, const char *str)
     xaccAccountCommitEdit(acc);
 }
 
+static char*
+stripdup_or_null (const char *value)
+{
+    if (value)
+    {
+        auto temp = g_strstrip (g_strdup (value));
+        if (*temp)
+            return temp;
+        g_free (temp);
+    }
+    return nullptr;
+}
+
+// note the *value argument is expected to be either a strstripped
+// char* or nullptr, as returned by stripdup_or_null above.
 static void
 set_kvp_string_tag (Account *acc, const char *tag, const char *value)
 {
@@ -2478,18 +2493,11 @@ set_kvp_string_tag (Account *acc, const char *tag, const char *value)
     xaccAccountBeginEdit(acc);
     if (value)
     {
-        gchar *tmp = g_strstrip(g_strdup(value));
-        if (strlen (tmp))
-        {
-            GValue v = G_VALUE_INIT;
-            g_value_init (&v, G_TYPE_STRING);
-            g_value_set_string (&v, tmp);
-            qof_instance_set_path_kvp (QOF_INSTANCE (acc), &v, {tag});
-            g_value_unset (&v);
-        }
-        else
-            qof_instance_set_path_kvp (QOF_INSTANCE (acc), NULL, {tag});
-        g_free(tmp);
+        GValue v = G_VALUE_INIT;
+        g_value_init (&v, G_TYPE_STRING);
+        g_value_set_string (&v, value);
+        qof_instance_set_path_kvp (QOF_INSTANCE (acc), &v, {tag});
+        g_value_unset (&v);
     }
     else
     {
@@ -2516,8 +2524,8 @@ xaccAccountSetColor (Account *acc, const char *str)
     auto priv = GET_PRIVATE (acc);
     if (priv->color != is_unset)
         g_free (priv->color);
-    priv->color = g_strdup (str);
-    set_kvp_string_tag (acc, "color", str);
+    priv->color = stripdup_or_null (str);
+    set_kvp_string_tag (acc, "color", priv->color);
 }
 
 void
@@ -2526,8 +2534,8 @@ xaccAccountSetFilter (Account *acc, const char *str)
     auto priv = GET_PRIVATE (acc);
     if (priv->filter != is_unset)
         g_free (priv->filter);
-    priv->filter = g_strdup (str);
-    set_kvp_string_tag (acc, "filter", str);
+    priv->filter = stripdup_or_null (str);
+    set_kvp_string_tag (acc, "filter", priv->filter);
 }
 
 void
@@ -2536,8 +2544,8 @@ xaccAccountSetSortOrder (Account *acc, const char *str)
     auto priv = GET_PRIVATE (acc);
     if (priv->sort_order != is_unset)
         g_free (priv->sort_order);
-    priv->sort_order = g_strdup (str);
-    set_kvp_string_tag (acc, "sort-order", str);
+    priv->sort_order = stripdup_or_null (str);
+    set_kvp_string_tag (acc, "sort-order", priv->sort_order);
 }
 
 void
@@ -2572,8 +2580,8 @@ xaccAccountSetNotes (Account *acc, const char *str)
     auto priv = GET_PRIVATE (acc);
     if (priv->notes != is_unset)
         g_free (priv->notes);
-    priv->notes = g_strdup (str);
-    set_kvp_string_tag (acc, "notes", str);
+    priv->notes = stripdup_or_null (str);
+    set_kvp_string_tag (acc, "notes", priv->notes);
 }
 
 void
@@ -4209,7 +4217,7 @@ xaccAccountGetIsOpeningBalance (const Account *acc)
     if (priv->equity_type == TriState::Unset)
     {
         auto equity_type = get_kvp_string_tag (acc, "equity-type");
-        priv->equity_type = g_strcmp0 (equity_type, "true") ?
+        priv->equity_type = g_strcmp0 (equity_type, "opening-balance") ?
             TriState::False : TriState::True;
         g_free (equity_type);
     }
@@ -4223,7 +4231,7 @@ xaccAccountSetIsOpeningBalance (Account *acc, gboolean val)
         return;
     auto priv = GET_PRIVATE (acc);
     priv->equity_type = val ? TriState::True : TriState::False;
-    set_kvp_string_tag(acc, "equity-type", val ? "opening-balance" : "");
+    set_kvp_string_tag(acc, "equity-type", val ? "opening-balance" : nullptr);
 }
 
 GNCPlaceholderType

libgnucash/engine/gnc-lot.c

@@ -498,6 +498,7 @@ gnc_lot_set_title (GNCLot *lot, const char *str)
     qof_begin_edit(QOF_INSTANCE(lot));
     g_value_init (&v, G_TYPE_STRING);
     g_value_set_string (&v, str);
+    priv->title = g_strdup (str);
     qof_instance_set_kvp (QOF_INSTANCE (lot), &v, 1, "title");
     qof_instance_set_dirty(QOF_INSTANCE(lot));
     gnc_lot_commit_edit(lot);
@@ -516,6 +517,7 @@ gnc_lot_set_notes (GNCLot *lot, const char *str)
     qof_begin_edit(QOF_INSTANCE(lot));
     g_value_init (&v, G_TYPE_STRING);
     g_value_set_string (&v, str);
+    priv->notes = g_strdup (str);
     qof_instance_set_kvp (QOF_INSTANCE (lot), &v, 1, "notes");
     qof_instance_set_dirty(QOF_INSTANCE(lot));
     gnc_lot_commit_edit(lot);

libgnucash/engine/test/test-commodities.cpp

@@ -122,6 +122,11 @@ test_commodity(void)
             gnc_commodity_get_fraction(com) == fraction,
             "reset fraction code equal test");
 
+        g_assert_cmpstr (gnc_commodity_get_user_symbol(com), ==, NULL);
+
+        gnc_commodity_set_user_symbol (com, "CA$");
+        g_assert_cmpstr (gnc_commodity_get_user_symbol(com), ==, "CA$");
+
         com2 = gnc_commodity_new(book, fullname, name_space, mnemonic,
                                  cusip, fraction);
         do_test(

libgnucash/engine/test/test-lots.cpp

@@ -32,6 +32,7 @@ extern "C"
 #include <ctype.h>
 #include "qof.h"
 #include "Account.h"
+#include "gnc-lot.h"
 #include "Scrub3.h"
 #include "cashobjects.h"
 #include "test-stuff.h"
@@ -42,6 +43,48 @@ extern "C"
 static gint transaction_num = 32;
 static gint	max_iterate = 1;
 
+
+static void
+test_lot_kvp ()
+{
+    QofSession *sess = get_random_session ();
+    QofBook *book = qof_session_get_book (sess);
+    GNCLot *lot = gnc_lot_new (book);
+
+    // title
+    g_assert_cmpstr (gnc_lot_get_title (lot), ==, NULL);
+
+    gnc_lot_set_title (lot, "");
+    g_assert_cmpstr (gnc_lot_get_title (lot), ==, "");
+
+    gnc_lot_set_title (lot, "doc");
+    g_assert_cmpstr (gnc_lot_get_title (lot), ==, "doc");
+
+    gnc_lot_set_title (lot, "unset");
+    g_assert_cmpstr (gnc_lot_get_title (lot), ==, "unset");
+
+    gnc_lot_set_title (lot, NULL);
+    g_assert_cmpstr (gnc_lot_get_title (lot), ==, NULL);
+
+    // notes
+    g_assert_cmpstr (gnc_lot_get_notes (lot), ==, NULL);
+
+    gnc_lot_set_notes (lot, "");
+    g_assert_cmpstr (gnc_lot_get_notes (lot), ==, "");
+
+    gnc_lot_set_notes (lot, "doc");
+    g_assert_cmpstr (gnc_lot_get_notes (lot), ==, "doc");
+
+    gnc_lot_set_notes (lot, "unset");
+    g_assert_cmpstr (gnc_lot_get_notes (lot), ==, "unset");
+
+    gnc_lot_set_notes (lot, NULL);
+    g_assert_cmpstr (gnc_lot_get_notes (lot), ==, NULL);
+
+    gnc_lot_destroy (lot);
+    qof_session_end (sess);
+}
+
 static void
 run_test (void)
 {
@@ -94,6 +137,9 @@ main (int argc, char **argv)
         fflush(stdout);
         run_test ();
     }
+
+    test_lot_kvp ();
+
     /* 'erase' the recurring tag line with dummy spaces. */
     fprintf(stdout, "Lots: Test series complete.\n");
     fflush(stdout);

libgnucash/engine/test/utest-Account.cpp

@@ -1067,6 +1067,105 @@ gnc_account_insert_split (Account *acc, Split *s)// C: 5 in 3
 
 Also tests gnc_account_remove_split ()
 */
+
+
+static void
+test_gnc_account_kvp_setters_getters (Fixture *fixture, gconstpointer pData)
+{
+    Account *account = xaccMallocAccount (gnc_account_get_book (fixture->acct));
+    xaccAccountSetType (account, ACCT_TYPE_EQUITY);
+
+    // equity_type getter/setter
+    g_assert (xaccAccountGetIsOpeningBalance (account) == FALSE);
+
+    xaccAccountSetIsOpeningBalance (account, TRUE);
+    g_assert (xaccAccountGetIsOpeningBalance (account) == TRUE);
+
+    xaccAccountSetIsOpeningBalance (account, FALSE);
+    g_assert (xaccAccountGetIsOpeningBalance (account) == FALSE);
+
+    // sortreversed getter/setter
+    g_assert (xaccAccountGetSortReversed (account) == FALSE);
+
+    xaccAccountSetSortReversed (account, TRUE);
+    g_assert (xaccAccountGetSortReversed (account) == TRUE);
+
+    xaccAccountSetSortReversed (account, FALSE);
+    g_assert (xaccAccountGetSortReversed (account) == FALSE);
+
+    // color getter/setter
+    g_assert_cmpstr (xaccAccountGetColor (account), ==, nullptr);
+
+    xaccAccountSetColor (account, "red");
+    g_assert_cmpstr (xaccAccountGetColor (account), ==, "red");
+
+    xaccAccountSetColor (account, "unset");
+    g_assert_cmpstr (xaccAccountGetColor (account), ==, "unset");
+
+    xaccAccountSetColor (account, "");
+    g_assert_cmpstr (xaccAccountGetColor (account), ==, nullptr);
+
+    xaccAccountSetColor (account, nullptr);
+    g_assert_cmpstr (xaccAccountGetColor (account), ==, nullptr);
+
+    // filter getter/setter
+    g_assert_cmpstr (xaccAccountGetFilter (account), ==, nullptr);
+
+    xaccAccountSetFilter (account, "bla");
+    g_assert_cmpstr (xaccAccountGetFilter (account), ==, "bla");
+
+    xaccAccountSetFilter (account, "unset");
+    g_assert_cmpstr (xaccAccountGetFilter (account), ==, "unset");
+
+    xaccAccountSetFilter (account, "   unset ");
+    g_assert_cmpstr (xaccAccountGetFilter (account), ==, "unset");
+
+    xaccAccountSetFilter (account, "");
+    g_assert_cmpstr (xaccAccountGetFilter (account), ==, nullptr);
+
+    xaccAccountSetFilter (account, nullptr);
+    g_assert_cmpstr (xaccAccountGetFilter (account), ==, nullptr);
+
+    // sortOrder getter/setter
+    g_assert_cmpstr (xaccAccountGetSortOrder (account), ==, nullptr);
+
+    xaccAccountSetSortOrder (account, "boo");
+    g_assert_cmpstr (xaccAccountGetSortOrder (account), ==, "boo");
+
+    xaccAccountSetSortOrder (account, "unset");
+    g_assert_cmpstr (xaccAccountGetSortOrder (account), ==, "unset");
+
+    xaccAccountSetSortOrder (account, "  unset ");
+    g_assert_cmpstr (xaccAccountGetSortOrder (account), ==, "unset");
+
+    xaccAccountSetSortOrder (account, "");
+    g_assert_cmpstr (xaccAccountGetSortOrder (account), ==, nullptr);
+
+    xaccAccountSetSortOrder (account, nullptr);
+    g_assert_cmpstr (xaccAccountGetSortOrder (account), ==, nullptr);
+
+    // Notes getter/setter
+    g_assert_cmpstr (xaccAccountGetNotes (account), ==, nullptr);
+
+    xaccAccountSetNotes (account, "boo");
+    g_assert_cmpstr (xaccAccountGetNotes (account), ==, "boo");
+
+    xaccAccountSetNotes (account, "unset");
+    g_assert_cmpstr (xaccAccountGetNotes (account), ==, "unset");
+
+    xaccAccountSetNotes (account, "    unset ");
+    g_assert_cmpstr (xaccAccountGetNotes (account), ==, "unset");
+
+    xaccAccountSetNotes (account, "");
+    g_assert_cmpstr (xaccAccountGetNotes (account), ==, nullptr);
+
+    xaccAccountSetNotes (account, nullptr);
+    g_assert_cmpstr (xaccAccountGetNotes (account), ==, nullptr);
+
+    xaccAccountBeginEdit (account);
+    xaccAccountDestroy (account);
+}
+
 static void
 test_gnc_account_insert_remove_split (Fixture *fixture, gconstpointer pData)
 {
@@ -2549,6 +2648,7 @@ test_suite_account (void)
     GNC_TEST_ADD (suitename, "xaccAccountCommitEdit", Fixture, &good_data, setup, test_xaccAccountCommitEdit,  NULL );
 // GNC_TEST_ADD (suitename, "xaccAcctChildrenEqual", Fixture, NULL, setup, test_xaccAcctChildrenEqual,  teardown );
 // GNC_TEST_ADD (suitename, "xaccAccountEqual", Fixture, NULL, setup, test_xaccAccountEqual,  teardown );
+    GNC_TEST_ADD (suitename, "gnc account kvp getters & setters", Fixture, NULL, setup, test_gnc_account_kvp_setters_getters,  teardown );
     GNC_TEST_ADD (suitename, "gnc account insert & remove split", Fixture, NULL, setup, test_gnc_account_insert_remove_split,  teardown );
     GNC_TEST_ADD (suitename, "xaccAccount Insert and Remove Lot", Fixture, &good_data, setup, test_xaccAccountInsertRemoveLot,  teardown );
     GNC_TEST_ADD (suitename, "xaccAccountRecomputeBalance", Fixture, &some_data, setup, test_xaccAccountRecomputeBalance,  teardown );

libgnucash/engine/test/utest-Invoice.c

@@ -175,6 +175,27 @@ test_invoice_post ( Fixture *fixture, gconstpointer pData )
     g_assert(!gncInvoiceIsPosted(fixture->invoice));
 }
 
+
+static void
+test_invoice_doclink ( Fixture *fixture, gconstpointer pData )
+{
+    GncInvoice* inv = fixture->invoice;
+
+    g_assert_cmpstr (gncInvoiceGetDocLink (inv), ==, NULL);
+
+    gncInvoiceSetDocLink (inv, "doc");
+    g_assert_cmpstr (gncInvoiceGetDocLink (inv), ==, "doc");
+
+    gncInvoiceSetDocLink (inv, "unset");
+    g_assert_cmpstr (gncInvoiceGetDocLink (inv), ==, "unset");
+
+    gncInvoiceSetDocLink (inv, "");
+    g_assert_cmpstr (gncInvoiceGetDocLink (inv), ==, NULL);
+
+    gncInvoiceSetDocLink (inv, NULL);
+    g_assert_cmpstr (gncInvoiceGetDocLink (inv), ==, NULL);
+}
+
 static gboolean account_has_one_split (const Account *acc)
 {
     GList *splits = xaccAccountGetSplitList (acc);
@@ -214,6 +235,7 @@ test_suite_gncInvoice ( void )
     static InvoiceData pData = { FALSE, FALSE, { 1000, 100 }, { 2000, 100 } };  // Vendor bill
     GNC_TEST_ADD( suitename, "post/unpost", Fixture, &pData, setup, test_invoice_post, teardown );
 
+    GNC_TEST_ADD( suitename, "doclink", Fixture, &pData, setup, test_invoice_doclink, teardown );
     GNC_TEST_ADD( suitename, "post trans - vendor bill", Fixture, &pData, setup_with_invoice, test_invoice_posted_trans, teardown_with_invoice );
     pData.is_cn = TRUE;   // Vendor credit note
     GNC_TEST_ADD( suitename, "post trans - vendor credit note", Fixture, &pData, setup_with_invoice, test_invoice_posted_trans, teardown_with_invoice );

libgnucash/engine/test/utest-Transaction.cpp

@@ -1843,6 +1843,26 @@ void
 xaccTransUnvoid (Transaction *trans)// C: 1  Local: 0:0:0
 */
 
+static void
+test_xaccTransSetDocLink (Fixture *fixture, gconstpointer pData)
+{
+    auto trans = fixture->txn;
+
+    g_assert_cmpstr (xaccTransGetDocLink (trans), ==, NULL);
+
+    xaccTransSetDocLink (trans, "doclink");
+    g_assert_cmpstr (xaccTransGetDocLink (trans), ==, "doclink");
+
+    xaccTransSetDocLink (trans, "unset");
+    g_assert_cmpstr (xaccTransGetDocLink (trans), ==, "unset");
+
+    xaccTransSetDocLink (trans, "");
+    g_assert_cmpstr (xaccTransGetDocLink (trans), ==, NULL);
+
+    xaccTransSetDocLink (trans, NULL);
+    g_assert_cmpstr (xaccTransGetDocLink (trans), ==, NULL);
+}
+
 static void
 test_xaccTransVoid (Fixture *fixture, gconstpointer pData)
 {
@@ -2045,6 +2065,7 @@ test_suite_transaction (void)
     GNC_TEST_ADD (suitename, "xaccTransRollbackEdit - Backend Errors", Fixture, NULL, setup, test_xaccTransRollbackEdit_BackendErrors, teardown);
     GNC_TEST_ADD (suitename, "xaccTransOrder_num_action", Fixture, NULL, setup, test_xaccTransOrder_num_action, teardown);
     GNC_TEST_ADD (suitename, "xaccTransGetTxnType", Fixture, NULL, setup, test_xaccTransGetTxnType, teardown);
+    GNC_TEST_ADD (suitename, "xaccTransSetDocLink", Fixture, NULL, setup, test_xaccTransSetDocLink, teardown);
     GNC_TEST_ADD (suitename, "xaccTransVoid", Fixture, NULL, setup, test_xaccTransVoid, teardown);
     GNC_TEST_ADD (suitename, "xaccTransReverse", Fixture, NULL, setup, test_xaccTransReverse, teardown);
     GNC_TEST_ADD (suitename, "xaccTransScrubGainsDate_no_dirty", GainsFixture, NULL, setup_with_gains, test_xaccTransScrubGainsDate_no_dirty, teardown_with_gains);