From 7c2a24951106c19f562efc745ed5c8d605cd5d53 Mon Sep 17 00:00:00 2001
From: Christopher Lam <christopher.lck@gmail.com>
Date: Wed, 27 Jul 2022 21:38:31 +0800
Subject: [PATCH] [gnc-sx-instance-model.c] refactor Scrub function

This scrubbing function calls xaccTransRollbackEdit which is
leaky. Instead of trying to fix xaccTransRollbackEdit which requires
superhuman skills, it's easiest to track the changes separately in a
GList, and use xaccTransBeginEdit/xaccTransCommitEdit only if
necessary.
---
 libgnucash/app-utils/gnc-sx-instance-model.c | 64 +++++++++++---------
 1 file changed, 37 insertions(+), 27 deletions(-)

diff --git a/libgnucash/app-utils/gnc-sx-instance-model.c b/libgnucash/app-utils/gnc-sx-instance-model.c
index f54b78e52a..f793966449 100644
--- a/libgnucash/app-utils/gnc-sx-instance-model.c
+++ b/libgnucash/app-utils/gnc-sx-instance-model.c
@@ -88,41 +88,45 @@ static void _gnc_sx_instance_event_handler(QofInstance *ent, QofEventId event_ty
 static gnc_commodity* get_transaction_currency(SxTxnCreationData *creation_data, SchedXaction *sx, Transaction *template_txn);
 /* ------------------------------------------------------------ */
 
-static gboolean
-scrub_sx_split_numeric (Split* split, const char *debcred)
+typedef struct
+{
+    const char *name;
+    gnc_numeric amount;
+} ScrubItem;
+
+static void
+scrub_sx_split_numeric (Split* split, gboolean is_credit, GList **changes)
 {
-    const gboolean is_credit = g_strcmp0 (debcred, "credit") == 0;
-    const char *formula = is_credit ?
-        "sx-credit-formula" : "sx-debit-formula";
-    const char *numeric = is_credit ?
-        "sx-credit-numeric" : "sx-debit-numeric";
+    const char *formula = is_credit ? "sx-credit-formula" : "sx-debit-formula";
+    const char *numeric = is_credit ? "sx-credit-numeric" : "sx-debit-numeric";
     char *formval;
     gnc_numeric *numval = NULL;
     GHashTable *parser_vars = g_hash_table_new (g_str_hash, g_str_equal);
     char *error_loc;
     gnc_numeric amount = gnc_numeric_zero ();
     gboolean parse_result = FALSE;
-    gboolean num_val_changed = FALSE;
-    qof_instance_get (QOF_INSTANCE (split),
-		  formula, &formval,
-		  numeric, &numval,
-		  NULL);
-    parse_result =
-        gnc_exp_parser_parse_separate_vars (formval, &amount,
-                                            &error_loc, parser_vars);
+
+    qof_instance_get (QOF_INSTANCE (split), formula, &formval,
+                      numeric, &numval, NULL);
+
+    parse_result = gnc_exp_parser_parse_separate_vars (formval, &amount,
+                                                       &error_loc, parser_vars);
+
     if (!parse_result || g_hash_table_size (parser_vars) != 0)
         amount = gnc_numeric_zero ();
+
     g_hash_table_unref (parser_vars);
+
     if (!numval || !gnc_numeric_eq (amount, *numval))
     {
-        qof_instance_set (QOF_INSTANCE (split),
-                          numeric, &amount,
-                          NULL);
-        num_val_changed = TRUE;
+        ScrubItem *change = g_new (ScrubItem, 1);
+        change->name = numeric;
+        change->amount = amount;
+        *changes = g_list_prepend (*changes, change);
     }
+
     g_free (formval);
     g_free (numval);
-    return num_val_changed;
 }
 
 /* Fixes error in pre-2.6.16 where the numeric slot wouldn't get changed if the
@@ -133,14 +137,20 @@ gnc_sx_scrub_split_numerics (gpointer psplit, gpointer puser)
 {
     Split *split = GNC_SPLIT (psplit);
     Transaction *trans = xaccSplitGetParent (split);
-    gboolean changed;
+    GList *changes = NULL;
+    scrub_sx_split_numeric (split, TRUE, &changes);
+    scrub_sx_split_numeric (split, FALSE, &changes);
+    if (!changes)
+        return;
+
     xaccTransBeginEdit (trans);
-    changed = scrub_sx_split_numeric (split, "credit") +
-        scrub_sx_split_numeric (split, "debit");
-    if (!changed)
-        xaccTransRollbackEdit (trans);
-    else
-        xaccTransCommitEdit (trans);
+    for (GList *n = changes; n; n = n->next)
+    {
+        ScrubItem *change = n->data;
+        qof_instance_set (QOF_INSTANCE (split), change->name, &change->amount, NULL);
+    }
+    xaccTransCommitEdit (trans);
+    g_list_free_full (changes, g_free);
 }
 
 static void