gnucash stable: Multiple changes pushed

John Ralls jralls at code.gnucash.org
Sat Apr 29 14:58:37 EDT 2023


Updated	 via  https://github.com/Gnucash/gnucash/commit/df878c6a (commit)
	 via  https://github.com/Gnucash/gnucash/commit/71802b56 (commit)
	 via  https://github.com/Gnucash/gnucash/commit/6be682b6 (commit)
	 via  https://github.com/Gnucash/gnucash/commit/0d86be6d (commit)
	 via  https://github.com/Gnucash/gnucash/commit/ed3fe008 (commit)
	 via  https://github.com/Gnucash/gnucash/commit/35aeed45 (commit)
	 via  https://github.com/Gnucash/gnucash/commit/bd94965d (commit)
	from  https://github.com/Gnucash/gnucash/commit/168765cf (commit)



commit df878c6a3deea44c29028b852cf35d692f7a8631
Merge: 168765cf77 71802b5627
Author: John Ralls <jralls at ceridwen.us>
Date:   Sat Apr 29 11:54:41 2023 -0700

    Merge Maarten Bosmans's 'memleak-fixes' into stable.


commit 71802b5627c94097f13549310be47c9f4bf5ac40
Author: Maarten Bosmans <mkbosmans at gmail.com>
Date:   Sat Mar 18 12:30:03 2023 +0100

    [test] Fix memory leak in test-gnc-quotes
    
    When the commodity table is registered, the current book will get
    a default table assigned.  When later setting the table explicitly
    using qof_book_set_data() the exisiting table gets overwritten and
    is thus leaked.
    
    There is no way of removing or freeing a currency table from a book,
    so the best we can do here is to set our own table on the book before
    calling gnc_commodity_table_register().

