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 +static bool +dom_tree_to_num (xmlNodePtr node, std::functionstring_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(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(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(node, string_to_guint, i); } gboolean 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 +#include + static QofLogModule log_module = GNC_MOD_IO; gboolean @@ -138,6 +141,27 @@ concatenate_child_result_chars (GSList* data_from_children) */ +template +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(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(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(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(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); 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 #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 +using TestcaseVec = std::vector>>; + +template +const TestcaseVec 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 test_cases_gint64 = { + { "-44" , -44 }, + { "9223372036854775807" , 9223372036854775807 }, // maxint64 + { "9223372036854775808" , {} }, // overflow + { "-9223372036854775807" , -9223372036854775807 }, // minint64 +}; + +const TestcaseVec test_cases_guint = { + { "-44" , {} }, // no negative allowed + { "4294967295" , 4294967295 }, // max_uint + { "4294967296" , {} }, // overflow +}; + +const TestcaseVec test_cases_guint16 = { + { "-44" , {} }, // no negative allowed + { "65535" , 65535 }, // max_int16 + { "65536" , {} }, // overflow +}; + +const TestcaseVec 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 +static void +test_string_to_num (const char *func_name, const TestcaseVec& test_cases, + const std::function &func) +{ + auto do_test = [&](std::pair> 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.begin(), test_cases_common.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 ("string_to_double", test_cases_double, string_to_double); +#endif + test_string_to_num ("string_to_gint64", test_cases_gint64, string_to_gint64); + test_string_to_num("string_to_guint16",test_cases_guint16,string_to_guint16); + test_string_to_num ("string_to_guint", test_cases_guint, string_to_guint); fflush (stdout); print_test_results (); exit (get_rv ());