gnucash stable: Multiple changes pushed
Christopher Lam
clam at code.gnucash.org
Thu Mar 28 08:40:24 EDT 2024
Updated via https://github.com/Gnucash/gnucash/commit/dc1eb874 (commit)
via https://github.com/Gnucash/gnucash/commit/6c82a131 (commit)
via https://github.com/Gnucash/gnucash/commit/249ec9f4 (commit)
via https://github.com/Gnucash/gnucash/commit/1f9ea6bc (commit)
from https://github.com/Gnucash/gnucash/commit/4b0379f1 (commit)
commit dc1eb874095e0c028a1fb3fbbf13ddfb88a0c310
Merge: 4b0379f1e9 6c82a1311b
Author: Christopher Lam <christopher.lck at gmail.com>
Date: Thu Mar 28 20:39:03 2024 +0800
Merge branch 'sixtp-string-converters' into stable #1892
commit 6c82a1311b85c7036d3bdc012ab938a3b443eabf
Author: Christopher Lam <christopher.lck at gmail.com>
Date: Thu Mar 21 00:25:09 2024 +0800
[sixtp-dom-parsers.cpp] use string_to_guint, string_to_guint16
diff --git a/libgnucash/backend/xml/sixtp-dom-parsers.cpp b/libgnucash/backend/xml/sixtp-dom-parsers.cpp
index f59a0afd5b..b6fc8ff1af 100644
--- a/libgnucash/backend/xml/sixtp-dom-parsers.cpp
+++ b/libgnucash/backend/xml/sixtp-dom-parsers.cpp
@@ -98,46 +98,32 @@ dom_tree_to_integer_kvp_value (xmlNodePtr node)
return ret;
}
-gboolean
-dom_tree_to_integer (xmlNodePtr node, gint64* daint)
+template <typename T>
+static bool
+dom_tree_to_num (xmlNodePtr node, std::function<bool(const char*, T*)>string_to_num, T* num_ptr)
{
- gchar* text;
- gboolean ret;
-
- text = dom_tree_to_text (node);
-
- ret = string_to_gint64 (text, daint);
-
+ auto text = dom_tree_to_text (node);
+ auto ret = string_to_num (text, num_ptr);
g_free (text);
-
return ret;
}
gboolean
-dom_tree_to_guint16 (xmlNodePtr node, guint16* i)
+dom_tree_to_integer (xmlNodePtr node, gint64* daint)
{
- gboolean ret;
- guint j = 0;
+ return dom_tree_to_num<gint64>(node, string_to_gint64, daint);
+}
- ret = dom_tree_to_guint (node, &j);
- *i = (guint16) j;
- return ret;
+gboolean
+dom_tree_to_guint16 (xmlNodePtr node, guint16* i)
+{
+ return dom_tree_to_num<guint16>(node, string_to_guint16, i);
}
gboolean
dom_tree_to_guint (xmlNodePtr node, guint* i)
{
- gchar* text, *endptr;
- gboolean ret;
-
- text = dom_tree_to_text (node);
- /* In spite of the strange string_to_gint64 function, I'm just
- going to use strtoul here until someone shows me the error of
- my ways. -CAS */
- *i = (guint) strtoul (text, &endptr, 0);
- ret = (endptr != text);
- g_free (text);
- return ret;
+ return dom_tree_to_num<guint>(node, string_to_guint, i);
}
gboolean
commit 249ec9f43a0780b743e2fb2f0be2955399daad11
Author: Christopher Lam <christopher.lck at gmail.com>
Date: Wed Mar 20 23:40:23 2024 +0800
[test-string-converters.cpp] add some string->number tests
diff --git a/libgnucash/backend/xml/test/test-string-converters.cpp b/libgnucash/backend/xml/test/test-string-converters.cpp
index c2baf05738..4dcd104d8d 100644
--- a/libgnucash/backend/xml/test/test-string-converters.cpp
+++ b/libgnucash/backend/xml/test/test-string-converters.cpp
@@ -25,10 +25,12 @@
#include "test-engine-stuff.h"
#include "test-file-stuff.h"
+#include "sixtp-utils.h"
#include "sixtp-dom-parsers.h"
#include "sixtp-dom-generators.h"
#include "test-stuff.h"
+#include <optional>
#define GNC_V2_STRING "gnc-v2"
const gchar* gnc_v2_xml_version_string = GNC_V2_STRING;
@@ -79,6 +81,85 @@ test_bad_string (void)
xmlFreeNode (test_node);
}
+template <class T>
+using TestcaseVec = std::vector<std::pair<const char*, std::optional<T>>>;
+
+template <class T>
+const TestcaseVec<T> test_cases_common = {
+ { "1" , 1 },
+ { " \t 2 \t\v\n\f\r " , 2 },
+ { " 0 " , 0 },
+ { "123" , 123 },
+ { "123z" , {} },
+ { "a123" , {} },
+ { " 23" , 23 },
+ { "\t23" , 23 },
+ { "44 " , 44 },
+ { "44\t" , 44 },
+ { " 56 " , 56 },
+ { "\t56\t" , 56 },
+ { "1 2" , {} },
+ { "1 2 \t" , {} },
+};
+
+const TestcaseVec<gint64> test_cases_gint64 = {
+ { "-44" , -44 },
+ { "9223372036854775807" , 9223372036854775807 }, // maxint64
+ { "9223372036854775808" , {} }, // overflow
+ { "-9223372036854775807" , -9223372036854775807 }, // minint64
+};
+
+const TestcaseVec<guint> test_cases_guint = {
+ { "-44" , {} }, // no negative allowed
+ { "4294967295" , 4294967295 }, // max_uint
+ { "4294967296" , {} }, // overflow
+};
+
+const TestcaseVec<guint16> test_cases_guint16 = {
+ { "-44" , {} }, // no negative allowed
+ { "65535" , 65535 }, // max_int16
+ { "65536" , {} }, // overflow
+};
+
+const TestcaseVec<double> test_cases_double = {
+ { "-3.5" , -3.5 },
+ { ".5" , 0.5 },
+ { "1e10" , 1e10 },
+ { "-1e10" , -1e10 },
+ { "1e-10" , 1e-10 },
+ { "-1e-10" , -1e-10 },
+ { "1.7976931348623158e+308", 1.7976931348623158e+308 }, // max_double
+ { "1.7976931348623159e+308", {} }, // overflow
+};
+
+template <typename T>
+static void
+test_string_to_num (const char *func_name, const TestcaseVec<T>& test_cases,
+ const std::function<bool(const char*,T*)> &func)
+{
+ auto do_test = [&](std::pair<const char*, std::optional<T>> test_pair)
+ {
+ T num = 0;
+ auto [test_str, test_int] = test_pair;
+ auto rv = func (test_str, &num);
+ // Output for debugging
+ // std::cout << "test_str = [" << test_str << "], test_int = ";
+ // if (test_int)
+ // std::cout << *test_int;
+ // else
+ // std::cout << "{}";
+ // std::cout << ", num = [" << num << "], rv = " << rv << std::endl;
+ do_test_args (rv == test_int.has_value(), func_name,
+ __FILE__, __LINE__, "with string %s", test_str);
+ if (rv)
+ do_test_args (num == *test_int, func_name,
+ __FILE__, __LINE__, "with string %s", test_str);
+ };
+
+ std::for_each (test_cases_common<T>.begin(), test_cases_common<T>.end(), do_test);
+ std::for_each (test_cases.begin(), test_cases.end(), do_test);
+}
+
int
main (int argc, char** argv)
{
@@ -86,6 +167,13 @@ main (int argc, char** argv)
fflush (stdout);
test_string_converters ();
test_bad_string ();
+#if __cpp_lib_to_chars >= 201611L
+ // because older strtod code is more liberal and parses "123z" as 123.0
+ test_string_to_num<double> ("string_to_double", test_cases_double, string_to_double);
+#endif
+ test_string_to_num<gint64> ("string_to_gint64", test_cases_gint64, string_to_gint64);
+ test_string_to_num<guint16>("string_to_guint16",test_cases_guint16,string_to_guint16);
+ test_string_to_num<guint> ("string_to_guint", test_cases_guint, string_to_guint);
fflush (stdout);
print_test_results ();
exit (get_rv ());
commit 1f9ea6bc997e8ab35c1299340046e2d7bf0aa68b
Author: Christopher Lam <christopher.lck at gmail.com>
Date: Thu Mar 21 00:03:28 2024 +0800
[sixtp-utils.cpp] std::from_chars speedup, remove string_to_gint32
Benchmarks running tests 4E6 times:
using sscanf
$ bin/test-string-converters
elapsed=7.33942s
Executed 92000006 tests. All tests passed.
$ bin/test-string-converters
elapsed=7.3811s
Executed 92000006 tests. All tests passed.
$ bin/test-string-converters
elapsed=7.3455s
Executed 92000006 tests. All tests passed.
with std::from_chars
$ bin/test-string-converters
elapsed=4.47369s
Executed 92000006 tests. All tests passed.
$ bin/test-string-converters
elapsed=4.46908s
Executed 92000006 tests. All tests passed.
$ bin/test-string-converters
elapsed=4.47067s
Executed 92000006 tests. All tests passed.
$ bin/test-string-converters
elapsed=4.48706s
Executed 92000006 tests. All tests passed.
diff --git a/libgnucash/backend/xml/sixtp-utils.cpp b/libgnucash/backend/xml/sixtp-utils.cpp
index b65d547357..4e3539ad3c 100644
--- a/libgnucash/backend/xml/sixtp-utils.cpp
+++ b/libgnucash/backend/xml/sixtp-utils.cpp
@@ -42,6 +42,9 @@
#include "sixtp.h"
#include "sixtp-utils.h"
+#include <charconv>
+#include <cctype>
+
static QofLogModule log_module = GNC_MOD_IO;
gboolean
@@ -138,6 +141,27 @@ concatenate_child_result_chars (GSList* data_from_children)
*/
+template <typename T>
+static bool parse_chars_into_num (const char* str, T* num_ptr)
+{
+ if (!str || !num_ptr)
+ return false;
+
+ while (std::isspace (*str))
+ ++str;
+
+ const char* end_ptr = str + std::strlen (str);
+
+ auto res = std::from_chars (str, end_ptr, *num_ptr);
+ if (res.ec != std::errc{})
+ return false;
+
+ while (std::isspace (*res.ptr))
+ ++res.ptr;
+
+ return (res.ptr == end_ptr);
+}
+
/*********/
/* double
*/
@@ -145,78 +169,42 @@ concatenate_child_result_chars (GSList* data_from_children)
gboolean
string_to_double (const char* str, double* result)
{
- char* endptr = 0x0;
-
- g_return_val_if_fail (str, FALSE);
- g_return_val_if_fail (result, FALSE);
-
- *result = strtod (str, &endptr);
- if (endptr == str) return (FALSE);
-
- return (TRUE);
+#if __cpp_lib_to_chars >= 201611L
+ return parse_chars_into_num<double>(str, result);
+#else
+ // because from_chars in cpp < 201611L cannot parse floats
+ char* endptr = nullptr;
+ g_return_val_if_fail (str && result, false);
+ *result = std::strtod (str, &endptr);
+ return (endptr != str);
+#endif
}
/*********/
/* gint64
*/
-/* Maybe there should be a comment here explaining why this function
- doesn't call g_ascii_strtoull, because it's not so obvious. -CAS */
gboolean
string_to_gint64 (const gchar* str, gint64* v)
{
- /* convert a string to a gint64. only whitespace allowed before and after. */
- long long int v_in;
- int num_read;
-
- g_return_val_if_fail (str, FALSE);
-
- /* must use "<" here because %n's effects aren't well defined */
- if (sscanf (str, " %lld%n", &v_in, &num_read) < 1)
- {
- return (FALSE);
- }
-
- /*
- * Mac OS X version 10.1 and under has a silly bug where scanf
- * returns bad values in num_read if there is a space before %n. It
- * is fixed in the next release 10.2 afaik
- */
- while ((* ((gchar*)str + num_read) != '\0') &&
- isspace (* ((unsigned char*)str + num_read)))
- num_read++;
-
- if (v)
- *v = v_in;
-
- if (!isspace_str (str + num_read, -1)) return (FALSE);
- return (TRUE);
+ return parse_chars_into_num<gint64>(str, v);
}
/*********/
-/* gint32
+/* guint16
*/
-
gboolean
-string_to_gint32 (const gchar* str, gint32* v)
+string_to_guint16 (const gchar* str, guint16* v)
{
- /* convert a string to a gint32. only whitespace allowed before and after. */
- int num_read;
- int v_in;
-
- /* must use "<" here because %n's effects aren't well defined */
- if (sscanf (str, " %d%n", &v_in, &num_read) < 1)
- {
- return (FALSE);
- }
- while ((* ((gchar*)str + num_read) != '\0') &&
- isspace (* ((unsigned char*)str + num_read)))
- num_read++;
-
- if (v)
- *v = v_in;
+ return parse_chars_into_num<guint16>(str, v);
+}
- if (!isspace_str (str + num_read, -1)) return (FALSE);
- return (TRUE);
+/*********/
+/* guint
+ */
+gboolean
+string_to_guint (const gchar* str, guint* v)
+{
+ return parse_chars_into_num<guint>(str, v);
}
/************/
diff --git a/libgnucash/backend/xml/sixtp-utils.h b/libgnucash/backend/xml/sixtp-utils.h
index ef7b0f03f0..417b767b72 100644
--- a/libgnucash/backend/xml/sixtp-utils.h
+++ b/libgnucash/backend/xml/sixtp-utils.h
@@ -63,7 +63,9 @@ gboolean string_to_double (const char* str, double* result);
gboolean string_to_gint64 (const gchar* str, gint64* v);
-gboolean string_to_gint32 (const gchar* str, gint32* v);
+gboolean string_to_guint16 (const gchar* str, guint16* v);
+
+gboolean string_to_guint (const gchar* str, guint* v);
gboolean hex_string_to_binary (const gchar* str, void** v, guint64* data_len);
Summary of changes:
libgnucash/backend/xml/sixtp-dom-parsers.cpp | 40 +++-----
libgnucash/backend/xml/sixtp-utils.cpp | 102 +++++++++------------
libgnucash/backend/xml/sixtp-utils.h | 4 +-
.../backend/xml/test/test-string-converters.cpp | 88 ++++++++++++++++++
4 files changed, 149 insertions(+), 85 deletions(-)
More information about the gnucash-changes
mailing list