diff --git a/libgnucash/app-utils/test/gtest-gnc-quotes.cpp b/libgnucash/app-utils/test/gtest-gnc-quotes.cpp
index 66900bef30..30b8a5dc6f 100644
--- a/libgnucash/app-utils/test/gtest-gnc-quotes.cpp
+++ b/libgnucash/app-utils/test/gtest-gnc-quotes.cpp
@@ -90,10 +90,15 @@ protected:
     m_book{qof_session_get_book(gnc_get_current_session())}
     {
         qof_init();
-        gnc_commodity_table_register();
-        gnc_pricedb_register();
+
+        /* By setting an empty commodity table on the book before registering
+         * the commodity_table type we avoid adding the default commodities */
         auto comm_table{gnc_commodity_table_new()};
         qof_book_set_data(m_book, GNC_COMMODITY_TABLE, comm_table);
+
+        gnc_commodity_table_register();
+        gnc_pricedb_register();
+
         auto eur = gnc_commodity_new(m_book, "Euro", "ISO4217", "EUR", NULL, 100);
         auto source{gnc_quote_source_lookup_by_internal("currency")};
         gnc_commodity_begin_edit(eur);

commit 6be682b6459c6a7015a783bcab5e6387d140fe77
Author: Maarten Bosmans <mkbosmans at gmail.com>
Date:   Fri Mar 17 21:39:26 2023 +0100

    Store allocated temporaries in a variable so they can be freed
    
    If a function that returns an allocated pointer is passed directly into
    something that does not take ownership of the pointer, the allocation is
    leaked.  This can be fixed by assigning the pointer to a new variable
    and freeing it after operation on the memory.

diff --git a/libgnucash/app-utils/gnc-quotes.cpp b/libgnucash/app-utils/gnc-quotes.cpp
index d8b1215de0..3f4f9fd4c1 100644
--- a/libgnucash/app-utils/gnc-quotes.cpp
+++ b/libgnucash/app-utils/gnc-quotes.cpp
@@ -122,7 +122,7 @@ private:
 class GncFQQuoteSource final : public GncQuoteSource
 {
     const bfs::path c_cmd;
-    const std::string c_fq_wrapper;
+    std::string c_fq_wrapper;
     std::string m_version;
     StrVec m_sources;
     std::string m_api_key;
@@ -145,11 +145,12 @@ static const std::string empty_string{};
 
 GncFQQuoteSource::GncFQQuoteSource() :
 c_cmd{bp::search_path("perl")},
-c_fq_wrapper{std::string(gnc_path_get_bindir()) + "/finance-quote-wrapper"},
 m_version{}, m_sources{}, m_api_key{}
 {
+    char *bindir = gnc_path_get_bindir();
+    c_fq_wrapper = std::string(bindir) + "/finance-quote-wrapper";
+    g_free(bindir);
     StrVec args{"-w", c_fq_wrapper, "-v"};
-    const std::string empty_string;
     auto [rv, sources, errors] = run_cmd(args, empty_string);
     if (rv)
     {
diff --git a/libgnucash/app-utils/test/gtest-gnc-quotes.cpp b/libgnucash/app-utils/test/gtest-gnc-quotes.cpp
index bd6739ea69..66900bef30 100644
--- a/libgnucash/app-utils/test/gtest-gnc-quotes.cpp
+++ b/libgnucash/app-utils/test/gtest-gnc-quotes.cpp
@@ -101,8 +101,7 @@ protected:
         gnc_commodity_set_quote_source(eur, source);
         gnc_commodity_commit_edit(eur);
         gnc_commodity_table_insert(comm_table, eur);
-        auto usd = gnc_commodity_new(m_book, "United States Dollar", "CURRENCY",
-                                  "USD", NULL, 100);
+        auto usd = gnc_commodity_new(m_book, "United States Dollar", "CURRENCY", "USD", NULL, 100);
         gnc_commodity_table_insert(comm_table, usd);
         source = gnc_quote_source_lookup_by_internal("yahoo_json");
         auto aapl = gnc_commodity_new(m_book, "Apple", "NASDAQ", "AAPL", NULL, 1);
@@ -111,8 +110,7 @@ protected:
         gnc_commodity_set_quote_source(aapl, source);
         gnc_commodity_commit_edit(aapl);
         gnc_commodity_table_insert(comm_table, aapl);
-        auto hpe = gnc_commodity_new(m_book, "Hewlett Packard", "NYSE", "HPE",
-                                  NULL, 1);
+        auto hpe = gnc_commodity_new(m_book, "Hewlett Packard", "NYSE", "HPE", NULL, 1);
         gnc_commodity_begin_edit(hpe);
         gnc_commodity_set_quote_flag(hpe, TRUE);
         gnc_commodity_set_quote_source(hpe, source);
@@ -124,7 +122,9 @@ protected:
         gnc_commodity_set_quote_source(fkcm, source);
         gnc_commodity_commit_edit(fkcm);
         gnc_commodity_table_insert(comm_table, fkcm);
-        gnc_quote_source_set_fq_installed("TestSuite", g_list_prepend(nullptr, (void*)"yahoo_json"));
+        GList *sources = g_list_prepend(nullptr, (void*)"yahoo_json");
+        gnc_quote_source_set_fq_installed("TestSuite", sources);
+        g_list_free(sources);
     }
     ~GncQuotesTest() {
         gnc_clear_current_session();
diff --git a/libgnucash/backend/sql/gnc-sql-column-table-entry.cpp b/libgnucash/backend/sql/gnc-sql-column-table-entry.cpp
index 6acdcbb258..cdd78fd13e 100644
--- a/libgnucash/backend/sql/gnc-sql-column-table-entry.cpp
+++ b/libgnucash/backend/sql/gnc-sql-column-table-entry.cpp
@@ -99,9 +99,11 @@ GncSqlColumnTableEntry::add_objectref_guid_to_query (QofIdTypeConst obj_name,
     auto inst = get_row_value_from_object<QofInstance*>(obj_name, pObject);
     if (inst == nullptr) return;
     auto guid = qof_instance_get_guid (inst);
-    if (guid != nullptr)
-        vec.emplace_back (std::make_pair (std::string{m_col_name},
-                                          quote_string(guid_to_string(guid))));
+    if (guid != nullptr) {
+        gchar *guid_s = guid_to_string(guid);
+        vec.emplace_back (std::make_pair (std::string{m_col_name}, quote_string(guid_s)));
+        g_free(guid_s);
+    }
 }
 
 void
@@ -356,9 +358,9 @@ GncSqlColumnTableEntryImpl<CT_GUID>::add_to_query(QofIdTypeConst obj_name,
 
     if (s != nullptr)
     {
-
-        vec.emplace_back (std::make_pair (std::string{m_col_name},
-                                          quote_string(guid_to_string(s))));
+        gchar *guid_s = guid_to_string(s);
+        vec.emplace_back (std::make_pair (std::string{m_col_name}, quote_string(guid_s)));
+        g_free(guid_s);
         return;
     }
 }
diff --git a/libgnucash/core-utils/test/gtest-path-utilities.cpp b/libgnucash/core-utils/test/gtest-path-utilities.cpp
index f2103fbc1a..7ca81abec3 100644
--- a/libgnucash/core-utils/test/gtest-path-utilities.cpp
+++ b/libgnucash/core-utils/test/gtest-path-utilities.cpp
@@ -8,6 +8,12 @@
 
 #include <gtest/gtest.h>
 
+
+/* Variant of EXPECT_STREQ that calls g_free()
+ * on its first argument after the check */
+#define EXPECT_STREQ_GFREE(a, b) do { char *p_; EXPECT_STREQ(p_ = (a), (b)); g_free(p_); } while (0)
+
+
 struct PathTest : public testing::Test
 {
     PathTest() : m_prefix{nullptr} {}
@@ -36,14 +42,14 @@ struct PathTest : public testing::Test
 TEST_F(PathTest, gnc_path_get_prefix)
 {
 #ifdef ENABLE_BINRELOC
-    EXPECT_STREQ(gnc_path_get_prefix(), m_prefix);
+    EXPECT_STREQ_GFREE(gnc_path_get_prefix(), m_prefix);
 #else
     g_setenv("GNC_UNINSTALLED", "1", TRUE);
     g_setenv("GNC_BUILDDIR", m_prefix, 1);
-    EXPECT_STREQ(gnc_path_get_prefix(), m_prefix);
+    EXPECT_STREQ_GFREE(gnc_path_get_prefix(), m_prefix);
     g_unsetenv("GNC_UNINSTALLED");
     g_unsetenv("GNC_BUILDDIR");
-    EXPECT_STREQ(gnc_path_get_prefix(), PREFIX);
+    EXPECT_STREQ_GFREE(gnc_path_get_prefix(), PREFIX);
 #endif
 }
 
@@ -53,16 +59,16 @@ TEST_F(PathTest, gnc_path_get_bindir)
     gchar *binpath = g_build_filename(m_prefix, dirname, NULL);
     g_free(dirname);
 #ifdef ENABLE_BINRELOC
-    EXPECT_STREQ(gnc_path_get_bindir(), binpath);
+    EXPECT_STREQ_GFREE(gnc_path_get_bindir(), binpath);
     g_free(binpath);
 #else
     g_setenv("GNC_UNINSTALLED", "1", TRUE);
     g_setenv("GNC_BUILDDIR", m_prefix, 1);
-    EXPECT_STREQ(gnc_path_get_bindir(), binpath);
+    EXPECT_STREQ_GFREE(gnc_path_get_bindir(), binpath);
     g_free(binpath);
     g_unsetenv("GNC_UNINSTALLED");
     g_unsetenv("GNC_BUILDDIR");
-    EXPECT_STREQ(gnc_path_get_bindir(), BINDIR);
+    EXPECT_STREQ_GFREE(gnc_path_get_bindir(), BINDIR);
 #endif
 }
 
@@ -72,16 +78,16 @@ TEST_F(PathTest, gnc_path_get_libdir)
     gchar *libpath = g_build_filename(m_prefix, dirname, NULL);
     g_free(dirname);
 #ifdef ENABLE_BINRELOC
-    EXPECT_STREQ(gnc_path_get_libdir(), libpath);
+    EXPECT_STREQ_GFREE(gnc_path_get_libdir(), libpath);
     g_free(libpath);
 #else
     g_setenv("GNC_UNINSTALLED", "1", TRUE);
     g_setenv("GNC_BUILDDIR", m_prefix, 1);
-    EXPECT_STREQ(gnc_path_get_libdir(), libpath);
+    EXPECT_STREQ_GFREE(gnc_path_get_libdir(), libpath);
     g_free(libpath);
     g_unsetenv("GNC_UNINSTALLED");
     g_unsetenv("GNC_BUILDDIR");
-    EXPECT_STREQ(gnc_path_get_libdir(), LIBDIR);
+    EXPECT_STREQ_GFREE(gnc_path_get_libdir(), LIBDIR);
 #endif
 }
 
@@ -91,16 +97,16 @@ TEST_F(PathTest, gnc_path_get_datadir)
     gchar *datapath = g_build_filename(m_prefix, dirname, NULL);
     g_free(dirname);
 #ifdef ENABLE_BINRELOC
-    EXPECT_STREQ(gnc_path_get_datadir(), datapath);
+    EXPECT_STREQ_GFREE(gnc_path_get_datadir(), datapath);
     g_free(datapath);
 #else
     g_setenv("GNC_UNINSTALLED", "1", TRUE);
     g_setenv("GNC_BUILDDIR", m_prefix, 1);
-    EXPECT_STREQ(gnc_path_get_datadir(), datapath);
+    EXPECT_STREQ_GFREE(gnc_path_get_datadir(), datapath);
     g_free(datapath);
     g_unsetenv("GNC_UNINSTALLED");
     g_unsetenv("GNC_BUILDDIR");
-    EXPECT_STREQ(gnc_path_get_datadir(), DATADIR);
+    EXPECT_STREQ_GFREE(gnc_path_get_datadir(), DATADIR);
 #endif
 }
 
@@ -110,17 +116,17 @@ TEST_F(PathTest, gnc_path_get_sysconfdir)
     gchar *sysconfpath = g_build_filename(m_prefix, dirname, PROJECT_NAME, NULL);
     g_free(dirname);
 #ifdef ENABLE_BINRELOC
-    EXPECT_STREQ(gnc_path_get_pkgsysconfdir(), sysconfpath);
+    EXPECT_STREQ_GFREE(gnc_path_get_pkgsysconfdir(), sysconfpath);
     g_free(sysconfpath);
 #else
     g_setenv("GNC_UNINSTALLED", "1", TRUE);
     g_setenv("GNC_BUILDDIR", m_prefix, 1);
-    EXPECT_STREQ(gnc_path_get_pkgsysconfdir(), sysconfpath);
+    EXPECT_STREQ_GFREE(gnc_path_get_pkgsysconfdir(), sysconfpath);
     g_free(sysconfpath);
     g_unsetenv("GNC_UNINSTALLED");
     g_unsetenv("GNC_BUILDDIR");
     sysconfpath = g_build_filename(SYSCONFDIR, PROJECT_NAME, NULL);
-    EXPECT_STREQ(gnc_path_get_pkgsysconfdir(), sysconfpath);
+    EXPECT_STREQ_GFREE(gnc_path_get_pkgsysconfdir(), sysconfpath);
     g_free(sysconfpath);
 #endif
 }
diff --git a/libgnucash/engine/test/test-guid.cpp b/libgnucash/engine/test/test-guid.cpp
index bc90905e3d..2f14d2bc66 100644
--- a/libgnucash/engine/test/test-guid.cpp
+++ b/libgnucash/engine/test/test-guid.cpp
@@ -28,9 +28,13 @@
 #include <guid.hpp>
 #include <glib.h>
 
+#include <vector>
+#include <memory>
+
 #include <config.h>
 #include <ctype.h>
 #include "cashobjects.h"
+#include "qofid.h"
 #include "test-stuff.h"
 #include "test-engine-stuff.h"
 #include "qof.h"
@@ -50,13 +54,17 @@ static void test_null_guid(void)
     guid_free(gp);
 }
 
