From d78ac827e94a8c8ebedfea296e7447aa85c87fb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Helleu?= Date: Tue, 7 Apr 2020 21:37:48 +0200 Subject: [PATCH] core: fix memory leak in calculation of expression on FreeBSD (closes #1469) The memory leak was caused by a bug in function setlocale on FreeBSD: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=243195 The fix is the following: * Remove the calls to setlocale when formatting the result. * The function snprintf is still called, and then is now locale dependent, for example in French the decimal separator is a comma instead of a dot. * A new function calc_sanitize_decimal_number is introduced to "sanitize" a decimal number: keep only the decimal separator (replace it by a dot) and remove any other separator found. Unit tests are added on these functions: * calc_sanitize_decimal_number * calc_format_result --- ChangeLog.adoc | 1 + src/core/wee-calc.c | 107 ++++++++++++++++++++++++++--- tests/unit/core/test-core-calc.cpp | 66 ++++++++++++++++++ 3 files changed, 163 insertions(+), 11 deletions(-) diff --git a/ChangeLog.adoc b/ChangeLog.adoc index f74238892..b032818ae 100644 --- a/ChangeLog.adoc +++ b/ChangeLog.adoc @@ -24,6 +24,7 @@ New features:: Bug fixes:: + * core: fix memory leak in calculation of expression on FreeBSD (issue #1469) * core: fix resize of a bar when its size is 0 (automatic) (issue #1470) * python: fix crash when invalid UTF-8 string is in a WeeChat hashtable converted to a Python dict (issue #1463) diff --git a/src/core/wee-calc.c b/src/core/wee-calc.c index 175cb9913..3348b0eb4 100644 --- a/src/core/wee-calc.c +++ b/src/core/wee-calc.c @@ -175,6 +175,98 @@ calc_operation_stacks (struct t_arraylist *list_values, arraylist_remove (list_ops, size_ops - 1); } +/* + * Sanitizes a decimal number: removes any thousands separator and replaces + * the decimal separator by a dot. + * + * The string is updated in-place, the result has always a length shorter or + * equal to the input string. + * + * Examples: + * 1.23 --> 1.23 + * 1,23 --> 1,23 + * 1.234,56 --> 1234.56 + * 123.456.789 --> 123456789 + * 123,456,789 --> 123456789 + * 1.234.567,89 --> 1234567.89 + * 1,234,567.89 --> 1234567.89 + * -2.345,67 --> -2345.67 + * + * Returns: + * 1: the number has decimal part + * 0: the number has no decimal part + */ + +int +calc_sanitize_decimal_number (char *string) +{ + int i, j, count_sep, different_sep, index_last_sep; + + count_sep = 0; + different_sep = 0; + index_last_sep = -1; + + i = strlen (string) - 1; + while (i >= 0) + { + if (!isdigit ((unsigned char)string[i]) && (string[i] != '-')) + { + count_sep++; + if (index_last_sep < 0) + { + /* last separator found */ + index_last_sep = i; + } + else + { + /* another separator found */ + if (!different_sep && (string[i] != string[index_last_sep])) + { + different_sep = 1; + break; + } + } + } + i--; + } + if ((count_sep > 1) && !different_sep) + { + /* + * case of only thousands separators, like 123.456.789 + * => we strip all separators + */ + index_last_sep = -1; + } + + if (index_last_sep >= 0) + string[index_last_sep] = '.'; + + i = 0; + j = 0; + while (1) + { + if (((index_last_sep < 0) || (i < index_last_sep)) + && string[i] + && !isdigit ((unsigned char)string[i]) + && (string[i] != '-')) + { + /* another separator found before the last one: skip it */ + i++; + } + else + { + if (j != i) + string[j] = string[i]; + if (!string[i]) + break; + i++; + j++; + } + } + + return (index_last_sep >= 0) ? 1 : 0; +} + /* * Formats the result as a decimal number (locale independent): remove any * extra "0" at the and the decimal point if needed. @@ -183,31 +275,24 @@ calc_operation_stacks (struct t_arraylist *list_values, void calc_format_result (double value, char *result, int max_size) { - char *pos_point; - int i; + int i, has_decimal; - /* - * local-independent formatting of value, so that a decimal point is always - * used (instead of a comma in French for example) - */ - setlocale (LC_ALL, "C"); snprintf (result, max_size, "%.10f", /* ensure result is not "-0" */ (value == -0.0) ? 0.0 : value); - setlocale (LC_ALL, ""); - pos_point = strchr (result, '.'); + has_decimal = calc_sanitize_decimal_number (result); i = strlen (result) - 1; while (i >= 0) { - if (!isdigit ((unsigned char)result[i]) && (result[i] != '-')) + if (!isdigit ((unsigned char)result[i]) && (result[i] != '-')) { result[i] = '\0'; break; } - if (pos_point && (result[i] == '0')) + if (has_decimal && (result[i] == '0')) { result[i] = '\0'; i--; diff --git a/tests/unit/core/test-core-calc.cpp b/tests/unit/core/test-core-calc.cpp index 53f5beaca..d50a43aa9 100644 --- a/tests/unit/core/test-core-calc.cpp +++ b/tests/unit/core/test-core-calc.cpp @@ -24,8 +24,21 @@ extern "C" { #include "src/core/wee-calc.h" + +extern int calc_sanitize_decimal_number (char *string); +extern void calc_format_result (double value, char *result, int max_size); } +#define WEE_CHECK_SANITIZE_DECIMAL_NUMBER(__result, __result_string, \ + __number) \ + snprintf (str_number, sizeof (str_number), "%s", __number); \ + LONGS_EQUAL(__result, calc_sanitize_decimal_number (str_number)); \ + STRCMP_EQUAL(__result_string, str_number); + +#define WEE_CHECK_FORMAT_RESULT(__result, __value) \ + calc_format_result (__value, str_result, sizeof (str_result)); \ + STRCMP_EQUAL(__result, str_result); + #define WEE_CHECK_CALC(__result, __expr) \ value = calc_expression (__expr); \ STRCMP_EQUAL(__result, value); \ @@ -35,6 +48,53 @@ TEST_GROUP(CoreCalc) { }; +/* + * Tests functions: + * calc_sanitize_decimal_number + */ + +TEST(CoreCalc, SanitizeDecimalNumber) +{ + char str_number[1024]; + + WEE_CHECK_SANITIZE_DECIMAL_NUMBER(0, "0", "0"); + WEE_CHECK_SANITIZE_DECIMAL_NUMBER(1, "0.0", "0.0"); + WEE_CHECK_SANITIZE_DECIMAL_NUMBER(1, "0.0", "0,0"); + WEE_CHECK_SANITIZE_DECIMAL_NUMBER(1, "1.23", "1.23"); + WEE_CHECK_SANITIZE_DECIMAL_NUMBER(1, "1.23", "1,23"); + WEE_CHECK_SANITIZE_DECIMAL_NUMBER(1, "1234.56", "1.234,56"); + WEE_CHECK_SANITIZE_DECIMAL_NUMBER(0, "123456789", "123.456.789"); + WEE_CHECK_SANITIZE_DECIMAL_NUMBER(0, "123456789", "123,456,789"); + WEE_CHECK_SANITIZE_DECIMAL_NUMBER(1, "1234567.89", "1.234.567,89"); + WEE_CHECK_SANITIZE_DECIMAL_NUMBER(1, "1234567.89", "1,234,567.89"); + WEE_CHECK_SANITIZE_DECIMAL_NUMBER(1, "-2345.67", "-2.345,67"); +} + +/* + * Tests functions: + * calc_format_result + */ + +TEST(CoreCalc, FormatResult) +{ + char str_result[64]; + + WEE_CHECK_FORMAT_RESULT("0", 0); + WEE_CHECK_FORMAT_RESULT("0", 0.0); + WEE_CHECK_FORMAT_RESULT("0", -0.0); + WEE_CHECK_FORMAT_RESULT("12.5", 12.5); + WEE_CHECK_FORMAT_RESULT("12.005", 12.005000); + WEE_CHECK_FORMAT_RESULT("-12.005", -12.005000); + WEE_CHECK_FORMAT_RESULT("0.0000000001", 0.0000000001); + WEE_CHECK_FORMAT_RESULT("0", 0.00000000001); + WEE_CHECK_FORMAT_RESULT("123456789012345", 123456789012345); + + setlocale (LC_ALL, "fr_FR.UTF-8"); + WEE_CHECK_FORMAT_RESULT("12.5", 12.5); + WEE_CHECK_FORMAT_RESULT("-12.5", -12.5); + setlocale (LC_ALL, ""); +} + /* * Tests functions: * calc_expression @@ -146,4 +206,10 @@ TEST(CoreCalc, Expression) WEE_CHECK_CALC("21", "(5+2)*3"); WEE_CHECK_CALC("3.15", "(1.5+2)*(1.8/2)"); WEE_CHECK_CALC("-1.26", "(1.5+2)*(1.8/(2-7))"); + + /* with French locale: the result must always have "." instead of "," */ + setlocale (LC_ALL, "fr_FR.UTF-8"); + WEE_CHECK_CALC("12.5", "10.5+2"); + WEE_CHECK_CALC("-12.5", "-10.5-2"); + setlocale (LC_ALL, ""); }