gnucash maint: Multiple changes pushed
Geert Janssens
gjanssens at code.gnucash.org
Mon Sep 10 13:50:11 EDT 2018
Updated via https://github.com/Gnucash/gnucash/commit/48b29f5e (commit)
via https://github.com/Gnucash/gnucash/commit/b866d7d9 (commit)
from https://github.com/Gnucash/gnucash/commit/196decf6 (commit)
commit 48b29f5e91b6bce411a10505ad08b657967ec222
Author: Geert Janssens <geert at kobaltwit.be>
Date: Mon Sep 10 19:49:43 2018 +0200
Fix memory leak in char* type KvpValue and fix improper uses
The core issue was that the delete visitor was never called because its parameter
type (char *) didn't match the boost::variant type (const char *).
Fixing the visitor's parameter type also require a const_cast
back to char * because that's what g_free takes as argument.
The rest of this commit is merely fixing KvpValue instantiations that
tried to create a char* KvpValue from a stack based const string instead
of a heap allocated one. That would bomb out on calling the
delete visitor.
diff --git a/libgnucash/app-utils/test/test-option-util.cpp b/libgnucash/app-utils/test/test-option-util.cpp
index 4c7d358..d0fff4e 100644
--- a/libgnucash/app-utils/test/test-option-util.cpp
+++ b/libgnucash/app-utils/test/test-option-util.cpp
@@ -70,7 +70,7 @@ setup_kvp (Fixture *fixture, gconstpointer pData)
NULL);
slots->set_path({"options", "Business", "Company Name"},
- new KvpValue("Bogus Company"));
+ new KvpValue(g_strdup ("Bogus Company")));
qof_commit_edit (QOF_INSTANCE (book));
}
diff --git a/libgnucash/backend/dbi/test/test-backend-dbi-basic.cpp b/libgnucash/backend/dbi/test/test-backend-dbi-basic.cpp
index 534c185..c1f635a 100644
--- a/libgnucash/backend/dbi/test/test-backend-dbi-basic.cpp
+++ b/libgnucash/backend/dbi/test/test-backend-dbi-basic.cpp
@@ -139,7 +139,7 @@ setup_memory (Fixture* fixture, gconstpointer pData)
frame->set ({"double-val"}, new KvpValue (3.14159));
frame->set ({"numeric-val"}, new KvpValue (gnc_numeric_zero ()));
frame->set ({"time-val"}, new KvpValue (gnc_time(nullptr)));
- frame->set ({"string-val"}, new KvpValue ("abcdefghijklmnop"));
+ frame->set ({"string-val"}, new KvpValue (g_strdup ("abcdefghijklmnop")));
auto guid = qof_instance_get_guid (QOF_INSTANCE (acct1));
frame->set ({"guid-val"}, new KvpValue (const_cast<GncGUID*> (guid_copy (
guid))));
diff --git a/libgnucash/backend/xml/sixtp-dom-parsers.cpp b/libgnucash/backend/xml/sixtp-dom-parsers.cpp
index 0bde990..7762903 100644
--- a/libgnucash/backend/xml/sixtp-dom-parsers.cpp
+++ b/libgnucash/backend/xml/sixtp-dom-parsers.cpp
@@ -205,7 +205,7 @@ dom_tree_to_numeric_kvp_value (xmlNodePtr node)
static KvpValue*
dom_tree_to_string_kvp_value (xmlNodePtr node)
{
- gchar* datext;
+ const gchar* datext;
KvpValue* ret = NULL;
datext = dom_tree_to_text (node);
diff --git a/libgnucash/engine/gnc-aqbanking-templates.cpp b/libgnucash/engine/gnc-aqbanking-templates.cpp
index ad6b8fd..6749d0e 100644
--- a/libgnucash/engine/gnc-aqbanking-templates.cpp
+++ b/libgnucash/engine/gnc-aqbanking-templates.cpp
@@ -106,13 +106,13 @@ KvpFrame*
_GncABTransTempl::make_kvp_frame()
{
auto frame = new KvpFrame;
- frame->set({TT_NAME}, new KvpValue(m_name.c_str()));
- frame->set({TT_RNAME}, new KvpValue(m_recipient_name.c_str()));
- frame->set({TT_RACC}, new KvpValue(m_recipient_account.c_str()));
- frame->set({TT_RBCODE}, new KvpValue(m_recipient_bankcode.c_str()));
+ frame->set({TT_NAME}, new KvpValue(g_strdup (m_name.c_str())));
+ frame->set({TT_RNAME}, new KvpValue(g_strdup (m_recipient_name.c_str())));
+ frame->set({TT_RACC}, new KvpValue(g_strdup (m_recipient_account.c_str())));
+ frame->set({TT_RBCODE}, new KvpValue(g_strdup (m_recipient_bankcode.c_str())));
frame->set({TT_AMOUNT}, new KvpValue(m_amount));
- frame->set({TT_PURPOS}, new KvpValue(m_purpose.c_str()));
- frame->set({TT_PURPOSCT}, new KvpValue(m_purpose_continuation.c_str()));
+ frame->set({TT_PURPOS}, new KvpValue(g_strdup (m_purpose.c_str())));
+ frame->set({TT_PURPOSCT}, new KvpValue(g_strdup (m_purpose_continuation.c_str())));
return frame;
}
diff --git a/libgnucash/engine/kvp-value.cpp b/libgnucash/engine/kvp-value.cpp
index ab2f3be..5c4addd 100644
--- a/libgnucash/engine/kvp-value.cpp
+++ b/libgnucash/engine/kvp-value.cpp
@@ -365,9 +365,9 @@ delete_visitor::operator()(GList * & value)
g_list_free_full(value, destroy_value);
}
template <> void
-delete_visitor::operator()(gchar * & value)
+delete_visitor::operator()(const gchar * & value)
{
- g_free(value);
+ g_free(const_cast<gchar *> (value));
}
template <> void
delete_visitor::operator()(GncGUID * & value)
@@ -390,7 +390,7 @@ void
KvpValueImpl::duplicate(const KvpValueImpl& other) noexcept
{
if (other.datastore.type() == typeid(const gchar *))
- this->datastore = g_strdup(other.get<const gchar *>());
+ this->datastore = const_cast<const gchar *>(g_strdup(other.get<const gchar *>()));
else if (other.datastore.type() == typeid(GncGUID*))
this->datastore = guid_copy(other.get<GncGUID *>());
else if (other.datastore.type() == typeid(GList*))
diff --git a/libgnucash/engine/kvp-value.hpp b/libgnucash/engine/kvp-value.hpp
index acc072b..61b7dfd 100644
--- a/libgnucash/engine/kvp-value.hpp
+++ b/libgnucash/engine/kvp-value.hpp
@@ -86,7 +86,7 @@ struct KvpValueImpl
/** Create a KvpValue containing the passed in item. Note that for pointer
* types const char*, KvpFrame*, GncGUID*, and GList* the KvpValue takes
- * ownership of the objcet and will delete/free it when the KvpValue is
+ * ownership of the object and will delete/free it when the KvpValue is
* destroyed. That means these objects must be allocated in the free store
* or heap as follows:
* * const char*: GLib string allocation, e.g. g_strdup()/
diff --git a/libgnucash/engine/qofbook.cpp b/libgnucash/engine/qofbook.cpp
index c1fb7b0..db7bfd2 100644
--- a/libgnucash/engine/qofbook.cpp
+++ b/libgnucash/engine/qofbook.cpp
@@ -1211,7 +1211,7 @@ qof_book_set_feature (QofBook *book, const gchar *key, const gchar *descr)
if (feature == nullptr || g_strcmp0 (feature->get<const char*>(), descr))
{
qof_book_begin_edit (book);
- delete frame->set_path({GNC_FEATURES, key}, new KvpValue(descr));
+ delete frame->set_path({GNC_FEATURES, key}, new KvpValue(g_strdup (descr)));
qof_instance_set_dirty (QOF_INSTANCE (book));
qof_book_commit_edit (book);
}
diff --git a/libgnucash/engine/test/test-kvp-frame.cpp b/libgnucash/engine/test/test-kvp-frame.cpp
index 29a162c..7ad855d 100644
--- a/libgnucash/engine/test/test-kvp-frame.cpp
+++ b/libgnucash/engine/test/test-kvp-frame.cpp
@@ -33,7 +33,7 @@ class KvpFrameTest : public ::testing::Test
public:
KvpFrameTest() :
t_int_val{new KvpValue {INT64_C(15)}},
- t_str_val{new KvpValue{"a value"}} {
+ t_str_val{new KvpValue{g_strdup ("a value")}} {
auto f1 = new KvpFrame;
t_root.set({"top"}, new KvpValue{f1});
f1->set({"first"}, t_int_val);
diff --git a/libgnucash/engine/test/utest-Transaction.cpp b/libgnucash/engine/test/utest-Transaction.cpp
index 4a3e93c..d8cc90b 100644
--- a/libgnucash/engine/test/utest-Transaction.cpp
+++ b/libgnucash/engine/test/utest-Transaction.cpp
@@ -151,7 +151,7 @@ setup (Fixture *fixture, gconstpointer pData)
xaccTransSetCurrency (txn, fixture->curr);
xaccSplitSetParent (split1, txn);
xaccSplitSetParent (split2, txn);
- frame->set({trans_notes_str}, new KvpValue("Salt pork sausage"));
+ frame->set({trans_notes_str}, new KvpValue(g_strdup ("Salt pork sausage")));
frame->set_path({"qux", "quux", "corge"}, new KvpValue(123.456));
qof_instance_set_slots (QOF_INSTANCE (txn), frame);
}
@@ -557,7 +557,7 @@ test_dupe_trans (Fixture *fixture, gconstpointer pData)
oldtxn->date_posted = posted;
oldtxn->date_entered = entered;
oldtxn->inst.kvp_data->set({"foo", "bar", "baz"},
- new KvpValue("The Great Waldo Pepper"));
+ new KvpValue(g_strdup ("The Great Waldo Pepper")));
newtxn = fixture->func->dupe_trans (oldtxn);
commit b866d7d955f89d74ac31ea49167c95a9cdbc8899
Author: Geert Janssens <geert at kobaltwit.be>
Date: Mon Sep 10 18:57:39 2018 +0200
Plug memory leak in xaccSplitDestroy
Splits were not marked for deletion if the transaction is read-only
and the account is not marked for deletion yet. The net result is
that split will not be freed later on.
However xaccSplitDestroy is also called from a Transaction's do_destroy.
At that point accounts are not necessarily marked for deletion yet (like
is the case when a datafile is closed). This turned out to be a problem
for invoice post transactions (which are also read only) and hence
would cause memory to leak.
diff --git a/libgnucash/engine/Split.c b/libgnucash/engine/Split.c
index 65c3e2e..288c424 100644
--- a/libgnucash/engine/Split.c
+++ b/libgnucash/engine/Split.c
@@ -1447,6 +1447,7 @@ xaccSplitDestroy (Split *split)
acc = split->acc;
trans = split->parent;
if (acc && !qof_instance_get_destroying(acc)
+ && !qof_instance_get_destroying(trans)
&& xaccTransGetReadOnly(trans))
return FALSE;
Summary of changes:
libgnucash/app-utils/test/test-option-util.cpp | 2 +-
libgnucash/backend/dbi/test/test-backend-dbi-basic.cpp | 2 +-
libgnucash/backend/xml/sixtp-dom-parsers.cpp | 2 +-
libgnucash/engine/Split.c | 1 +
libgnucash/engine/gnc-aqbanking-templates.cpp | 12 ++++++------
libgnucash/engine/kvp-value.cpp | 6 +++---
libgnucash/engine/kvp-value.hpp | 2 +-
libgnucash/engine/qofbook.cpp | 2 +-
libgnucash/engine/test/test-kvp-frame.cpp | 2 +-
libgnucash/engine/test/utest-Transaction.cpp | 4 ++--
10 files changed, 18 insertions(+), 17 deletions(-)
More information about the gnucash-changes
mailing list