From cdb674ebf1d097cb2c15a71f07426a0c1ae12f36 Mon Sep 17 00:00:00 2001 From: John Ralls Date: Mon, 6 Mar 2023 11:43:50 -0800 Subject: [PATCH] [c++options]Really fix the gnc_option_db_lookup_string_value leak By deleting the function and using GncOptionDbImpl::lookup_string_option directly. It returns a string that we don't have to worry about memory-managing. Also create a new GncOptionDbImpl::set_string_option to replace gnc_option_db_set_string_value. --- gnucash/gnome/gnc-plugin-page-report.cpp | 42 +++++++++++------------- libgnucash/engine/gnc-optiondb-impl.hpp | 4 +++ libgnucash/engine/gnc-optiondb.cpp | 15 --------- libgnucash/engine/gnc-optiondb.h | 28 +--------------- 4 files changed, 25 insertions(+), 64 deletions(-) diff --git a/gnucash/gnome/gnc-plugin-page-report.cpp b/gnucash/gnome/gnc-plugin-page-report.cpp index 5135c8e352..d2bf4368ee 100644 --- a/gnucash/gnome/gnc-plugin-page-report.cpp +++ b/gnucash/gnome/gnc-plugin-page-report.cpp @@ -773,8 +773,6 @@ gnc_plugin_page_report_option_change_cb(gpointer data) GncPluginPageReport *report; GncPluginPageReportPrivate *priv; SCM dirty_report = scm_c_eval_string("gnc:report-set-dirty?!"); - const gchar *old_name; - gchar *new_name; g_return_if_fail(GNC_IS_PLUGIN_PAGE_REPORT(data)); report = GNC_PLUGIN_PAGE_REPORT(data); @@ -787,19 +785,19 @@ gnc_plugin_page_report_option_change_cb(gpointer data) DEBUG( "set-dirty, queue-draw" ); /* Update the page (i.e. the notebook tab and window title) */ - old_name = gnc_plugin_page_get_page_name(GNC_PLUGIN_PAGE(report)); - new_name = g_strdup(gnc_option_db_lookup_string_value(priv->cur_odb, - "General", - "Report name")); - if (strcmp(old_name, new_name) != 0) + std::string old_name{gnc_plugin_page_get_page_name(GNC_PLUGIN_PAGE(report))}; + auto new_name{priv->cur_odb->lookup_string_option("General", + "Report name")}; + if (new_name != old_name) { /* Bug 727130, 760711 - remove only the non-printable * characters from the new name */ - gnc_utf8_strip_invalid_and_controls(new_name); - ENTER("Cleaned-up new report name: %s", new_name); - main_window_update_page_name(GNC_PLUGIN_PAGE(report), new_name); + char *clean_name{g_strdup(new_name.c_str())}; + gnc_utf8_strip_invalid_and_controls(clean_name); + ENTER("Cleaned-up new report name: %s", clean_name ? clean_name : "(null)"); + main_window_update_page_name(GNC_PLUGIN_PAGE(report), clean_name); + g_free(clean_name); } - g_free(new_name); /* it's probably already dirty, but make sure */ scm_call_2(dirty_report, priv->cur_report, SCM_BOOL_T); @@ -1083,7 +1081,6 @@ static void gnc_plugin_page_report_name_changed (GncPluginPage *page, const gchar *name) { GncPluginPageReportPrivate *priv; - const gchar *old_name; g_return_if_fail(GNC_IS_PLUGIN_PAGE_REPORT(page)); g_return_if_fail(name != nullptr); @@ -1095,19 +1092,19 @@ gnc_plugin_page_report_name_changed (GncPluginPage *page, const gchar *name) { /* Is this a redundant call? */ - old_name = gnc_option_db_lookup_string_value(priv->cur_odb, "General", - "Report name"); + auto old_name = priv->cur_odb->lookup_string_option("General", + "Report name"); + std::string name_str{name}; DEBUG("Comparing old name '%s' to new name '%s'", - old_name ? old_name : "(null)", name); - if (old_name && (strcmp(old_name, name) == 0)) + old_name.empty() ? "(null)" : old_name.c_str(), name); + if (old_name == name_str) { LEAVE("no change"); return; } /* Store the new name for the report. */ - gnc_option_db_set_string_value(priv->cur_odb, "General", - "Report name", name); + priv->cur_odb->set_string_option("General", "Report name", name_str); } @@ -1900,10 +1897,11 @@ static gchar *report_create_jobname(GncPluginPageReportPrivate *priv) * so I added yet another hack below for this. cstim. */ GncInvoice *invoice; - report_name = g_strdup(gnc_option_db_lookup_string_value(priv->cur_odb, - "General", - "Report name")); - if (!report_name) + auto report_name_str = priv->cur_odb->lookup_string_option("General", + "Report name"); + if (!report_name_str.empty()) + report_name = g_strdup(report_name_str.c_str()); + if (report_name) report_name = g_strdup (_(default_jobname)); if (g_strcmp0(report_name, _("Printable Invoice")) == 0 || g_strcmp0(report_name, _("Tax Invoice")) == 0 diff --git a/libgnucash/engine/gnc-optiondb-impl.hpp b/libgnucash/engine/gnc-optiondb-impl.hpp index 6febc36f08..a1ccc85205 100644 --- a/libgnucash/engine/gnc-optiondb-impl.hpp +++ b/libgnucash/engine/gnc-optiondb-impl.hpp @@ -132,6 +132,10 @@ public: void set_default_section(const char* section); const GncOptionSection* const get_default_section() const noexcept; std::string lookup_string_option(const char* section, const char* name); + bool set_string_option(const char* section, const char* name, const std::string& value) + { + return set_option(section, name, value); + } template bool set_option(const char* section, const char* name, ValueType value) { diff --git a/libgnucash/engine/gnc-optiondb.cpp b/libgnucash/engine/gnc-optiondb.cpp index 5309ae7ac8..b7f4f4083e 100644 --- a/libgnucash/engine/gnc-optiondb.cpp +++ b/libgnucash/engine/gnc-optiondb.cpp @@ -1256,21 +1256,6 @@ gnc_option_db_book_options(GncOptionDB* odb) N_("The electronic tax number of your business"), empty_string); } - -const char* -gnc_option_db_lookup_string_value(GncOptionDB* odb, const char* section, const char* name) -{ - auto value{odb->lookup_string_option(section, name)}; - return value.empty() ? nullptr : strdup(value.c_str()); -} - -void -gnc_option_db_set_string_value(GncOptionDB* odb, const char* section, - const char* name, const char* value) -{ - odb->set_option(section, name, value); -} - const QofInstance* gnc_option_db_lookup_qofinstance_value(GncOptionDB* odb, const char* section, const char* name) diff --git a/libgnucash/engine/gnc-optiondb.h b/libgnucash/engine/gnc-optiondb.h index 61ad9afc5c..1a49920f53 100644 --- a/libgnucash/engine/gnc-optiondb.h +++ b/libgnucash/engine/gnc-optiondb.h @@ -126,33 +126,7 @@ void gnc_option_db_save(GncOptionDB* odb, QofBook* book, void gnc_option_db_book_options(GncOptionDB*); /** - * Retrieve the string value of an option in the GncOptionDB - * - * @param odb the GncOptionDB - * @param section the section in which the option is stored - * @param name the option name - * @return the static char* of the value or nullptr if the option isn't found - * or if its value isn't a string. - */ -const char* gnc_option_db_lookup_string_value(GncOptionDB*, const char*, - const char*); - -/** - * Set the string value of an option in the GncOptionDB. - * - * The value will not be saved if the option is not in the GncOptionDB or if the - * type of the option isn't string or text. - * - * @param odb the GncOptionDB - * @param section the section in which the option is stored - * @param name the option name - * @param value the value to be stored in the option. - */ -void gnc_option_db_set_string_value(GncOptionDB*, const char*, - const char*, const char*); - -/** - * Retrieve the string value of an option in the GncOptionDB + * Retrieve the QofInstance value of an option in the GncOptionDB * * @param odb the GncOptionDB * @param section the section in which the option is stored