[PATCH v5 0/1] MR1937: kernelbase: Fix grouping repeat for number formatting.

[Gabriel Ivăncescu]

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

--
  v5: kernelbase: Fix grouping repeat for number formatting.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1937

[PATCH v5 1/1] kernelbase: Fix grouping repeat for number formatting.

[Gabriel Ivăncescu]

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 | 73 ++++++++++++++++++++++++++++++++++++
 dlls/kernelbase/locale.c     | 51 ++++++++++++++++++++++---
 2 files changed, 119 insertions(+), 5 deletions(-)

diff --git a/dlls/kernel32/tests/locale.c b/dlls/kernel32/tests/locale.c
index 886240cc22c..1f52266b6a1 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;
 
@@ -1522,6 +1523,22 @@ static void test_GetNumberFormatA(void)
   ret = GetNumberFormatA(lcid, 0, "123456789", &format, buffer, ARRAY_SIZE(buffer));
   expect_str(ret, buffer, "1234,567,89.0");
 
+  format.Grouping = 203;
+  ret = GetNumberFormatA(lcid, 0, "123456789", &format, buffer, ARRAY_SIZE(buffer));
+  expect_str(ret, buffer, "1,234,567,,89.0");
+
+  format.Grouping = 2030;
+  ret = GetNumberFormatA(lcid, 0, "123456789", &format, buffer, ARRAY_SIZE(buffer));
+  expect_str(ret, buffer, "1234,567,,89.0");
+
+  format.Grouping = 2003;
+  ret = GetNumberFormatA(lcid, 0, "123456789", &format, buffer, ARRAY_SIZE(buffer));
+  expect_str(ret, buffer, "1,234,567,,,89.0");
+
+  format.Grouping = 1200;
+  ret = GetNumberFormatA(lcid, 0, "123456789", &format, buffer, ARRAY_SIZE(buffer));
+  expect_str(ret, buffer, "123456,,78,9.0");
+
   /* Grouping of a negative number */
   format.NegativeOrder = NEG_LEFT;
   format.Grouping = 3;
@@ -1563,6 +1580,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..42fa6cb371d 100644
--- a/dlls/kernelbase/locale.c
+++ b/dlls/kernelbase/locale.c
@@ -7245,8 +7245,20 @@ 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. */
+    if (last_digit == 0)
+    {
+        grouping /= 10;
+
+        /* Special case: two or more trailing zeros result in zero-sided groupings, with no repeats */
+        if (grouping % 10 == 0)
+            last_digit = ~0;
+    }
+
     while (grouping)
     {
         *p++ = '0' + grouping % 10;
@@ -7257,6 +7269,12 @@ static void grouping_to_string( UINT grouping, WCHAR *buffer )
         *buffer++ = *(--p);
         if (p > tmp) *buffer++ = ';';
     }
+    if (last_digit != 0)
+    {
+        *buffer++ = ';';
+        if (last_digit != ~0)
+            *buffer++ = '0';
+    }
     *buffer = 0;
 }
 
@@ -7272,9 +7290,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 +7345,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 +7387,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 +7397,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