gnucash master: Multiple changes pushed

John Ralls jralls at code.gnucash.org
Mon Mar 6 18:38:21 EST 2023


Updated	 via  https://github.com/Gnucash/gnucash/commit/1341511e (commit)
	 via  https://github.com/Gnucash/gnucash/commit/cdb674eb (commit)
	from  https://github.com/Gnucash/gnucash/commit/c474d81e (commit)



commit 1341511edb7e8166867b6d7069841bbb1b2dbddd
Author: John Ralls <jralls at ceridwen.us>
Date:   Mon Mar 6 15:36:23 2023 -0800

    [c++options] Save an empty SCM string for nil or empty option values.
    
    Avoids crash at reload due to unknown variable.

diff --git a/bindings/guile/gnc-optiondb.i b/bindings/guile/gnc-optiondb.i
index ac29b91326..6aba978442 100644
--- a/bindings/guile/gnc-optiondb.i
+++ b/bindings/guile/gnc-optiondb.i
@@ -1195,13 +1195,13 @@ inline SCM return_scm_value(ValueType value)
 //scm_simple_format needs a scheme list of arguments to match the format
 //placeholders.
         return std::visit([$self] (auto &option) -> SCM {
-                static const auto no_value{scm_from_utf8_string("No Value")};
+                static const auto no_value{scm_from_utf8_string("")};
                 if constexpr (is_same_decayed_v<decltype(option),
                               GncOptionAccountListValue>)
                 {
                     auto guid_list{option.get_value()};
                     if (guid_list.empty())
-                        return no_value;
+                        return scm_simple_format(SCM_BOOL_F, list_format_str, scm_list_1(no_value));
                     SCM string_list{SCM_EOL};
                     for(auto guid : guid_list)
                     {
@@ -1215,10 +1215,8 @@ inline SCM return_scm_value(ValueType value)
                 if constexpr (is_QofInstanceValue_v<decltype(option)>)
                 {
                     auto serial{option.serialize()};
-                    if (serial.empty())
-                        return no_value;
-                    auto value{scm_list_1(scm_from_utf8_string(serial.c_str()))};
-                        return scm_simple_format(SCM_BOOL_F, plain_format_str, value);
+                    auto value{scm_list_1(scm_from_utf8_string(serial.empty() ? "" : serial.c_str()))};
+                    return scm_simple_format(SCM_BOOL_F, plain_format_str, value);
                 }
                 if constexpr (is_same_decayed_v<decltype(option),
                               GncOptionCommodityValue>)
@@ -1244,9 +1242,7 @@ inline SCM return_scm_value(ValueType value)
                               GncOptionDateValue>)
                 {
                     auto serial{option.serialize()};
-                    if (serial.empty())
-                        return no_value;
-                    auto value{scm_list_1(scm_from_utf8_string(serial.c_str()))};
+                    auto value{scm_list_1(scm_from_utf8_string(serial.empty() ? "" :serial.c_str()))};
                     const SCM date_fmt{scm_from_utf8_string("'~a")};
                     return scm_simple_format(SCM_BOOL_F, date_fmt, value);
                 }
@@ -1272,7 +1268,8 @@ inline SCM return_scm_value(ValueType value)
                     auto serial{option.serialize()};
                     if (serial.empty())
                     {
-                        return no_value;
+                        return scm_simple_format(SCM_BOOL_F, list_format_str,
+                                                 scm_list_1(no_value));
                     }
                     else
                     {
@@ -1298,15 +1295,8 @@ inline SCM return_scm_value(ValueType value)
                               GncOptionRangeValue<double>>)
                 {
                     auto serial{get_scm_value(option)};
-                    if (serial == SCM_BOOL_F)
-                    {
-                        return no_value;
-                    }
-                    else
-                    {
-                        auto scm_str{scm_list_1(serial)};
-                        return scm_simple_format(SCM_BOOL_F, ticked_format_str, scm_str);
-                    }
+                    auto scm_str{serial == SCM_BOOL_F ? scm_list_1(no_value) : scm_list_1(serial)};
+                    return scm_simple_format(SCM_BOOL_F, ticked_format_str, scm_str);
                 }
                 if constexpr (is_same_decayed_v<decltype(option),
                               GncOptionValue<bool>>)
@@ -1325,7 +1315,7 @@ inline SCM return_scm_value(ValueType value)
                 auto serial{option.serialize()};
                 if (serial.empty())
                 {
-                    return no_value;
+                    return scm_simple_format(SCM_BOOL_F, plain_format_str, scm_list_1(no_value));
                 }
                 else if ($self->get_ui_type() == GncOptionUIType::COLOR)
                 {

commit cdb674ebf1d097cb2c15a71f07426a0c1ae12f36
Author: John Ralls <jralls at ceridwen.us>
Date:   Mon Mar 6 11:43:50 2023 -0800

    [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.

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<std::string>(section, name, value);
+    }
     template <typename ValueType>
     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<std::string>(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



Summary of changes:
 bindings/guile/gnc-optiondb.i            | 30 ++++++++---------------
 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 +--------------------
 5 files changed, 35 insertions(+), 84 deletions(-)



More information about the gnucash-changes mailing list