gnucash stable: Multiple changes pushed

John Ralls jralls at code.gnucash.org
Sun Jun 2 17:40:56 EDT 2024


Updated	 via  https://github.com/Gnucash/gnucash/commit/1ad2e66a (commit)
	 via  https://github.com/Gnucash/gnucash/commit/c4d44ea0 (commit)
	from  https://github.com/Gnucash/gnucash/commit/8106ffb8 (commit)



commit 1ad2e66aac608c528a5a4dccf4d576924b6e00b0
Merge: 8106ffb84b c4d44ea024
Author: John Ralls <jralls at ceridwen.us>
Date:   Sun Jun 2 14:40:21 2024 -0700

    Merge Daniel Harding's Bug799324 into stable.


commit c4d44ea02411b44613d0069b6cbbd9948f48274b
Author: Daniel Harding <dharding at living180.net>
Date:   Sun Jun 2 14:54:42 2024 +0300

    Bug 799324 - Invalid free in gvalue_from_kvp_value()
    
    As of ddc3f28899c71579abd9b2bd822baa6b95fc41d6, gvalue_from_kvp_value()
    takes a GValue pointer from the caller, which in some cases points to
    memory on the stack.  If that is the case and the code also hits the
    default case in the switch statement, the unconditional g_slice_free()
    call will attempt to free stack memory, causing the program to abort.
    
    Fix by requiring the caller to always pass in a valid GValue pointer,
    making the caller responsible for freeing it if necessary.  This also
    means that it is no longer necessary for gvalue_from_kvp_value() to
    return a value, so make it a void function.

diff --git a/libgnucash/engine/kvp-frame.cpp b/libgnucash/engine/kvp-frame.cpp
index 438b360683..48f8037660 100644
--- a/libgnucash/engine/kvp-frame.cpp
+++ b/libgnucash/engine/kvp-frame.cpp
@@ -232,14 +232,11 @@ int compare(const KvpFrameImpl & one, const KvpFrameImpl & two) noexcept
     return 0;
 }
 
-GValue*
+void
 gvalue_from_kvp_value (const KvpValue *kval, GValue* val)
 {
-    if (kval == NULL) return NULL;
-    if (!val)
-        val = g_slice_new0 (GValue);
-    else
-        g_value_unset(val);
+    if (kval == NULL) return;
+    g_value_unset(val);
 
     switch (kval->get_type())
     {
@@ -274,11 +271,8 @@ gvalue_from_kvp_value (const KvpValue *kval, GValue* val)
         default:
 /* No transfer outside of QofInstance-derived classes! */
             PWARN ("Error! Invalid attempt to transfer Kvp type %d", kval->get_type());
-            g_slice_free (GValue, val);
-            val = NULL;
             break;
     }
-    return val;
 }
 
 KvpValue*
diff --git a/libgnucash/engine/kvp-value.hpp b/libgnucash/engine/kvp-value.hpp
index e3f4f9cd8b..6518dc4e80 100644
--- a/libgnucash/engine/kvp-value.hpp
+++ b/libgnucash/engine/kvp-value.hpp
@@ -175,9 +175,9 @@ KvpValueImpl::set(T val) noexcept
 /** @internal @{ */
 /** Convert a kvp_value into a GValue. Frames aren't converted.
  * @param kval: A KvpValue.
- * @return GValue*. Must be freed with g_free().
+ * @param val: The GValue in which to store the converted value.
  */
-GValue* gvalue_from_kvp_value (const KvpValue *kval, GValue* val = nullptr);
+void gvalue_from_kvp_value (const KvpValue *kval, GValue* val);
 
 /** Convert a gvalue into a kvpvalue.
  * @param gval: A GValue of a type KvpValue can digest.
diff --git a/libgnucash/engine/qofinstance.cpp b/libgnucash/engine/qofinstance.cpp
index 2ef3bf7882..11df55c1e8 100644
--- a/libgnucash/engine/qofinstance.cpp
+++ b/libgnucash/engine/qofinstance.cpp
@@ -1317,11 +1317,11 @@ static void
 wrap_gvalue_function (const char* key, KvpValue *val, wrap_param & param)
 {
     GValue *gv;
+    gv = g_slice_new0 (GValue);
     if (val->get_type() != KvpValue::Type::FRAME)
-        gv = gvalue_from_kvp_value(val);
+        gvalue_from_kvp_value(val, gv);
     else
     {
-        gv = g_slice_new0 (GValue);
         g_value_init (gv, G_TYPE_STRING);
         g_value_set_string (gv, nullptr);
     }



Summary of changes:
 libgnucash/engine/kvp-frame.cpp   | 12 +++---------
 libgnucash/engine/kvp-value.hpp   |  4 ++--
 libgnucash/engine/qofinstance.cpp |  4 ++--
 3 files changed, 7 insertions(+), 13 deletions(-)



More information about the gnucash-changes mailing list