Fix memory leak in xaccTransGetReadOnly

In addition implement a cache for this value as suggested in the comments
as this function is called on every transaction commit.

gnucash/gnome/gnc-split-reg.c

@@ -903,7 +903,7 @@ gnc_split_reg_reverse_trans_cb (GtkWidget *w, gpointer data)
 
 
 static gboolean
-is_trans_readonly_and_warn (GtkWindow *parent, const Transaction *trans)
+is_trans_readonly_and_warn (GtkWindow *parent, Transaction *trans)
 {
     GtkWidget *dialog;
     const gchar *reason;

gnucash/register/ledger-core/split-register-model.c

@@ -2030,7 +2030,7 @@ gnc_split_register_get_security_io_flags (VirtualLocation virt_loc,
 }
 
 static gboolean
-xaccTransWarnReadOnly (GtkWidget *parent, const Transaction *trans)
+xaccTransWarnReadOnly (GtkWidget *parent, Transaction *trans)
 {
     GtkWidget *dialog;
     const gchar *reason;

libgnucash/engine/Transaction.c

@@ -281,6 +281,8 @@ gnc_transaction_init(Transaction* trans)
     trans->date_posted  = 0;
     trans->marker = 0;
     trans->orig = NULL;
+    trans->readonly_reason = NULL;
+    trans->reason_cache_valid = FALSE;
     LEAVE (" ");
 }
 
@@ -811,12 +813,16 @@ xaccFreeTransaction (Transaction *trans)
     /* free up transaction strings */
     CACHE_REMOVE(trans->num);
     CACHE_REMOVE(trans->description);
+    if (trans->readonly_reason)
+        g_free (trans->readonly_reason);
 
     /* Just in case someone looks up freed memory ... */
     trans->num         = (char *) 1;
     trans->description = NULL;
     trans->date_entered = 0;
     trans->date_posted = 0;
+    trans->readonly_reason = NULL;
+    trans->reason_cache_valid = FALSE;
     if (trans->orig)
     {
         xaccFreeTransaction (trans->orig);
@@ -2069,6 +2075,11 @@ void xaccTransClearReadOnly (Transaction *trans)
         qof_instance_set_kvp (QOF_INSTANCE (trans), NULL, 1, TRANS_READ_ONLY_REASON);
         qof_instance_set_dirty(QOF_INSTANCE(trans));
         xaccTransCommitEdit(trans);
+
+        if (trans->readonly_reason)
+            g_free (trans->readonly_reason);
+        trans->readonly_reason = NULL;
+        trans->reason_cache_valid = TRUE;
     }
 }
 
@@ -2084,6 +2095,11 @@ xaccTransSetReadOnly (Transaction *trans, const char *reason)
         qof_instance_set_kvp (QOF_INSTANCE (trans), &v, 1, TRANS_READ_ONLY_REASON);
         qof_instance_set_dirty(QOF_INSTANCE(trans));
         xaccTransCommitEdit(trans);
+
+        if (trans->readonly_reason)
+            g_free (trans->readonly_reason);
+        trans->readonly_reason = g_strdup (reason);
+        trans->reason_cache_valid = TRUE;
     }
 }
 
@@ -2440,21 +2456,28 @@ xaccTransGetTxnType (const Transaction *trans)
 }
 
 const char *
-xaccTransGetReadOnly (const Transaction *trans)
+xaccTransGetReadOnly (Transaction *trans)
 {
-    /* XXX This flag should be cached in the transaction structure
-     * for performance reasons, since its checked every trans commit.
-     */
-    GValue v = G_VALUE_INIT;
-    const char *s = NULL;
-    if (trans == NULL) return NULL;
-    qof_instance_get_kvp (QOF_INSTANCE(trans), &v, 1, TRANS_READ_ONLY_REASON);
-    if (G_VALUE_HOLDS_STRING (&v))
-         s = g_value_get_string (&v);
-    if (s && strlen (s))
-        return s;
+    if (!trans)
+        return NULL;
 
-    return NULL;
+    if (!trans->reason_cache_valid)
+    {
+        GValue v = G_VALUE_INIT;
+        qof_instance_get_kvp (QOF_INSTANCE(trans), &v, 1, TRANS_READ_ONLY_REASON);
+
+        /* Clear possible old cache value first */
+        if (trans->readonly_reason)
+            g_free (trans->readonly_reason);
+        trans->readonly_reason = NULL;
+
+        /* Then set the new one */
+        if (G_VALUE_HOLDS_STRING (&v))
+            trans->readonly_reason = g_value_dup_string (&v);
+        g_value_unset (&v);
+        trans->reason_cache_valid = TRUE;
+    }
+    return trans->readonly_reason;
 }
 
 static gboolean

libgnucash/engine/Transaction.h

@@ -423,7 +423,7 @@ void	      xaccTransClearReadOnly (Transaction *trans);
 
 /** Returns a non-NULL value if this Transaction was marked as read-only with
  * some specific "reason" text. */
-const char *  xaccTransGetReadOnly (const Transaction *trans);
+const char *  xaccTransGetReadOnly (Transaction *trans);
 
 /** Returns TRUE if this Transaction is read-only because its posted-date is
  * older than the "auto-readonly" threshold of this book. See

libgnucash/engine/TransactionP.h

@@ -110,6 +110,15 @@ struct transaction_s
      * any changes made if/when the edit is abandoned.
      */
     Transaction *orig;
+
+    /* The readonly_reason is a string that indicates why a transaction
+     * is marked as read-only. If NULL, the transaction is read-write.
+     * This value is stored in kvp, but we cache a copy here for
+     * performance reasons. reason_cache_valid indicates whether the
+     * cached value is valid.
+     */
+    char * readonly_reason;
+    gboolean reason_cache_valid;
 };
 
 struct _TransactionClass