gnucash stable: Multiple changes pushed
John Ralls
jralls at code.gnucash.org
Sat Jan 17 20:15:49 EST 2026
Updated via https://github.com/Gnucash/gnucash/commit/e5fe2f3e (commit)
via https://github.com/Gnucash/gnucash/commit/113af562 (commit)
via https://github.com/Gnucash/gnucash/commit/7f0d66d4 (commit)
via https://github.com/Gnucash/gnucash/commit/be5933a1 (commit)
via https://github.com/Gnucash/gnucash/commit/0e3853bf (commit)
via https://github.com/Gnucash/gnucash/commit/6e9a20df (commit)
via https://github.com/Gnucash/gnucash/commit/a2cc9881 (commit)
via https://github.com/Gnucash/gnucash/commit/9079d873 (commit)
from https://github.com/Gnucash/gnucash/commit/cad455e3 (commit)
commit e5fe2f3e0ee3d5a768ef29aae296c3559d626466
Merge: cad455e3c0 113af56272
Author: John Ralls <jralls at ceridwen.us>
Date: Sat Jan 17 17:11:27 2026 -0800
Merge Stefan Koch's 'sk-unittest-qofid' into stable.
commit 113af56272deac7deeb728c9a7caf5ad37190b9a
Author: Stefan Koch <stefan.koch at micromeritics.com>
Date: Thu Jan 15 13:14:36 2026 -0500
fixup: Review comments about style of code.
diff --git a/libgnucash/engine/test/test-qofid.cpp b/libgnucash/engine/test/test-qofid.cpp
index b9fbd4b76f..3e0b5cf34f 100644
--- a/libgnucash/engine/test/test-qofid.cpp
+++ b/libgnucash/engine/test/test-qofid.cpp
@@ -27,11 +27,11 @@
#include <algorithm>
#include <vector>
-#include "../qofid.h"
-#include "../qofid-p.h"
-#include "../qofbook.h"
-#include "../gncJob.h"
-#include "../qofinstance-p.h"
+#include "qofid.h"
+#include "qofid-p.h"
+#include "qofbook.h"
+#include "gncJob.h"
+#include "qofinstance-p.h"
// This is the error handler that does nothing but count how many times it is
// called. The handler_count is incremented every time.
@@ -150,19 +150,17 @@ static gint cb_compare(GncJob* job1, GncJob* job2)
return g_strcmp0(gncJobGetID(job1), gncJobGetID(job2));
}
-static bool is_in(const char* str, const std::vector<const char*> str_vect)
+static bool is_in(const char* str, const std::vector<const char*>& str_vect)
{
- for(auto id : str_vect)
- if(!g_strcmp0(id, str))
- return true;
- return false;
+ return std::find_if(str_vect.begin(), str_vect.end(),
+ [str](auto id)->bool{ return !g_strcmp0(id, str); }) != str_vect.end();
}
TEST(QofIDTest, collection_foreach)
{
auto col = qof_collection_new(GNC_ID_JOB);
auto book = qof_book_new();
- auto job_ids = std::vector<const char*>{"zzz", "ggg", "qqq", "aaa"};
+ std::vector<const char*> job_ids{"zzz", "ggg", "qqq", "aaa"};
std::vector<GncJob*> jobs;
for(auto id : job_ids)
{
commit 7f0d66d43977816de38b70554ad5555e6ab8fbe2
Author: Stefan Koch <stefan.koch at micromeritics.com>
Date: Thu Jan 15 13:00:52 2026 -0500
fixup: Add the new test-qofid.cpp file the the source distribution.
diff --git a/libgnucash/engine/test/CMakeLists.txt b/libgnucash/engine/test/CMakeLists.txt
index 137ad90252..6d5b073694 100644
--- a/libgnucash/engine/test/CMakeLists.txt
+++ b/libgnucash/engine/test/CMakeLists.txt
@@ -242,6 +242,7 @@ set(test_engine_SOURCES_DIST
test-numeric.cpp
test-object.c
test-qof.c
+ test-qofid.cpp
test-qofbook.c
test-qofinstance.cpp
test-qofobject.c
commit be5933a1ec6dfc0c6d0211d62f330872c092d788
Author: Stefan Koch <stefan.koch at micromeritics.com>
Date: Thu Jan 15 11:52:59 2026 -0500
fixup: Remove memory leaks in the test code.
diff --git a/libgnucash/engine/test/test-qofid.cpp b/libgnucash/engine/test/test-qofid.cpp
index 9897a87eee..b9fbd4b76f 100644
--- a/libgnucash/engine/test/test-qofid.cpp
+++ b/libgnucash/engine/test/test-qofid.cpp
@@ -134,6 +134,10 @@ TEST(QofIDTest, collection_compare)
qof_instance_set_guid(QOF_INSTANCE(book4), guid_null());
EXPECT_EQ(-1, qof_collection_compare(target, merge));
qof_instance_set_guid(QOF_INSTANCE(book4), old_guid);
+
+ qof_collection_destroy(target);
+ qof_collection_destroy(merge);
+ qof_collection_destroy(incompat);
}
static void cb_visit(GncJob* job, std::vector<const char*>* results_ptr)
@@ -182,6 +186,8 @@ TEST(QofIDTest, collection_foreach)
EXPECT_STREQ(results[1], "ggg");
EXPECT_STREQ(results[2], "qqq");
EXPECT_STREQ(results[3], "zzz");
+
+ qof_collection_destroy(col);
}
TEST(QofIDTest, collection_print_dirty)
@@ -202,4 +208,5 @@ TEST(QofIDTest, collection_print_dirty)
output = testing::internal::GetCapturedStdout();
EXPECT_EQ(output.find("Book collection is dirty."), size_t(0));
qof_collection_mark_clean(col);
+ qof_collection_destroy(col);
}
commit 0e3853bfb73cf0edf276ce98ac74523c230b2a38
Author: Stefan Koch <stefan.koch at micromeritics.com>
Date: Thu Jan 15 07:10:20 2026 -0500
Fix undefined behaviour in collection_compare_cb function.
The collection_compare_cb function set the user_data pointer to point to a local variable
of the function. That pointer is then used in the (only) calling function
qof_collection_compare to make decisions. This is undefined. It is likely not an actual
problem because the stack depth of the qof_collection_foreach followed by
collection_compare_cb is deeper than the qof_collection_get_data call (and others
in between if any) that the stack data user data stays uncorrupted.
But, it is undefined behavior, and could cause really subtle bugs if these there are code
changes that have deeper stack between the setting and using.
Also, using this local variable is not necessary, the qof_collection_compare function
already sets up a variable local to its scope for this that the collection_compare_cb can
use directly.
This commit removes the local (to collection_compare_cb) variable and uses the one setup
in qof_collection_compare.
The full coverage test for qofid.cpp passed before and after this change.
diff --git a/libgnucash/engine/qofid.cpp b/libgnucash/engine/qofid.cpp
index b1bf7e8e50..3e47ff9158 100644
--- a/libgnucash/engine/qofid.cpp
+++ b/libgnucash/engine/qofid.cpp
@@ -139,7 +139,7 @@ collection_compare_cb (QofInstance *ent, gpointer user_data)
QofCollection *target;
QofInstance *e;
const GncGUID *guid;
- gint value;
+ gint* value;
e = NULL;
target = (QofCollection*)user_data;
@@ -147,28 +147,25 @@ collection_compare_cb (QofInstance *ent, gpointer user_data)
{
return;
}
- value = *(gint*)qof_collection_get_data(target);
- if (value != 0)
+ value = (gint*)qof_collection_get_data(target);
+ if (*value != 0)
{
return;
}
guid = qof_instance_get_guid(ent);
if (guid_equal(guid, guid_null()))
{
- value = -1;
- qof_collection_set_data(target, &value);
+ *value = -1;
return;
}
g_return_if_fail (target->e_type == ent->e_type);
e = qof_collection_lookup_entity(target, guid);
if ( e == NULL )
{
- value = 1;
- qof_collection_set_data(target, &value);
+ *value = 1;
return;
}
- value = 0;
- qof_collection_set_data(target, &value);
+ *value = 0;
}
gint
commit 6e9a20dfebca6bbba8d9cfdeb6b4183a3d0fe364
Author: Stefan Koch <stefan.koch at micromeritics.com>
Date: Tue Jan 13 12:30:22 2026 -0500
Implement full test coverage of libgnucash/engine/qofid
NOTE: This does not have full coverage because the "if (!target || !ent)" body in the
collection_compare_cp function cannot be reached. There is too much safety in the rest of
the system to test this error.
NOTE: I made this a separate test from the test-engine so that I was able to check that it
by itself could test the full coverage of the qofid.cpp file. If it was part of the larger
test, I could have missed some parts that were covered incidentally elsewhere.
diff --git a/libgnucash/engine/test/CMakeLists.txt b/libgnucash/engine/test/CMakeLists.txt
index c920f0d38a..137ad90252 100644
--- a/libgnucash/engine/test/CMakeLists.txt
+++ b/libgnucash/engine/test/CMakeLists.txt
@@ -130,6 +130,9 @@ set(test_qofsession_SOURCES
gnc_add_test(test-qofsession "${test_qofsession_SOURCES}"
gtest_engine_INCLUDES gtest_old_engine_LIBS)
+gnc_add_test(test-qofid test-qofid.cpp
+ gtest_engine_INCLUDES gtest_old_engine_LIBS)
+
gnc_add_test(test-gnc-euro gtest-gnc-euro.cpp
gtest_engine_INCLUDES gtest_old_engine_LIBS)
diff --git a/libgnucash/engine/test/test-qofid.cpp b/libgnucash/engine/test/test-qofid.cpp
new file mode 100644
index 0000000000..9897a87eee
--- /dev/null
+++ b/libgnucash/engine/test/test-qofid.cpp
@@ -0,0 +1,205 @@
+/********************************************************************
+ * test-qofid.c: GLib g_test test suite for qofobject.c. *
+ * Copyright 2025 Stefan Koch <stefan.koch.micro at gmail.com> *
+ * *
+ * This program is free software; you can redistribute it and/or *
+ * modify it under the terms of the GNU General Public License as *
+ * published by the Free Software Foundation; either version 2 of *
+ * the License, or (at your option) any later version. *
+ * *
+ * This program is distributed in the hope that it will be useful, *
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of *
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the *
+ * GNU General Public License for more details. *
+ * *
+ * You should have received a copy of the GNU General Public License*
+ * along with this program; if not, contact: *
+ * *
+ * Free Software Foundation Voice: +1-617-542-5942 *
+ * 51 Franklin Street, Fifth Floor Fax: +1-617-542-2652 *
+ * Boston, MA 02110-1301, USA gnu at gnu.org *
+********************************************************************/
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wcpp"
+#include <gtest/gtest.h>
+#pragma GCC diagnostic pop
+#include <glib.h>
+#include <algorithm>
+#include <vector>
+
+#include "../qofid.h"
+#include "../qofid-p.h"
+#include "../qofbook.h"
+#include "../gncJob.h"
+#include "../qofinstance-p.h"
+
+// This is the error handler that does nothing but count how many times it is
+// called. The handler_count is incremented every time.
+static void
+test_count_handler (const char *log_domain, GLogLevelFlags log_level,
+ const gchar *msg, void* user_data )
+{
+ auto pint = (int*)user_data;
+ *pint += 1;
+}
+
+TEST(QofIDTest, collection_new)
+{
+ auto col = qof_collection_new (QOF_ID_BOOK);
+ EXPECT_STREQ(qof_collection_get_type(col), QOF_ID_BOOK);
+ EXPECT_EQ(qof_collection_count(col), 0u);
+
+ EXPECT_FALSE(qof_collection_is_dirty(col));
+ qof_collection_mark_dirty(col);
+ EXPECT_TRUE(qof_collection_is_dirty(col));
+ qof_collection_mark_clean(col);
+ EXPECT_FALSE(qof_collection_is_dirty(col));
+
+ EXPECT_EQ(qof_collection_get_data(col), nullptr);
+ int i = 0;
+ gpointer data = &i;
+ qof_collection_set_data(col, data);
+ EXPECT_EQ(qof_collection_get_data(col), data);
+ qof_collection_set_data(col, nullptr);
+
+ qof_collection_destroy(col);
+}
+
+
+TEST(QofIDTest, collection_add)
+{
+ auto col = qof_collection_new(QOF_ID_BOOK);
+ auto book = qof_book_new();
+ auto job = gncJobCreate(book);
+ gncJobBeginEdit(job);
+
+ EXPECT_FALSE(qof_collection_add_entity(nullptr, nullptr));
+ EXPECT_FALSE(qof_collection_add_entity(col, nullptr));
+ EXPECT_FALSE(qof_collection_add_entity(nullptr, QOF_INSTANCE(book)));
+
+ EXPECT_TRUE(qof_collection_add_entity(col, QOF_INSTANCE(book)));
+ EXPECT_FALSE(qof_collection_add_entity(col, QOF_INSTANCE(book)));
+
+ auto old_guid = qof_instance_get_guid(QOF_INSTANCE(book));
+ qof_instance_set_guid(QOF_INSTANCE(book), guid_null());
+ EXPECT_FALSE(qof_collection_add_entity(col, QOF_INSTANCE(book)));
+ qof_instance_set_guid(QOF_INSTANCE(book), old_guid);
+
+
+ auto assert_count = 0;
+ auto oldlogger = g_log_set_default_handler(test_count_handler, &assert_count);
+ EXPECT_FALSE(qof_collection_add_entity(col, QOF_INSTANCE(job)));
+ EXPECT_EQ(assert_count, 1);
+ g_log_set_default_handler(oldlogger, nullptr);
+
+ gncJobDestroy(job);
+ qof_collection_destroy(col);
+}
+
+TEST(QofIDTest, collection_compare)
+{
+ auto target = qof_collection_new(QOF_ID_BOOK);
+ auto merge = qof_collection_new(QOF_ID_BOOK);
+ auto incompat = qof_collection_new(GNC_ID_JOB);
+ auto book1 = qof_book_new();
+ auto book2 = qof_book_new();
+ auto book3 = qof_book_new();
+ auto book4 = qof_book_new();
+
+ // Special cases.
+ EXPECT_EQ(0, qof_collection_compare(nullptr, nullptr));
+ EXPECT_EQ(0, qof_collection_compare(target, target));
+ EXPECT_EQ(-1, qof_collection_compare(nullptr, merge));
+ EXPECT_EQ(1, qof_collection_compare(target, nullptr));
+ EXPECT_EQ(-1, qof_collection_compare(target, incompat));
+
+ EXPECT_EQ(0, qof_collection_compare(target, merge));
+ qof_collection_add_entity(target, QOF_INSTANCE(book1));
+ qof_collection_add_entity(merge, QOF_INSTANCE(book1));
+ EXPECT_EQ(0, qof_collection_compare(target, merge));
+
+ qof_collection_add_entity(target, QOF_INSTANCE(book2));
+ EXPECT_EQ(1, qof_collection_compare(target, merge));
+ qof_collection_add_entity(merge, QOF_INSTANCE(book2));
+ qof_collection_add_entity(merge, QOF_INSTANCE(book3));
+ EXPECT_EQ(1, qof_collection_compare(target, merge));
+
+ auto old_guid = qof_instance_get_guid(QOF_INSTANCE(book3));
+ qof_instance_set_guid(QOF_INSTANCE(book3), guid_null());
+ EXPECT_EQ(-1, qof_collection_compare(target, merge));
+ qof_instance_set_guid(QOF_INSTANCE(book3), old_guid);
+
+ qof_collection_add_entity(target, QOF_INSTANCE(book4));
+ old_guid = qof_instance_get_guid(QOF_INSTANCE(book4));
+ qof_instance_set_guid(QOF_INSTANCE(book4), guid_null());
+ EXPECT_EQ(-1, qof_collection_compare(target, merge));
+ qof_instance_set_guid(QOF_INSTANCE(book4), old_guid);
+}
+
+static void cb_visit(GncJob* job, std::vector<const char*>* results_ptr)
+{
+ results_ptr->push_back(gncJobGetID(job));
+}
+
+static gint cb_compare(GncJob* job1, GncJob* job2)
+{
+ return g_strcmp0(gncJobGetID(job1), gncJobGetID(job2));
+}
+
+static bool is_in(const char* str, const std::vector<const char*> str_vect)
+{
+ for(auto id : str_vect)
+ if(!g_strcmp0(id, str))
+ return true;
+ return false;
+}
+
+TEST(QofIDTest, collection_foreach)
+{
+ auto col = qof_collection_new(GNC_ID_JOB);
+ auto book = qof_book_new();
+ auto job_ids = std::vector<const char*>{"zzz", "ggg", "qqq", "aaa"};
+ std::vector<GncJob*> jobs;
+ for(auto id : job_ids)
+ {
+ auto job = gncJobCreate(book);
+ gncJobSetID(job, id);
+ jobs.push_back(job);
+ qof_collection_add_entity(col, QOF_INSTANCE(job));
+ }
+
+ std::vector<const char*> results;
+ qof_collection_foreach(col, (QofInstanceForeachCB)&cb_visit, &results);
+ EXPECT_EQ(job_ids.size(), results.size());
+ for( auto id : job_ids)
+ EXPECT_TRUE(is_in(id, results));
+
+ results.clear();
+ qof_collection_foreach_sorted(
+ col, (QofInstanceForeachCB)&cb_visit, &results, (GCompareFunc)&cb_compare);
+ EXPECT_EQ(job_ids.size(), results.size());
+ EXPECT_STREQ(results[0], "aaa");
+ EXPECT_STREQ(results[1], "ggg");
+ EXPECT_STREQ(results[2], "qqq");
+ EXPECT_STREQ(results[3], "zzz");
+}
+
+TEST(QofIDTest, collection_print_dirty)
+{
+ auto col = qof_collection_new(QOF_ID_BOOK);
+ auto book = qof_book_new();
+ qof_collection_add_entity(col, QOF_INSTANCE(book));
+
+ testing::internal::CaptureStdout();
+ qof_collection_print_dirty(col, nullptr);
+ std::string output = testing::internal::GetCapturedStdout();
+ EXPECT_STREQ("", output.c_str());
+
+
+ qof_collection_mark_dirty(col);
+ testing::internal::CaptureStdout();
+ qof_collection_print_dirty(col, nullptr);
+ output = testing::internal::GetCapturedStdout();
+ EXPECT_EQ(output.find("Book collection is dirty."), size_t(0));
+ qof_collection_mark_clean(col);
+}
commit a2cc9881dfc08ebde84ce0b9a421b9f2d7e5d708
Author: Stefan Koch <stefan.koch at micromeritics.com>
Date: Thu Jan 15 08:42:20 2026 -0500
Explicitly set the is_dirty attribute in qof_collection_new
It is likely that the memory was zeroed anyway, but the explicitness makes it easier to read.
diff --git a/libgnucash/engine/qofid.cpp b/libgnucash/engine/qofid.cpp
index 2d3bf402e6..b1bf7e8e50 100644
--- a/libgnucash/engine/qofid.cpp
+++ b/libgnucash/engine/qofid.cpp
@@ -50,6 +50,7 @@ qof_collection_new (QofIdType type)
QofCollection *col;
col = g_new0(QofCollection, 1);
col->e_type = static_cast<QofIdType>(CACHE_INSERT (type));
+ col->is_dirty = FALSE;
col->hash_of_entities = guid_hash_table_new();
col->data = NULL;
return col;
commit 9079d87307c3a1b7e7d33c64c8ed30ebe7c29d7c
Author: Stefan Koch <stefan.koch at micromeritics.com>
Date: Thu Jan 15 08:41:35 2026 -0500
Remove unused qof_collection_from_glist function.
diff --git a/libgnucash/engine/qofid.cpp b/libgnucash/engine/qofid.cpp
index 78fefff1df..2d3bf402e6 100644
--- a/libgnucash/engine/qofid.cpp
+++ b/libgnucash/engine/qofid.cpp
@@ -220,26 +220,6 @@ qof_collection_lookup_entity (const QofCollection *col, const GncGUID * guid)
return ent;
}
-QofCollection *
-qof_collection_from_glist (QofIdType type, const GList *glist)
-{
- QofCollection *coll;
- QofInstance *ent;
- const GList *list;
-
- coll = qof_collection_new(type);
- for (list = glist; list != NULL; list = list->next)
- {
- ent = QOF_INSTANCE(list->data);
- if (FALSE == qof_collection_add_entity(coll, ent))
- {
- qof_collection_destroy(coll);
- return NULL;
- }
- }
- return coll;
-}
-
guint
qof_collection_count (const QofCollection *col)
{
diff --git a/libgnucash/engine/qofid.h b/libgnucash/engine/qofid.h
index 8a6f6157fa..b3eabbdf45 100644
--- a/libgnucash/engine/qofid.h
+++ b/libgnucash/engine/qofid.h
@@ -203,19 +203,6 @@ not in the other.
gint
qof_collection_compare (QofCollection *target, QofCollection *merge);
-/** \brief Create a secondary collection from a GList
-
- at param type The QofIdType of the QofCollection \b and of
- \b all entities in the GList.
- at param glist GList of entities of the same QofIdType.
-
- at return NULL if any of the entities fail to match the
- QofCollection type, else a pointer to the collection
- on success.
-*/
-QofCollection*
-qof_collection_from_glist (QofIdType type, const GList *glist);
-
/** @} */
/** @} */
Summary of changes:
libgnucash/engine/qofid.cpp | 36 ++----
libgnucash/engine/qofid.h | 13 ---
libgnucash/engine/test/CMakeLists.txt | 4 +
libgnucash/engine/test/test-qofid.cpp | 210 ++++++++++++++++++++++++++++++++++
4 files changed, 221 insertions(+), 42 deletions(-)
create mode 100644 libgnucash/engine/test/test-qofid.cpp
More information about the gnucash-changes
mailing list