+static void
+free_entry (QofInstance* inst, void* data)
+{
+    g_object_unref (G_OBJECT(inst));
+}
+
 static void
 run_test (void)
 {
-    int i;
     QofSession *sess;
     QofBook *book;
-    QofInstance *ent;
     QofCollection *col;
     QofIdType type;
     GncGUID guid;
@@ -68,12 +76,10 @@ run_test (void)
     col = qof_book_get_collection (book, "asdf");
     type = qof_collection_get_type (col);
 
-    for (i = 0; i < NENT; i++)
+    for (int i = 0; i < NENT; i++)
     {
-        ent = static_cast<QofInstance*>(g_object_new(QOF_TYPE_INSTANCE, NULL));
         guid_replace(&guid);
-        ent = static_cast<QofInstance*>(g_object_new(QOF_TYPE_INSTANCE,
-                                                     "guid", &guid, NULL));
+        auto ent = QOF_INSTANCE(g_object_new(QOF_TYPE_INSTANCE, "guid", &guid, NULL));
         do_test ((NULL == qof_collection_lookup_entity (col, &guid)),
                  "duplicate guid");
         ent->e_type = type;
@@ -83,6 +89,7 @@ run_test (void)
     }
 
     /* Make valgrind happy -- destroy the session. */
+    qof_collection_foreach(col, free_entry, nullptr);
     qof_session_destroy(sess);
 }
 

