[PATCH v3 0/1] MR1937: kernelbase: Fix grouping repeat for number formatting.
LOCALE_SGROUPING's string and the `Grouping` field in NUMBERFMTW are confusingly different. The former, which is what is fixed here, treats a '0' as a repeat of the previous grouping. But a 0 at the end of the `Grouping` field prevents it from repeating (it repeats by default otherwise) so it's the opposite. Note that without a '0' in the LOCALE_SGROUPING string, it shouldn't even repeat in the first place. This fixes the typical "3;0" default grouping, for example. See: https://learn.microsoft.com/en-us/windows/win32/intl/locale-sgrouping -- v3: kernelbase: Fix grouping repeat for number formatting. https://gitlab.winehq.org/wine/wine/-/merge_requests/1937
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Fixes a regression introduced by 56099a31242dcc522fb94643e70553e152c522ec. LOCALE_SGROUPING's string and the `Grouping` field in NUMBERFMTW are confusingly different. The former, which is what is fixed here, treats a '0' as a repeat of the previous grouping. But a 0 at the end of the `Grouping` field prevents it from repeating (it repeats by default otherwise) so it's the opposite. Note that without a '0' in the LOCALE_SGROUPING string, it shouldn't even repeat in the first place. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/kernel32/tests/locale.c | 57 ++++++++++++++++++++++++++++++++++++ dlls/kernelbase/locale.c | 45 ++++++++++++++++++++++++---- 2 files changed, 96 insertions(+), 6 deletions(-) diff --git a/dlls/kernel32/tests/locale.c b/dlls/kernel32/tests/locale.c index 886240cc22c..9255c0d7776 100644 --- a/dlls/kernel32/tests/locale.c +++ b/dlls/kernel32/tests/locale.c @@ -1404,6 +1404,7 @@ static void test_GetNumberFormatA(void) static char szComma[] = { ',', '\0' }; int ret; LCID lcid = MAKELCID(MAKELANGID(LANG_ENGLISH, SUBLANG_ENGLISH_US), SORT_DEFAULT); + WCHAR grouping[32], t1000[8], dec[8], frac[8], lzero[8]; char buffer[BUFFER_SIZE]; NUMBERFMTA format; @@ -1563,6 +1564,62 @@ static void test_GetNumberFormatA(void) ret = GetNumberFormatA(lcid, NUO, "-12345", NULL, buffer, ARRAY_SIZE(buffer)); expect_str(ret, buffer, "-12\xa0\x33\x34\x35,00"); /* Non breaking space */ } + + /* Test the actual LOCALE_SGROUPING string, the rules for repeats are opposite */ + if (GetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_SGROUPING, grouping, ARRAY_SIZE(grouping)) && + GetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_STHOUSAND, t1000, ARRAY_SIZE(t1000)) && + GetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_SDECIMAL, dec, ARRAY_SIZE(dec)) && + GetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_IDIGITS, frac, ARRAY_SIZE(frac)) && + GetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_ILZERO, lzero, ARRAY_SIZE(lzero))) + { + static const struct + { + const char *grouping; + const char *expected; + } tests[] = { + { "3;0", "1,234,567,890.54321" }, + { "2;3", "12345,678,90.54321" }, + { "1", "123456789,0.54321" }, + { "1;0", "1,2,3,4,5,6,7,8,9,0.54321" }, + { "1;0;3", "123456,789,,0.54321" }, + { "0", "1234567890.54321" }, + { "0;0", "1234567890.54321" }, + { "0;1", "123456789,0.54321" }, + { "0;1;0", "1,2,3,4,5,6,7,8,9,0.54321" }, + { "1;3;2", "1234,56,789,0.54321" }, + { "1;3;2;0", "12,34,56,789,0.54321" }, + { "3;1;1;2;0", "1,23,45,6,7,890.54321" }, + { "6;1", "123,4,567890.54321" }, + }; + unsigned i; + + SetLocaleInfoA(LOCALE_USER_DEFAULT, LOCALE_STHOUSAND, ","); + SetLocaleInfoA(LOCALE_USER_DEFAULT, LOCALE_SDECIMAL, "."); + SetLocaleInfoA(LOCALE_USER_DEFAULT, LOCALE_IDIGITS, "5"); + SetLocaleInfoA(LOCALE_USER_DEFAULT, LOCALE_ILZERO, "0"); + + for (i = 0; i < ARRAY_SIZE(tests); i++) + { + SetLocaleInfoA(LOCALE_USER_DEFAULT, LOCALE_SGROUPING, tests[i].grouping); + SetLastError(0xdeadbeef); + ret = GetNumberFormatA(LOCALE_USER_DEFAULT, 0, "1234567890.54321", NULL, buffer, ARRAY_SIZE(buffer)); + if (ret) + { + ok(GetLastError() == 0xdeadbeef, "[%u] unexpected error %lu\n", i, GetLastError()); + ok(ret == strlen(tests[i].expected) + 1, "[%u] unexpected ret %d\n", i, ret); + ok(!strcmp(buffer, tests[i].expected), "[%u] unexpected string %s\n", i, buffer); + } + else + ok(0, "[%u] expected success, got error %ld\n", i, GetLastError()); + } + + /* Restore */ + ok(SetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_SGROUPING, grouping), "Restoring SGROUPING failed\n"); + ok(SetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_STHOUSAND, t1000), "Restoring STHOUSAND failed\n"); + ok(SetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_SDECIMAL, dec), "Restoring SDECIMAL failed\n"); + ok(SetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_IDIGITS, frac), "Restoring IDIGITS failed\n"); + ok(SetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_ILZERO, lzero), "Restoring ILZERO failed\n"); + } } static void test_GetNumberFormatEx(void) diff --git a/dlls/kernelbase/locale.c b/dlls/kernelbase/locale.c index 1e96e49622e..29467e8a3e6 100644 --- a/dlls/kernelbase/locale.c +++ b/dlls/kernelbase/locale.c @@ -7245,11 +7245,16 @@ BOOL WINAPI SetUserGeoName(PWSTR geo_name) static void grouping_to_string( UINT grouping, WCHAR *buffer ) { + UINT last_digit = grouping % 10; WCHAR tmp[10], *p = tmp; + /* The string is confusingly different when it comes to repetitions (trailing zeros). For a string, + * a 0 signals that the format needs to be repeated, which is the opposite of the grouping integer. */ while (grouping) { - *p++ = '0' + grouping % 10; + UINT digit = grouping % 10; + if (digit) + *p++ = '0' + digit; grouping /= 10; } while (p > tmp) @@ -7257,6 +7262,11 @@ static void grouping_to_string( UINT grouping, WCHAR *buffer ) *buffer++ = *(--p); if (p > tmp) *buffer++ = ';'; } + if (last_digit != 0) + { + *buffer++ = ';'; + *buffer++ = '0'; + } *buffer = 0; } @@ -7272,9 +7282,9 @@ static WCHAR *prepend_str( WCHAR *end, const WCHAR *str ) static WCHAR *format_number( WCHAR *end, const WCHAR *value, const WCHAR *decimal_sep, const WCHAR *thousand_sep, const WCHAR *grouping, UINT digits, BOOL lzero ) { + BOOL round = FALSE, repeat = FALSE; + UINT i, len = 0, prev = ~0; const WCHAR *frac = NULL; - BOOL round = FALSE; - UINT i, len = 0; *(--end) = 0; @@ -7327,9 +7337,33 @@ static WCHAR *format_number( WCHAR *end, const WCHAR *value, const WCHAR *decima } if (len) lzero = FALSE; + /* leading 0s are ignored */ + while (grouping[0] == '0' && grouping[1] == ';') + grouping += 2; + while (len) { - UINT limit = *grouping == '0' ? ~0u : *grouping - '0'; + UINT limit = prev; + + if (!repeat) + { + limit = *grouping - '0'; + if (grouping[1] == ';') + { + grouping += 2; + if (limit) + prev = limit; + } + else + { + repeat = TRUE; + if (!limit) + limit = prev; + else + prev = ~0; + } + } + while (len && limit--) { WCHAR ch = value[--len]; @@ -7345,7 +7379,6 @@ static WCHAR *format_number( WCHAR *end, const WCHAR *value, const WCHAR *decima *(--end) = ch; } if (len) end = prepend_str( end, thousand_sep ); - if (grouping[1] == ';') grouping += 2; } if (round) *(--end) = '1'; else if (lzero) *(--end) = '0'; @@ -7356,7 +7389,7 @@ static WCHAR *format_number( WCHAR *end, const WCHAR *value, const WCHAR *decima static int get_number_format( const NLS_LOCALE_DATA *locale, DWORD flags, const WCHAR *value, const NUMBERFMTW *format, WCHAR *buffer, int len ) { - WCHAR *num, fmt_decimal[4], fmt_thousand[4], fmt_neg[5], grouping[20], output[256]; + WCHAR *num, fmt_decimal[4], fmt_thousand[4], fmt_neg[5], grouping[22], output[256]; const WCHAR *decimal_sep = fmt_decimal, *thousand_sep = fmt_thousand; DWORD digits, lzero, order; int ret = 0; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1937
I found an issue with the Grouping integer field, it did not handle zeros in the middle of the number (or multiple trailing ones) correctly. I've added tests and corrected it now. It has a special case for that, unfortunately, since it's how native works… -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1937#note_21153
participants (1)
-
Gabriel Ivăncescu