From ddc3f28899c71579abd9b2bd822baa6b95fc41d6 Mon Sep 17 00:00:00 2001
From: John Ralls <jralls@ceridwen.us>
Date: Mon, 27 Feb 2023 13:02:27 -0800
Subject: [PATCH] [kvp] Use static strings and boxed in gvalue_from_kvp_value.

Saves allocating and copying complex values, avoiding potential
memory leaks.
---
 libgnucash/engine/kvp-frame.cpp   | 28 +++++++++++-----------------
 libgnucash/engine/kvp-value.hpp   | 11 ++++++++++-
 libgnucash/engine/qofinstance.cpp | 10 +---------
 3 files changed, 22 insertions(+), 27 deletions(-)

diff --git a/libgnucash/engine/kvp-frame.cpp b/libgnucash/engine/kvp-frame.cpp
index 56be042d4c..c21370d054 100644
--- a/libgnucash/engine/kvp-frame.cpp
+++ b/libgnucash/engine/kvp-frame.cpp
@@ -261,15 +261,17 @@ kvp_value_list_from_gvalue (GValue *gval, gpointer pList)
 }
 
 GValue*
-gvalue_from_kvp_value (const KvpValue *kval)
+gvalue_from_kvp_value (const KvpValue *kval, GValue* val)
 {
-    GValue *val;
     gnc_numeric num;
     Time64 tm;
     GDate gdate;
 
     if (kval == NULL) return NULL;
-    val = g_slice_new0 (GValue);
+    if (!val)
+        val = g_slice_new0 (GValue);
+    else
+        g_value_unset(val);
 
     switch (kval->get_type())
     {
@@ -283,36 +285,28 @@ gvalue_from_kvp_value (const KvpValue *kval)
             break;
         case KvpValue::Type::NUMERIC:
             g_value_init (val, GNC_TYPE_NUMERIC);
-            num = kval->get<gnc_numeric>();
-            g_value_set_boxed (val, &num);
+            g_value_set_static_boxed (val, kval->get_ptr<gnc_numeric>());
             break;
         case KvpValue::Type::STRING:
             g_value_init (val, G_TYPE_STRING);
-            g_value_set_string (val, kval->get<const char*>());
+            g_value_set_static_string (val, kval->get<const char*>());
             break;
         case KvpValue::Type::GUID:
             g_value_init (val, GNC_TYPE_GUID);
-            g_value_set_boxed (val, kval->get<GncGUID*>());
+            g_value_set_static_boxed (val, kval->get<GncGUID*>());
             break;
         case KvpValue::Type::TIME64:
             g_value_init (val, GNC_TYPE_TIME64);
-            tm = kval->get<Time64>();
-            g_value_set_boxed (val, &tm);
+            g_value_set_boxed (val, kval->get_ptr<Time64>());
             break;
         case KvpValue::Type::GDATE:
             g_value_init (val, G_TYPE_DATE);
-            gdate = kval->get<GDate>();
-            g_value_set_boxed (val, &gdate);
+            g_value_set_static_boxed (val, kval->get_ptr<GDate>());
             break;
         case KvpValue::Type::GLIST:
         {
-            GList *gvalue_list = NULL;
-            GList *kvp_list = kval->get<GList*>();
-            g_list_foreach (kvp_list, (GFunc)gvalue_list_from_kvp_value,
-                            &gvalue_list);
             g_value_init (val, GNC_TYPE_VALUE_LIST);
-            gvalue_list = g_list_reverse (gvalue_list);
-            g_value_set_boxed (val, gvalue_list);
+            g_value_set_static_boxed (val, kval->get<GList*>());
             break;
         }
 /* No transfer of KVP frames outside of QofInstance-derived classes! */
diff --git a/libgnucash/engine/kvp-value.hpp b/libgnucash/engine/kvp-value.hpp
index 61b7dfd99f..e7083b2f24 100644
--- a/libgnucash/engine/kvp-value.hpp
+++ b/libgnucash/engine/kvp-value.hpp
@@ -142,6 +142,8 @@ struct KvpValueImpl
 
     template <typename T>
     T get() const noexcept;
+    template <typename T>
+    const T* get_ptr() const noexcept;
 
     template <typename T>
     void set(T) noexcept;
@@ -178,6 +180,13 @@ KvpValueImpl::get() const noexcept
     return boost::get<T>(datastore);
 }
 
+template <typename T> const T*
+KvpValueImpl::get_ptr() const noexcept
+{
+    if (this->datastore.type() != typeid(T)) return nullptr;
+    return boost::get<T>(&datastore);
+}
+
 template <typename T> void
 KvpValueImpl::set(T val) noexcept
 {
@@ -190,7 +199,7 @@ KvpValueImpl::set(T val) noexcept
  * @param kval: A KvpValue.
  * @return GValue*. Must be freed with g_free().
  */
-GValue* gvalue_from_kvp_value (const KvpValue *kval);
+GValue* gvalue_from_kvp_value (const KvpValue *kval, GValue* val = nullptr);
 
 /** 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 16b964b7ed..97c3063e4e 100644
--- a/libgnucash/engine/qofinstance.cpp
+++ b/libgnucash/engine/qofinstance.cpp
@@ -1090,15 +1090,7 @@ qof_instance_get_kvp (QofInstance * inst, GValue * value, unsigned count, ...)
     for (unsigned i{0}; i < count; ++i)
         path.push_back (va_arg (args, char const *));
     va_end (args);
-    auto temp = gvalue_from_kvp_value (inst->kvp_data->get_slot (path));
-    if (G_IS_VALUE (temp))
-    {
-        if (G_IS_VALUE (value))
-            g_value_unset (value);
-        g_value_init (value, G_VALUE_TYPE (temp));
-        g_value_copy (temp, value);
-        gnc_gvalue_free (temp);
-    }
+    gvalue_from_kvp_value (inst->kvp_data->get_slot (path), value);
 }
 
 void