commit 0d86be6d2adeae336028126c817bd592ed5a2fa0
Author: Maarten Bosmans <mkbosmans at gmail.com>
Date:   Fri Mar 17 21:16:01 2023 +0100

    [test] Properly destroy resources on end of tests
    
    This fixes memory leaks that are only present in testing code.
    Not very useful on itself, but it does make it easier to fix memory
    leaks and other AddressSanitizer problems in actual gnucash code later.

diff --git a/libgnucash/app-utils/test/gtest-gnc-quotes.cpp b/libgnucash/app-utils/test/gtest-gnc-quotes.cpp
index d3295585a3..bd6739ea69 100644
--- a/libgnucash/app-utils/test/gtest-gnc-quotes.cpp
+++ b/libgnucash/app-utils/test/gtest-gnc-quotes.cpp
@@ -270,6 +270,7 @@ TEST_F(GncQuotesTest, fetch_one_commodity)
                                   gnc_price_get_value(price)));
     EXPECT_STREQ("Finance::Quote", gnc_price_get_source_string(price));
     EXPECT_STREQ("last", gnc_price_get_typestr(price));
+    gnc_price_unref(price);
 }
 
 TEST_F(GncQuotesTest, fetch_one_currency)
@@ -300,6 +301,7 @@ TEST_F(GncQuotesTest, fetch_one_currency)
                                   gnc_price_get_value(price)));
     EXPECT_STREQ("Finance::Quote", gnc_price_get_source_string(price));
     EXPECT_STREQ("last", gnc_price_get_typestr(price));
+    gnc_price_unref(price);
 }
 
 TEST_F(GncQuotesTest, no_currency)
@@ -366,6 +368,7 @@ TEST_F(GncQuotesTest, no_date)
                                   gnc_price_get_value(price)));
     EXPECT_STREQ("Finance::Quote", gnc_price_get_source_string(price));
     EXPECT_STREQ("last", gnc_price_get_typestr(price));
+    gnc_price_unref(price);
 }
 
 TEST_F(GncQuotesTest, test_version)
diff --git a/libgnucash/engine/test/gtest-qofquerycore.cpp b/libgnucash/engine/test/gtest-qofquerycore.cpp
index 81983bb510..2a5d340e8a 100644
--- a/libgnucash/engine/test/gtest-qofquerycore.cpp
+++ b/libgnucash/engine/test/gtest-qofquerycore.cpp
@@ -55,6 +55,7 @@ TEST_F(QofQueryCoreTest, construct_predicate_string)
     EXPECT_EQ (QOF_STRING_MATCH_NORMAL, pdata->options);
     EXPECT_STREQ ("Test",               pdata->matchstring);
     EXPECT_EQ (FALSE,                   pdata->is_regex);
+    qof_query_core_predicate_free ((QofQueryPredData*) pdata);
 }
 
 TEST_F(QofQueryCoreTest, construct_predicate_date)
@@ -69,6 +70,7 @@ TEST_F(QofQueryCoreTest, construct_predicate_date)
     EXPECT_EQ (QOF_COMPARE_LT,     pdata->pd.how);
     EXPECT_EQ (QOF_DATE_MATCH_DAY, pdata->options);
     EXPECT_EQ (1524772012,         pdata->date);
+    qof_query_core_predicate_free ((QofQueryPredData*) pdata);
 }
 
 TEST_F(QofQueryCoreTest, construct_predicate_numeric)
