From c8a53c54f288eca5f125518128a00ffb5b950b87 Mon Sep 17 00:00:00 2001 From: Christopher Lam Date: Mon, 13 Sep 2021 23:26:58 +0800 Subject: [PATCH 1/3] [account.cpp] refactor gnc_account_get_map_entry Have comprehensive tests in a5d101d1b --- libgnucash/engine/Account.cpp | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/libgnucash/engine/Account.cpp b/libgnucash/engine/Account.cpp index f6ac68f381..9253139d52 100644 --- a/libgnucash/engine/Account.cpp +++ b/libgnucash/engine/Account.cpp @@ -6021,23 +6021,10 @@ gnc_account_imap_get_info (Account *acc, const char *category) gchar * gnc_account_get_map_entry (Account *acc, const char *head, const char *category) { - GValue v = G_VALUE_INIT; - gchar *text = NULL; - std::vector path {head}; if (category) - path.emplace_back (category); - if (qof_instance_has_path_slot (QOF_INSTANCE (acc), path)) - { - qof_instance_get_path_kvp (QOF_INSTANCE (acc), &v, path); - if (G_VALUE_HOLDS_STRING (&v)) - { - gchar const *string; - string = g_value_get_string (&v); - text = g_strdup (string); - } - } - g_value_unset (&v); - return text; + return get_kvp_string_path (acc, {head, category}); + else + return get_kvp_string_path (acc, {head}); } From 86cf327f094885882cd8ce1babe4c48e9407a1ee Mon Sep 17 00:00:00 2001 From: Christopher Lam Date: Mon, 13 Sep 2021 23:43:38 +0800 Subject: [PATCH 2/3] [account.cpp] GValue containing string must be unset already have comprehensive tests in 15852031d --- libgnucash/engine/Account.cpp | 81 +++++++++++++++++------------------ libgnucash/engine/AccountP.h | 3 ++ 2 files changed, 42 insertions(+), 42 deletions(-) diff --git a/libgnucash/engine/Account.cpp b/libgnucash/engine/Account.cpp index 9253139d52..68afc3801b 100644 --- a/libgnucash/engine/Account.cpp +++ b/libgnucash/engine/Account.cpp @@ -325,6 +325,9 @@ gnc_account_init(Account* acc) priv->starting_reconciled_balance = gnc_numeric_zero(); priv->balance_dirty = FALSE; + priv->last_num = (char*) is_unset; + priv->tax_us_code = (char*) is_unset; + priv->tax_us_pns = (char*) is_unset; priv->color = (char*) is_unset; priv->sort_order = (char*) is_unset; priv->notes = (char*) is_unset; @@ -1374,6 +1377,12 @@ xaccFreeAccount (Account *acc) qof_string_cache_remove(priv->description); priv->accountName = priv->accountCode = priv->description = nullptr; + if (priv->last_num != is_unset) + g_free (priv->last_num); + if (priv->tax_us_code != is_unset) + g_free (priv->tax_us_code); + if (priv->tax_us_pns != is_unset) + g_free (priv->tax_us_pns); if (priv->color != is_unset) g_free (priv->color); if (priv->sort_order != is_unset) @@ -1386,6 +1395,9 @@ xaccFreeAccount (Account *acc) /* zero out values, just in case stray * pointers are pointing here. */ + priv->last_num = nullptr; + priv->tax_us_code = nullptr; + priv->tax_us_pns = nullptr; priv->color == nullptr; priv->sort_order == nullptr; priv->notes == nullptr; @@ -4098,49 +4110,39 @@ xaccAccountSetTaxRelated (Account *acc, gboolean tax_related) const char * xaccAccountGetTaxUSCode (const Account *acc) { - GValue v = G_VALUE_INIT; - g_return_val_if_fail(GNC_IS_ACCOUNT(acc), FALSE); - qof_instance_get_path_kvp (QOF_INSTANCE(acc), &v, {"tax-US", "code"}); - return G_VALUE_HOLDS_STRING (&v) ? g_value_get_string (&v) : NULL; + auto priv = GET_PRIVATE (acc); + if (priv->tax_us_code == is_unset) + priv->tax_us_code = get_kvp_string_path (acc, {"tax-US", "code"}); + return priv->tax_us_code; } void xaccAccountSetTaxUSCode (Account *acc, const char *code) { - GValue v = G_VALUE_INIT; - g_return_if_fail(GNC_IS_ACCOUNT(acc)); - - g_value_init (&v, G_TYPE_STRING); - g_value_set_string (&v, code); - xaccAccountBeginEdit (acc); - qof_instance_set_path_kvp (QOF_INSTANCE (acc), &v, {"tax-US", "code"}); - mark_account (acc); - xaccAccountCommitEdit (acc); - g_value_unset (&v); + auto priv = GET_PRIVATE (acc); + if (priv->tax_us_code != is_unset) + g_free (priv->tax_us_code); + priv->tax_us_code = g_strdup (code); + set_kvp_string_path (acc, {"tax-US", "code"}, priv->tax_us_code); } const char * xaccAccountGetTaxUSPayerNameSource (const Account *acc) { - GValue v = G_VALUE_INIT; - g_return_val_if_fail(GNC_IS_ACCOUNT(acc), FALSE); - qof_instance_get_path_kvp (QOF_INSTANCE(acc), &v, {"tax-US", "payer-name-source"}); - return G_VALUE_HOLDS_STRING (&v) ? g_value_get_string (&v) : NULL; + auto priv = GET_PRIVATE (acc); + if (priv->tax_us_pns == is_unset) + priv->tax_us_pns = get_kvp_string_path (acc, {"tax-US", "payer-name-source"}); + return priv->tax_us_pns; } void xaccAccountSetTaxUSPayerNameSource (Account *acc, const char *source) { - GValue v = G_VALUE_INIT; - g_return_if_fail(GNC_IS_ACCOUNT(acc)); - - g_value_init (&v, G_TYPE_STRING); - g_value_set_string (&v, source); - xaccAccountBeginEdit (acc); - qof_instance_set_path_kvp (QOF_INSTANCE (acc), &v, {"tax-US", "payer-name-source"}); - mark_account (acc); - xaccAccountCommitEdit (acc); - g_value_unset (&v); + auto priv = GET_PRIVATE (acc); + if (priv->tax_us_pns != is_unset) + g_free (priv->tax_us_pns); + priv->tax_us_pns = g_strdup (source); + set_kvp_string_path (acc, {"tax-US", "payer-name-source"}, priv->tax_us_pns); } gint64 @@ -4871,10 +4873,10 @@ xaccAccountClearReconcilePostpone (Account *acc) const char * xaccAccountGetLastNum (const Account *acc) { - GValue v = G_VALUE_INIT; - g_return_val_if_fail(GNC_IS_ACCOUNT(acc), FALSE); - qof_instance_get_path_kvp (QOF_INSTANCE(acc), &v, {"last-num"}); - return G_VALUE_HOLDS_STRING (&v) ? g_value_get_string (&v) : NULL; + auto priv = GET_PRIVATE (acc); + if (priv->last_num == is_unset) + priv->last_num = get_kvp_string_tag (acc, "last-num"); + return priv->last_num; } /********************************************************************\ @@ -4883,16 +4885,11 @@ xaccAccountGetLastNum (const Account *acc) void xaccAccountSetLastNum (Account *acc, const char *num) { - GValue v = G_VALUE_INIT; - g_return_if_fail(GNC_IS_ACCOUNT(acc)); - g_value_init (&v, G_TYPE_STRING); - - g_value_set_string (&v, num); - xaccAccountBeginEdit (acc); - qof_instance_set_path_kvp (QOF_INSTANCE (acc), &v, {"last-num"}); - mark_account (acc); - xaccAccountCommitEdit (acc); - g_value_unset (&v); + auto priv = GET_PRIVATE (acc); + if (priv->last_num != is_unset) + g_free (priv->last_num); + priv->last_num = g_strdup (num); + set_kvp_string_tag (acc, "last-num", priv->last_num); } static Account * diff --git a/libgnucash/engine/AccountP.h b/libgnucash/engine/AccountP.h index 250232206d..14faeb6c0e 100644 --- a/libgnucash/engine/AccountP.h +++ b/libgnucash/engine/AccountP.h @@ -133,6 +133,9 @@ typedef struct AccountPrivate TriState equity_type; char *notes; char *color; + char *tax_us_code; + char *tax_us_pns; + char *last_num; char *sort_order; char *filter; From 87b61bf6fb623f97ee89528d028549d9a9a28ac1 Mon Sep 17 00:00:00 2001 From: Christopher Lam Date: Sun, 19 Sep 2021 10:36:09 +0800 Subject: [PATCH 3/3] [account.cpp] GValue must be unset in old functions GValue in DxaccAccount[Set|Get]Currency is unset correctly. However dxaccAccountGetPriceSrc and dxaccAccountSetQuoteTZ reuse a static char* therefore the latter must be used carefully before calling the function again. The functions are tested in 43094697306ec68c1133dfb11ef357bae8f473ca. --- libgnucash/engine/Account.cpp | 52 +++++++++++++---------------------- libgnucash/engine/Account.h | 5 ++-- 2 files changed, 22 insertions(+), 35 deletions(-) diff --git a/libgnucash/engine/Account.cpp b/libgnucash/engine/Account.cpp index 68afc3801b..03e461d591 100644 --- a/libgnucash/engine/Account.cpp +++ b/libgnucash/engine/Account.cpp @@ -2733,6 +2733,7 @@ DxaccAccountSetCurrency (Account * acc, gnc_commodity * currency) qof_instance_set_path_kvp (QOF_INSTANCE (acc), &v, {"old-currency"}); mark_account (acc); xaccAccountCommitEdit(acc); + g_value_unset (&v); table = gnc_commodity_table_get_table (qof_instance_get_book(acc)); commodity = gnc_commodity_table_lookup_unique (table, s); @@ -3384,16 +3385,20 @@ DxaccAccountGetCurrency (const Account *acc) GValue v = G_VALUE_INIT; const char *s = NULL; gnc_commodity_table *table; + gnc_commodity *retval = NULL; if (!acc) return NULL; qof_instance_get_path_kvp (QOF_INSTANCE(acc), &v, {"old-currency"}); if (G_VALUE_HOLDS_STRING (&v)) s = g_value_get_string (&v); - if (!s) return NULL; - - table = gnc_commodity_table_get_table (qof_instance_get_book(acc)); + if (s) + { + table = gnc_commodity_table_get_table (qof_instance_get_book(acc)); + retval = gnc_commodity_table_lookup_unique (table, s); + } + g_value_unset (&v); - return gnc_commodity_table_lookup_unique (table, s); + return retval; } gnc_commodity * @@ -4983,22 +4988,7 @@ dxaccAccountSetPriceSrc(Account *acc, const char *src) if (!acc) return; if (xaccAccountIsPriced(acc)) - { - xaccAccountBeginEdit(acc); - if (src) - { - GValue v = G_VALUE_INIT; - g_value_init (&v, G_TYPE_STRING); - g_value_set_string (&v, src); - qof_instance_set_path_kvp (QOF_INSTANCE(acc), &v, {"old-price-source"}); - g_value_unset (&v); - } - else - qof_instance_set_path_kvp (QOF_INSTANCE(acc), nullptr, {"old-price-source"}); - - mark_account (acc); - xaccAccountCommitEdit(acc); - } + set_kvp_string_tag (acc, "old-price-source", src); } /********************************************************************\ @@ -5007,13 +4997,14 @@ dxaccAccountSetPriceSrc(Account *acc, const char *src) const char* dxaccAccountGetPriceSrc(const Account *acc) { - GValue v = G_VALUE_INIT; + static char *source = nullptr; if (!acc) return NULL; if (!xaccAccountIsPriced(acc)) return NULL; - qof_instance_get_path_kvp (QOF_INSTANCE(acc), &v, {"old-price-source"}); - return G_VALUE_HOLDS_STRING (&v) ? g_value_get_string (&v) : NULL; + g_free (source); + source = get_kvp_string_tag (acc, "old-price-source"); + return source; } /********************************************************************\ @@ -5022,15 +5013,9 @@ dxaccAccountGetPriceSrc(const Account *acc) void dxaccAccountSetQuoteTZ(Account *acc, const char *tz) { - GValue v = G_VALUE_INIT; if (!acc) return; if (!xaccAccountIsPriced(acc)) return; - xaccAccountBeginEdit(acc); - g_value_init (&v, G_TYPE_STRING); - g_value_set_string (&v, tz); - qof_instance_set_path_kvp (QOF_INSTANCE (acc), &v, {"old-quote-tz"}); - mark_account (acc); - xaccAccountCommitEdit(acc); + set_kvp_string_tag (acc, "old-quote-tz", tz); } /********************************************************************\ @@ -5039,11 +5024,12 @@ dxaccAccountSetQuoteTZ(Account *acc, const char *tz) const char* dxaccAccountGetQuoteTZ(const Account *acc) { - GValue v = G_VALUE_INIT; + static char *quote_tz = nullptr; if (!acc) return NULL; if (!xaccAccountIsPriced(acc)) return NULL; - qof_instance_get_path_kvp (QOF_INSTANCE (acc), &v, {"old-quote-tz"}); - return G_VALUE_HOLDS_STRING (&v) ? g_value_get_string (&v) : NULL; + g_free (quote_tz); + quote_tz = get_kvp_string_tag (acc, "old-quote-tz"); + return quote_tz; } /********************************************************************\ diff --git a/libgnucash/engine/Account.h b/libgnucash/engine/Account.h index 72f525a03b..6822bfd655 100644 --- a/libgnucash/engine/Account.h +++ b/libgnucash/engine/Account.h @@ -469,7 +469,7 @@ Account * xaccAccountGainsAccount (Account *acc, gnc_commodity *curr); void dxaccAccountSetPriceSrc (Account *account, const char *src); /** Get a string that identifies the Finance::Quote backend that * should be used to retrieve online prices. See price-quotes.scm - * for more information. + * for more information. This function uses a static char*. * * @deprecated Price quote information is now stored on the * commodity, not the account. */ @@ -1588,7 +1588,8 @@ gnc_commodity * DxaccAccountGetCurrency (const Account *account); void dxaccAccountSetQuoteTZ (Account *account, const char *tz); /** Get the timezone to be used when interpreting the results from a * given Finance::Quote backend. Unfortunately, the upstream sources - * don't label their output, so the user has to specify this bit. + * don't label their output, so the user has to specify this + * bit. This function uses a static char*. * * @deprecated Price quote information is now stored on the * commodity, not the account. */