From 3634e8f59d576ce2c3b97fc6a817ada506d47989 Mon Sep 17 00:00:00 2001 From: Geert Janssens Date: Sun, 9 Sep 2018 17:43:05 +0200 Subject: [PATCH] Fix memory leak using qof_instance_get on a GncGUID The underlying boxed type will return a copy so we should free this when no longer needed. --- gnucash/gnome-utils/gnc-tree-util-split-reg.c | 1 + gnucash/gnome/dialog-payment.c | 1 + gnucash/gnome/dialog-sx-editor.c | 1 + gnucash/gnome/dialog-sx-editor2.c | 1 + gnucash/gnome/gnc-plugin-page-register2.c | 1 + gnucash/gnome/gnc-split-reg.c | 1 + gnucash/import-export/ofx/gnc-ofx-import.c | 13 ++++++++----- libgnucash/app-utils/gnc-sx-instance-model.c | 7 +++++-- libgnucash/app-utils/gnc-ui-util.c | 7 +++++-- libgnucash/engine/SX-book.c | 4 ++-- libgnucash/engine/cap-gains.c | 2 ++ libgnucash/engine/gnc-budget.c | 5 +++-- libgnucash/engine/gncInvoice.c | 10 ++++++++-- libgnucash/engine/gncOwner.c | 2 ++ libgnucash/engine/qofbook.cpp | 2 +- libgnucash/engine/qofbook.h | 2 +- libgnucash/engine/test/test-engine-kvp-properties.c | 3 +++ 17 files changed, 46 insertions(+), 17 deletions(-) diff --git a/gnucash/gnome-utils/gnc-tree-util-split-reg.c b/gnucash/gnome-utils/gnc-tree-util-split-reg.c index d7d383623a..7f01c28c77 100644 --- a/gnucash/gnome-utils/gnc-tree-util-split-reg.c +++ b/gnucash/gnome-utils/gnc-tree-util-split-reg.c @@ -389,6 +389,7 @@ gnc_tree_util_split_reg_template_get_transfer_entry (Split *split) "sx-account", &guid, NULL); account = xaccAccountLookup (guid, gnc_get_current_book ()); + guid_free (guid); name = account ? gnc_get_account_name_for_register (account) : NULL; return name; diff --git a/gnucash/gnome/dialog-payment.c b/gnucash/gnome/dialog-payment.c index ef3a9ea401..47f4da3278 100644 --- a/gnucash/gnome/dialog-payment.c +++ b/gnucash/gnome/dialog-payment.c @@ -626,6 +626,7 @@ gnc_payment_dialog_owner_changed (PaymentWindow *pw) "payment-last-account", &guid, NULL); last_acct = xaccAccountLookup(guid, pw->book); + guid_free (guid); if (last_acct) gnc_tree_view_account_set_selected_account(GNC_TREE_VIEW_ACCOUNT(pw->acct_tree), last_acct); diff --git a/gnucash/gnome/dialog-sx-editor.c b/gnucash/gnome/dialog-sx-editor.c index 4bb2790e79..c4c667e026 100644 --- a/gnucash/gnome/dialog-sx-editor.c +++ b/gnucash/gnome/dialog-sx-editor.c @@ -643,6 +643,7 @@ gnc_sxed_split_check_account (GncSxEditorDialog *sxed, Split *s, "sx-account", &acct_guid, NULL); acct = xaccAccountLookup (acct_guid, gnc_get_current_book ()); + guid_free (acct_guid); if (acct == NULL) return FALSE; split_cmdty = xaccAccountGetCommodity(acct); diff --git a/gnucash/gnome/dialog-sx-editor2.c b/gnucash/gnome/dialog-sx-editor2.c index 7ee3b2c445..208773d81d 100644 --- a/gnucash/gnome/dialog-sx-editor2.c +++ b/gnucash/gnome/dialog-sx-editor2.c @@ -617,6 +617,7 @@ gnc_sxed_check_consistent (GncSxEditorDialog2 *sxed) "sx-debit-formula", &debit_formula, NULL); acct = xaccAccountLookup( acct_guid, gnc_get_current_book ()); + guid_free (acct_guid); split_cmdty = xaccAccountGetCommodity(acct); if (base_cmdty == NULL) { diff --git a/gnucash/gnome/gnc-plugin-page-register2.c b/gnucash/gnome/gnc-plugin-page-register2.c index 464d856197..63cce2de67 100644 --- a/gnucash/gnome/gnc-plugin-page-register2.c +++ b/gnucash/gnome/gnc-plugin-page-register2.c @@ -3683,6 +3683,7 @@ gnc_plugin_page_register2_cmd_schedule (GtkAction *action, ((guid_equal (xaccSchedXactionGetGUID (sx), fromSXId)) ? sx : NULL); } + guid_free (fromSXId); if (theSX) { diff --git a/gnucash/gnome/gnc-split-reg.c b/gnucash/gnome/gnc-split-reg.c index a867fb2e86..8887191d50 100644 --- a/gnucash/gnome/gnc-split-reg.c +++ b/gnucash/gnome/gnc-split-reg.c @@ -1477,6 +1477,7 @@ gsr_default_schedule_handler( GNCSplitReg *gsr, gpointer data ) ((guid_equal (xaccSchedXactionGetGUID (sx), fromSXId)) ? sx : NULL); } + guid_free (fromSXId); if ( theSX ) { diff --git a/gnucash/import-export/ofx/gnc-ofx-import.c b/gnucash/import-export/ofx/gnc-ofx-import.c index aea382f6e6..101c03010a 100644 --- a/gnucash/import-export/ofx/gnc-ofx-import.c +++ b/gnucash/import-export/ofx/gnc-ofx-import.c @@ -81,13 +81,16 @@ static const char *PROP_OFX_INCOME_ACCOUNT = "ofx-income-account"; static Account* get_associated_income_account(const Account* investment_account) { - GncGUID *income_guid= NULL; + GncGUID *income_guid = NULL; + Account *acct = NULL; g_assert(investment_account); qof_instance_get (QOF_INSTANCE (investment_account), - PROP_OFX_INCOME_ACCOUNT, &income_guid, - NULL); - return xaccAccountLookup(income_guid, - gnc_account_get_book(investment_account)); + PROP_OFX_INCOME_ACCOUNT, &income_guid, + NULL); + acct = xaccAccountLookup (income_guid, + gnc_account_get_book(investment_account)); + guid_free (income_guid); + return acct; } static void diff --git a/libgnucash/app-utils/gnc-sx-instance-model.c b/libgnucash/app-utils/gnc-sx-instance-model.c index cb8160c24f..79d76789d6 100644 --- a/libgnucash/app-utils/gnc-sx-instance-model.c +++ b/libgnucash/app-utils/gnc-sx-instance-model.c @@ -266,6 +266,7 @@ _get_vars_helper(Transaction *txn, void *var_hash_data) "sx-debit-formula", &debit_formula, NULL); acct = xaccAccountLookup(acct_guid, gnc_get_current_book()); + guid_free (acct_guid); split_cmdty = xaccAccountGetCommodity(acct); // existing... ------------------------------------------ if (credit_formula && strlen(credit_formula) != 0) @@ -974,6 +975,7 @@ _get_template_split_account(const SchedXaction* sx, Account **split_acct, GList **creation_errors) { + gboolean success = TRUE; GncGUID *acct_guid = NULL; qof_instance_get (QOF_INSTANCE (template_split), "sx-account", &acct_guid, @@ -987,10 +989,11 @@ _get_template_split_account(const SchedXaction* sx, gchar* err = N_("Unknown account for guid [%s], cancelling SX [%s] creation."); guid_to_string_buff((const GncGUID*)acct_guid, guid_str); REPORT_ERROR(creation_errors, err, guid_str, xaccSchedXactionGetName(sx)); - return FALSE; + success = FALSE; } - return TRUE; + guid_free (acct_guid); + return success; } static void diff --git a/libgnucash/app-utils/gnc-ui-util.c b/libgnucash/app-utils/gnc-ui-util.c index 7b9b8cb5fc..8936fa460a 100644 --- a/libgnucash/app-utils/gnc-ui-util.c +++ b/libgnucash/app-utils/gnc-ui-util.c @@ -386,8 +386,11 @@ gnc_book_get_default_gain_loss_acct (QofBook *book) if (!book) return NULL; if (gnc_book_use_book_currency (book)) - gains_account = xaccAccountLookup - (qof_book_get_default_gain_loss_acct_guid (book), book); + { + GncGUID *guid = qof_book_get_default_gain_loss_acct_guid (book); + gains_account = xaccAccountLookup (guid, book); + guid_free (guid); + } if (gains_account && !xaccAccountGetPlaceholder(gains_account) && diff --git a/libgnucash/engine/SX-book.c b/libgnucash/engine/SX-book.c index 612e97b44b..b5465992d8 100644 --- a/libgnucash/engine/SX-book.c +++ b/libgnucash/engine/SX-book.c @@ -380,9 +380,9 @@ gnc_sx_get_sxes_referencing_account(QofBook *book, Account *acct) GncGUID *guid = NULL; qof_instance_get (QOF_INSTANCE (s), "sx-account", &guid, NULL); if (guid_equal(acct_guid, guid)) - { rtn = g_list_append(rtn, sx); - } + + guid_free (guid); } } return rtn; diff --git a/libgnucash/engine/cap-gains.c b/libgnucash/engine/cap-gains.c index f32f154093..32122c0450 100644 --- a/libgnucash/engine/cap-gains.c +++ b/libgnucash/engine/cap-gains.c @@ -495,6 +495,7 @@ xaccSplitGetCapGainsSplit (const Split *split) gains_split = (Split*) qof_collection_lookup_entity ( qof_instance_get_collection(split), gains_guid); PINFO ("split=%p has gains-split=%p", split, gains_split); + guid_free (gains_guid); return gains_split; } @@ -517,6 +518,7 @@ xaccSplitGetGainsSourceSplit (const Split *split) source_split = (Split*) qof_collection_lookup_entity( qof_instance_get_collection(split), source_guid); PINFO ("split=%p has source-split=%p", split, source_split); + guid_free (source_guid); return source_split; } diff --git a/libgnucash/engine/gnc-budget.c b/libgnucash/engine/gnc-budget.c index a82f299b1d..9619dbcd29 100644 --- a/libgnucash/engine/gnc-budget.c +++ b/libgnucash/engine/gnc-budget.c @@ -637,14 +637,14 @@ gnc_budget_get_default (QofBook *book) { QofCollection *col; GncBudget *bgt = NULL; - const GncGUID *default_budget_guid = NULL; + GncGUID *default_budget_guid = NULL; g_return_val_if_fail(book, NULL); qof_instance_get (QOF_INSTANCE (book), "default-budget", &default_budget_guid, NULL); - if (default_budget_guid != NULL) + if (default_budget_guid) { col = qof_book_get_collection(book, GNC_ID_BUDGET); bgt = (GncBudget *) qof_collection_lookup_entity(col, @@ -662,6 +662,7 @@ gnc_budget_get_default (QofBook *book) } } + guid_free (default_budget_guid); return bgt; } diff --git a/libgnucash/engine/gncInvoice.c b/libgnucash/engine/gncInvoice.c index 670242c905..3d5aae14af 100644 --- a/libgnucash/engine/gncInvoice.c +++ b/libgnucash/engine/gncInvoice.c @@ -1250,12 +1250,15 @@ GncInvoice * gncInvoiceGetInvoiceFromLot (GNCLot *lot) { GncGUID *guid = NULL; QofBook *book; + GncInvoice *invoice = NULL; if (!lot) return NULL; book = gnc_lot_get_book (lot); qof_instance_get (QOF_INSTANCE (lot), "invoice", &guid, NULL); - return gncInvoiceLookup(book, guid); + invoice = gncInvoiceLookup(book, guid); + guid_free (guid); + return invoice; } void @@ -1279,12 +1282,15 @@ gncInvoiceGetInvoiceFromTxn (const Transaction *txn) { GncGUID *guid = NULL; QofBook *book; + GncInvoice *invoice = NULL; if (!txn) return NULL; book = xaccTransGetBook (txn); qof_instance_get (QOF_INSTANCE (txn), "invoice", &guid, NULL); - return gncInvoiceLookup(book, guid); + invoice = gncInvoiceLookup(book, guid); + guid_free (guid); + return invoice; } gboolean gncInvoiceAmountPositive (const GncInvoice *invoice) diff --git a/libgnucash/engine/gncOwner.c b/libgnucash/engine/gncOwner.c index a409b79340..23578bff45 100644 --- a/libgnucash/engine/gncOwner.c +++ b/libgnucash/engine/gncOwner.c @@ -641,9 +641,11 @@ gboolean gncOwnerGetOwnerFromLot (GNCLot *lot, GncOwner *owner) gncOwnerInitJob (owner, gncJobLookup (book, guid)); break; default: + guid_free (guid); return FALSE; } + guid_free (guid); return (owner->owner.undefined != NULL); } diff --git a/libgnucash/engine/qofbook.cpp b/libgnucash/engine/qofbook.cpp index 15f3a7f90c..c1fb7b0942 100644 --- a/libgnucash/engine/qofbook.cpp +++ b/libgnucash/engine/qofbook.cpp @@ -1004,7 +1004,7 @@ qof_book_get_default_gains_policy (QofBook *book) * valid book-currency, both of which are required, for the 'book-currency' * currency accounting method to apply. Use instead * 'gnc_book_get_default_gain_loss_acct' which does these validations. */ -const GncGUID * +GncGUID * qof_book_get_default_gain_loss_acct_guid (QofBook *book) { GncGUID *guid = NULL; diff --git a/libgnucash/engine/qofbook.h b/libgnucash/engine/qofbook.h index f2b4ae41c9..272ec9345c 100644 --- a/libgnucash/engine/qofbook.h +++ b/libgnucash/engine/qofbook.h @@ -282,7 +282,7 @@ const gchar * qof_book_get_default_gains_policy (QofBook *book); * valid book-currency, both of which are required, for the 'book-currency' * currency accounting method to apply. Use instead * 'gnc_book_get_default_gain_loss_acct' which does these validations. */ -const GncGUID * qof_book_get_default_gain_loss_acct_guid (QofBook *book); +GncGUID * qof_book_get_default_gain_loss_acct_guid (QofBook *book); /** Returns TRUE if the auto-read-only feature should be used, otherwise * FALSE. This is just a wrapper on qof_book_get_num_days_autoreadonly() == 0. */ diff --git a/libgnucash/engine/test/test-engine-kvp-properties.c b/libgnucash/engine/test/test-engine-kvp-properties.c index baddd3598a..7a11dfd050 100644 --- a/libgnucash/engine/test/test-engine-kvp-properties.c +++ b/libgnucash/engine/test/test-engine-kvp-properties.c @@ -178,6 +178,9 @@ test_account_kvp_properties (Fixture *fixture, gconstpointer pData) g_assert_cmpint (ab_acct_uid, ==, ab_acct_uid_r); g_assert_cmpint (trans_retr.t, ==, trans_retr_r->t); g_assert (!qof_instance_is_dirty (QOF_INSTANCE (fixture->acct))); + + guid_free (ofx_income_acct); + guid_free (ofx_income_acct_r); } static void