@@ -83,6 +85,7 @@ TEST_F(QofQueryCoreTest, construct_predicate_numeric)
     EXPECT_EQ (QOF_COMPARE_LTE,          pdata->pd.how);
     EXPECT_EQ (QOF_NUMERIC_MATCH_CREDIT, pdata->options);
     EXPECT_TRUE (gnc_numeric_eq({ 500, 100 }, pdata->amount));
+    qof_query_core_predicate_free ((QofQueryPredData*) pdata);
 }
 
 TEST_F(QofQueryCoreTest, construct_predicate_guid)
@@ -98,6 +101,7 @@ TEST_F(QofQueryCoreTest, construct_predicate_guid)
     EXPECT_EQ (QOF_GUID_MATCH_ANY, pdata->options);
     EXPECT_TRUE (guid_equal (guid, (const GncGUID*)pdata->guids->data));
     EXPECT_EQ (NULL,               pdata->guids->next);
+    qof_query_core_predicate_free ((QofQueryPredData*) pdata);
 }
 
 TEST_F(QofQueryCoreTest, construct_predicate_int32)
@@ -110,6 +114,7 @@ TEST_F(QofQueryCoreTest, construct_predicate_int32)
     EXPECT_STREQ (QOF_TYPE_INT32, pdata->pd.type_name);
     EXPECT_EQ (QOF_COMPARE_EQUAL, pdata->pd.how);
     EXPECT_EQ (-613,              pdata->val);
+    qof_query_core_predicate_free ((QofQueryPredData*) pdata);
 }
 
 TEST_F(QofQueryCoreTest, query_construct_predicate_int64)
@@ -122,6 +127,7 @@ TEST_F(QofQueryCoreTest, query_construct_predicate_int64)
     EXPECT_STREQ (QOF_TYPE_INT64, pdata->pd.type_name);
     EXPECT_EQ (QOF_COMPARE_GT,    pdata->pd.how);
     EXPECT_EQ (1000000,           pdata->val);
+    qof_query_core_predicate_free ((QofQueryPredData*) pdata);
 }
 
 TEST_F(QofQueryCoreTest, construct_predicate_double)
@@ -134,6 +140,7 @@ TEST_F(QofQueryCoreTest, construct_predicate_double)
     EXPECT_STREQ (QOF_TYPE_DOUBLE, pdata->pd.type_name);
     EXPECT_EQ (QOF_COMPARE_GTE,    pdata->pd.how);
     EXPECT_EQ (10.05,              pdata->val);
+    qof_query_core_predicate_free ((QofQueryPredData*) pdata);
 }
 
 TEST_F(QofQueryCoreTest, construct_predicate_boolean)
@@ -146,6 +153,7 @@ TEST_F(QofQueryCoreTest, construct_predicate_boolean)
     EXPECT_STREQ (QOF_TYPE_BOOLEAN, pdata->pd.type_name);
     EXPECT_EQ (QOF_COMPARE_NEQ,     pdata->pd.how);
     EXPECT_EQ (TRUE,                pdata->val);
+    qof_query_core_predicate_free ((QofQueryPredData*) pdata);
 }
 
 TEST_F(QofQueryCoreTest, construct_predicate_char)
@@ -159,6 +167,7 @@ TEST_F(QofQueryCoreTest, construct_predicate_char)
     EXPECT_EQ (QOF_COMPARE_EQUAL,  pdata->pd.how);
     EXPECT_EQ (QOF_CHAR_MATCH_ANY, pdata->options);
     EXPECT_STREQ ("Foo",           pdata->char_list);
+    qof_query_core_predicate_free ((QofQueryPredData*) pdata);
 }
 
 TEST_F(QofQueryCoreTest, date_predicate_copy)
@@ -170,11 +179,12 @@ TEST_F(QofQueryCoreTest, date_predicate_copy)
         1524772012
     );
     pdata2 = (query_date_def*) qof_query_core_predicate_copy ((QofQueryPredData*)pdata);
-
     EXPECT_STREQ (pdata2->pd.type_name, pdata->pd.type_name);
     EXPECT_EQ (pdata2->pd.how,          pdata->pd.how);
     EXPECT_EQ (pdata2->options,         pdata->options);
     EXPECT_EQ (pdata2->date,            pdata->date);
+    qof_query_core_predicate_free ((QofQueryPredData*) pdata);
+    qof_query_core_predicate_free ((QofQueryPredData*) pdata2);
 }
 
 TEST_F(QofQueryCoreTest, date_predicate_get_date)
@@ -186,9 +196,9 @@ TEST_F(QofQueryCoreTest, date_predicate_get_date)
         QOF_DATE_MATCH_DAY,
         1524772012
     );
-
     EXPECT_TRUE (qof_query_date_predicate_get_date(pdata, &date));
     EXPECT_EQ (1524772012, date);
+    qof_query_core_predicate_free (pdata);
 }
 
 TEST_F(QofQueryCoreTest, numeric_predicate_get_date)
@@ -200,6 +210,6 @@ TEST_F(QofQueryCoreTest, numeric_predicate_get_date)
         QOF_NUMERIC_MATCH_CREDIT,
         { 1000, 100 }
     );
-
     EXPECT_FALSE (qof_query_date_predicate_get_date(pdata, &date));
+    qof_query_core_predicate_free (pdata);
 }
diff --git a/libgnucash/engine/test/test-account-object.cpp b/libgnucash/engine/test/test-account-object.cpp
index 004abb054c..8487583dd7 100644
--- a/libgnucash/engine/test/test-account-object.cpp
+++ b/libgnucash/engine/test/test-account-object.cpp
@@ -80,8 +80,7 @@ run_test (void)
 
     /*****/
 
-    qof_session_end (sess);
-
+    qof_session_destroy (sess);
 }
 
 int
diff --git a/libgnucash/engine/test/test-lots.cpp b/libgnucash/engine/test/test-lots.cpp
index 7bf3a3857d..88bd3cfe23 100644
--- a/libgnucash/engine/test/test-lots.cpp
+++ b/libgnucash/engine/test/test-lots.cpp
@@ -79,7 +79,7 @@ test_lot_kvp ()
     g_assert_cmpstr (gnc_lot_get_notes (lot), ==, NULL);
 
     gnc_lot_destroy (lot);
-    qof_session_end (sess);
+    qof_session_destroy (sess);
 }
 
 static void
@@ -108,7 +108,7 @@ run_test (void)
      * XXX not implemented
      */
     success ("automatic lot scrubbing lightly tested and seem to work");
-    qof_session_end (sess);
+    qof_session_destroy (sess);
 
 }
 
diff --git a/libgnucash/engine/test/test-query.cpp b/libgnucash/engine/test/test-query.cpp
index 112a1ad6b1..165b37b3cc 100644
--- a/libgnucash/engine/test/test-query.cpp
+++ b/libgnucash/engine/test/test-query.cpp
@@ -36,12 +36,12 @@ test_trans_query (Transaction *trans, gpointer data)
 {
     QofBook *book = QOF_BOOK(data);
     GList *list;
-    QofQuery *q;
 
-    q = make_trans_query (trans, ALL_QT);
+    QofQuery *q = make_trans_query (trans, ALL_QT);
     qof_query_set_book (q, book);
-
     list = xaccQueryGetTransactions (q, QUERY_TXN_MATCH_ANY);
+    qof_query_destroy (q);
+
     if (g_list_length (list) != 1)
     {
         failure_args ("test number returned", __FILE__, __LINE__,
@@ -59,7 +59,6 @@ test_trans_query (Transaction *trans, gpointer data)
     }
 
     success ("found right transaction");
-    qof_query_destroy (q);
     g_list_free (list);
 
     return 0;
@@ -80,7 +79,7 @@ run_test (void)
 
     xaccAccountTreeForEachTransaction (root, test_trans_query, book);
 
-    qof_session_end (session);
+    qof_session_destroy (session);
 }
 
 int

commit ed3fe008807edc77b1e201147d11a3fe371d4c55
Author: Maarten Bosmans <mkbosmans at gmail.com>
Date:   Fri Mar 17 21:10:44 2023 +0100

    [test] Use test fixture to initialize and destroy resources

diff --git a/libgnucash/app-utils/test/gtest-gnc-quotes.cpp b/libgnucash/app-utils/test/gtest-gnc-quotes.cpp
index 8a5221d19d..d3295585a3 100644
--- a/libgnucash/app-utils/test/gtest-gnc-quotes.cpp
+++ b/libgnucash/app-utils/test/gtest-gnc-quotes.cpp
@@ -128,6 +128,7 @@ protected:
     }
     ~GncQuotesTest() {
         gnc_clear_current_session();
+        qof_close();
     }
 
     QofSession* m_session;
diff --git a/libgnucash/core-utils/test/gtest-path-utilities.cpp b/libgnucash/core-utils/test/gtest-path-utilities.cpp
index 3f6f8459dd..f2103fbc1a 100644
--- a/libgnucash/core-utils/test/gtest-path-utilities.cpp
+++ b/libgnucash/core-utils/test/gtest-path-utilities.cpp
@@ -26,10 +26,13 @@ struct PathTest : public testing::Test
     {
         if (m_prefix)
             g_free(m_prefix);
+        /* Clear the statically allocated exe string */
+        gnc_gbr_set_exe(NULL);
     }
     char *m_prefix;
 };
 
+
 TEST_F(PathTest, gnc_path_get_prefix)
 {
 #ifdef ENABLE_BINRELOC
diff --git a/libgnucash/engine/test/gtest-qofquerycore.cpp b/libgnucash/engine/test/gtest-qofquerycore.cpp
index 353bee051c..81983bb510 100644
--- a/libgnucash/engine/test/gtest-qofquerycore.cpp
+++ b/libgnucash/engine/test/gtest-qofquerycore.cpp
@@ -29,7 +29,19 @@
 #include "../qofquerycore-p.h"
 #include <gtest/gtest.h>
 
-TEST(qof_query_construct_predicate, string)
+
+class QofQueryCoreTest : public ::testing::Test {
+    protected:
+    QofQueryCoreTest() {
+        qof_query_core_init();
+    }
+    ~QofQueryCoreTest() {
+        qof_query_core_shutdown();
+    }
+};
+
+
+TEST_F(QofQueryCoreTest, construct_predicate_string)
 {
     query_string_def *pdata;
     pdata = (query_string_def*)qof_query_string_predicate(
@@ -45,7 +57,7 @@ TEST(qof_query_construct_predicate, string)
     EXPECT_EQ (FALSE,                   pdata->is_regex);
 }
 
-TEST(qof_query_construct_predicate, date)
+TEST_F(QofQueryCoreTest, construct_predicate_date)
 {
     query_date_def *pdata;
     pdata = (query_date_def*)qof_query_date_predicate(
@@ -59,7 +71,7 @@ TEST(qof_query_construct_predicate, date)
     EXPECT_EQ (1524772012,         pdata->date);
 }
 
-TEST(qof_query_construct_predicate, numeric)
+TEST_F(QofQueryCoreTest, construct_predicate_numeric)
 {
     query_numeric_def *pdata;
     pdata = (query_numeric_def*)qof_query_numeric_predicate(
@@ -73,7 +85,7 @@ TEST(qof_query_construct_predicate, numeric)
     EXPECT_TRUE (gnc_numeric_eq({ 500, 100 }, pdata->amount));
 }
 
-TEST(qof_query_construct_predicate, guid)
+TEST_F(QofQueryCoreTest, construct_predicate_guid)
 {
     GncGUID *guid = guid_new();
     GList *guidlist = g_list_prepend (NULL, guid);
@@ -88,7 +100,7 @@ TEST(qof_query_construct_predicate, guid)
     EXPECT_EQ (NULL,               pdata->guids->next);
 }
 
-TEST(qof_query_construct_predicate, int32)
+TEST_F(QofQueryCoreTest, construct_predicate_int32)
 {
     query_int32_def *pdata;
     pdata = (query_int32_def*)qof_query_int32_predicate(
@@ -100,7 +112,7 @@ TEST(qof_query_construct_predicate, int32)
     EXPECT_EQ (-613,              pdata->val);
 }
 
-TEST(qof_query_construct_predicate, int64)
+TEST_F(QofQueryCoreTest, query_construct_predicate_int64)
 {
     query_int64_def *pdata;
     pdata = (query_int64_def*)qof_query_int64_predicate(
@@ -112,7 +124,7 @@ TEST(qof_query_construct_predicate, int64)
     EXPECT_EQ (1000000,           pdata->val);
 }
 
-TEST(qof_query_construct_predicate, double)
+TEST_F(QofQueryCoreTest, construct_predicate_double)
 {
     query_double_def *pdata;
     pdata = (query_double_def*)qof_query_double_predicate(
@@ -124,7 +136,7 @@ TEST(qof_query_construct_predicate, double)
     EXPECT_EQ (10.05,              pdata->val);
 }
 
-TEST(qof_query_construct_predicate, boolean)
+TEST_F(QofQueryCoreTest, construct_predicate_boolean)
 {
     query_boolean_def *pdata;
     pdata = (query_boolean_def*)qof_query_boolean_predicate(
@@ -136,7 +148,7 @@ TEST(qof_query_construct_predicate, boolean)
     EXPECT_EQ (TRUE,                pdata->val);
 }
 
-TEST(qof_query_construct_predicate, char)
+TEST_F(QofQueryCoreTest, construct_predicate_char)
 {
     query_char_def *pdata;
     pdata = (query_char_def*)qof_query_char_predicate(
@@ -149,9 +161,8 @@ TEST(qof_query_construct_predicate, char)
     EXPECT_STREQ ("Foo",           pdata->char_list);
 }
 
-TEST(qof_query_core_predicate_copy, date)
+TEST_F(QofQueryCoreTest, date_predicate_copy)
 {
-    qof_query_core_init();
     query_date_def *pdata, *pdata2;
     pdata = (query_date_def*)qof_query_date_predicate(
         QOF_COMPARE_LT,
@@ -166,9 +177,8 @@ TEST(qof_query_core_predicate_copy, date)
     EXPECT_EQ (pdata2->date,            pdata->date);
 }
 
-TEST(qof_query_core_predicate_get_date, date)
+TEST_F(QofQueryCoreTest, date_predicate_get_date)
 {
-    qof_query_core_init();
     time64 date;
     QofQueryPredData *pdata;
     pdata = qof_query_date_predicate(
@@ -181,9 +191,8 @@ TEST(qof_query_core_predicate_get_date, date)
     EXPECT_EQ (1524772012, date);
 }
 
-TEST(qof_query_core_predicate_get_date, numeric)
+TEST_F(QofQueryCoreTest, numeric_predicate_get_date)
 {
-    qof_query_core_init();
     time64 date;
     QofQueryPredData *pdata;
     pdata = qof_query_numeric_predicate(

commit 35aeed45ec4b8c1246dbec71de36e5a2136ece07
Author: Maarten Bosmans <mkbosmans at gmail.com>
Date:   Thu Mar 16 22:08:10 2023 +0100

    [engine] Remove two replace functions from KvpValue
    
    These were not used outside a test.
    
    And that test was not leak free, as a result of the functions not doing
    what they are supposed to do when the current value is not of the type
    that is expected. (NULL is returned, but the value is not replaced)

diff --git a/libgnucash/engine/kvp-value.cpp b/libgnucash/engine/kvp-value.cpp
index 456138a39a..ac3329016f 100644
--- a/libgnucash/engine/kvp-value.cpp
+++ b/libgnucash/engine/kvp-value.cpp
@@ -99,26 +99,6 @@ KvpValueImpl::get_type() const noexcept
     return KvpValue::Type::INVALID;
 }
 
-KvpFrame *
-KvpValueImpl::replace_frame_nc (KvpFrame * new_value) noexcept
-{
-    if (datastore.type() != type_id<KvpFrame *>())
-        return {};
-    auto ret = boost::get<KvpFrame *>(datastore);
-    datastore = new_value;
-    return ret;
-}
-
-GList *
-KvpValueImpl::replace_glist_nc (GList * new_value) noexcept
-{
-    if (datastore.type() != type_id<GList *>())
-        return {};
-    auto ret = boost::get<GList *>(datastore);
-    datastore = new_value;
-    return ret;
-}
-
 struct to_string_visitor : boost::static_visitor<void>
 {
     std::ostringstream & output;
diff --git a/libgnucash/engine/kvp-value.hpp b/libgnucash/engine/kvp-value.hpp
index ea2f2003b9..b61f98911c 100644
--- a/libgnucash/engine/kvp-value.hpp
+++ b/libgnucash/engine/kvp-value.hpp
@@ -100,22 +100,6 @@ struct KvpValueImpl
      */
     ~KvpValueImpl() noexcept;
 
-    /**
-     * Replaces the frame within this KvpValueImpl.
-     *
-     * If this KvpValueImpl doesn't contain a KvpFrame, nullptr
-     * is returned. Otherwise, the old KvpFrame * is returned.
-     */
-    KvpFrame * replace_frame_nc (KvpFrame *) noexcept;
-
-    /**
-     * Replaces the glist within this KvpValueImpl.
-     *
-     * If this KvpValueImpl doesn't contain a GList, nullptr
-     * is returned. Otherwise, the old GList * is returned.
-     */
-    GList * replace_glist_nc (GList *) noexcept;
-
     /**
      * Adds another value to this KvpValueImpl.
      *
diff --git a/libgnucash/engine/test/test-kvp-value.cpp b/libgnucash/engine/test/test-kvp-value.cpp
index b4b8772b67..7cf67b0c6b 100644
--- a/libgnucash/engine/test/test-kvp-value.cpp
+++ b/libgnucash/engine/test/test-kvp-value.cpp
@@ -30,19 +30,6 @@
 #include <memory>
 #include <gtest/gtest.h>
 
-TEST (KvpValueTest, Replace_Frame)
-{
-    auto f1 = new KvpFrameImpl;
-    std::unique_ptr<KvpValueImpl> v1 {new KvpValueImpl {f1}};
-    auto f2 = new KvpFrameImpl;
-    EXPECT_EQ (f1, v1->replace_frame_nc (f2));
-    v1->set (5.2);
-    /*f1 and f2 should be deleted now*/
-    f1 = new KvpFrameImpl;
-    EXPECT_EQ (nullptr, v1->replace_frame_nc (f1));
-    delete f1;
-}
-
 TEST (KvpValueTest, Equality)
 {
     std::unique_ptr<KvpValueImpl> v1 {new KvpValueImpl { (int64_t)1}};

commit bd94965d9b0eb2f64a188aa44ec86b679c31a993
Author: Maarten Bosmans <mkbosmans at gmail.com>
Date:   Thu Mar 16 19:55:35 2023 +0100

    [backend/xml] Remove unused string copy

diff --git a/libgnucash/backend/xml/sixtp-dom-generators.cpp b/libgnucash/backend/xml/sixtp-dom-generators.cpp
index ed849d5dbb..3a90aae72e 100644
--- a/libgnucash/backend/xml/sixtp-dom-generators.cpp
+++ b/libgnucash/backend/xml/sixtp-dom-generators.cpp
@@ -205,12 +205,8 @@ double_to_string (double value)
 static void
 add_text_to_node (xmlNodePtr node, const gchar* type, gchar* val)
 {
-    gchar* newtype = g_strdup (type);
-    gchar* newval = g_strdup (val);
     xmlSetProp (node, BAD_CAST "type", BAD_CAST type);
     xmlNodeSetContent (node, checked_char_cast (val));
-    g_free (newtype);
-    g_free (newval);
 }
 
 static void add_kvp_slot (const char* key, KvpValue* value, void* data);



Summary of changes:
 libgnucash/app-utils/gnc-quotes.cpp                |  7 +--
 libgnucash/app-utils/test/gtest-gnc-quotes.cpp     | 23 ++++++---
 .../backend/sql/gnc-sql-column-table-entry.cpp     | 14 +++---
 libgnucash/backend/xml/sixtp-dom-generators.cpp    |  4 --
 .../core-utils/test/gtest-path-utilities.cpp       | 39 +++++++++------
 libgnucash/engine/kvp-value.cpp                    | 20 --------
 libgnucash/engine/kvp-value.hpp                    | 16 -------
 libgnucash/engine/test/gtest-qofquerycore.cpp      | 55 +++++++++++++++-------
 libgnucash/engine/test/test-account-object.cpp     |  3 +-
 libgnucash/engine/test/test-guid.cpp               | 19 +++++---
 libgnucash/engine/test/test-kvp-value.cpp          | 13 -----
 libgnucash/engine/test/test-lots.cpp               |  4 +-
 libgnucash/engine/test/test-query.cpp              |  9 ++--
 13 files changed, 109 insertions(+), 117 deletions(-)



More information about the gnucash-changes mailing list