[PATCH v2 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

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

[PATCH v2 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.

This fixes the typical "3;0" default grouping, for example.

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 dlls/kernelbase/locale.c | 30 +++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/dlls/kernelbase/locale.c b/dlls/kernelbase/locale.c
index 1e96e49622e..cb1b460090d 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 repeat, 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 )
 {
+    UINT i, len = 0, limit = ~0, repeat = ~0;
     const WCHAR *frac = NULL;
     BOOL round = FALSE;
-    UINT i, len = 0;
 
     *(--end) = 0;
 
@@ -7327,9 +7337,10 @@ static WCHAR *format_number( WCHAR *end, const WCHAR *value, const WCHAR *decima
     }
     if (len) lzero = FALSE;
 
+    if (*grouping >= '1' && *grouping <= '9')
+        limit = repeat = *grouping - '0';
     while (len)
     {
-        UINT limit = *grouping == '0' ? ~0u : *grouping - '0';
         while (len && limit--)
         {
             WCHAR ch = value[--len];
@@ -7345,7 +7356,16 @@ 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 (*grouping == '0')
+            limit = repeat;
+        else if (grouping[1] == ';')
+        {
+            grouping += 2;
+            if (*grouping == '0')
+                limit = repeat;
+            else if (*grouping >= '1' && *grouping <= '9')
+                limit = repeat = *grouping - '0';
+        }
     }
     if (round) *(--end) = '1';
     else if (lzero) *(--end) = '0';
@@ -7356,7 +7376,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

[Alexandre Julliard (＠julliard)]

Please add a test demonstrating the broken case.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1937#note_21042

[Gabriel Ivăncescu]

On Mon Jan 16 15:59:31 2023 +0000, Alexandre Julliard wrote:
> Please add a test demonstrating the broken case.
> Sure, I just don't know how to do it in a proper way.

Is there a way to change the locale string (i.e. "3;0" for default
English locale) without messing with the user's locale configuration in
the registry? I'm talking about:

[HKCU\Control Panel\International]
"sGrouping"="3;0"

Or do you think I should parse that value myself and then use a similar
algorithm to compare the results?

Or maybe there's an API I'm missing…

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1937#note_21043

[Alexandre Julliard (＠julliard)]

On Mon Jan 16 15:59:30 2023 +0000, Gabriel Ivăncescu wrote:
> Sure, I just don't know how to do it in a proper way.
> Is there a way to change the locale string (i.e. "3;0" for default
> English locale) without messing with the user's locale configuration in
> the registry? I'm talking about:
> [HKCU\Control Panel\International]
> "sGrouping"="3;0"
> Or do you think I should parse that value myself and then use a similar
> algorithm to compare the results?
> Or maybe there's an API I'm missing…
> Didn't you say that the default "3;0" grouping is broken already?

In any case you can use SetLocaleInfo to change it, check how oleaut32:vartest does it.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1937#note_21056

[Gabriel Ivăncescu]

> Alexandre Julliard <https://gitlab.winehq.org/julliard> commented on a
> discussion
> <https://gitlab.winehq.org/wine/wine/-/merge_requests/1937#note_21056>:
>
> Didn't you say that the default "3;0" grouping is broken already?

Yes, but that depends on the user's locale settings. It's default for
me, not for every locale.
> In any case you can use SetLocaleInfo to change it, check how
> oleaut32:vartest does it.
>
> —
> Reply to this email directly or view it on GitLab
> <https://gitlab.winehq.org/wine/wine/-/merge_requests/1937#note_21056>.
> You're receiving this email because of your account on gitlab.winehq.org
> <https://gitlab.winehq.org>. Unsubscribe
> <https://gitlab.winehq.org/-/sent_notifications/REDACTED/unsubscribe>
> from this thread · Manage all notifications
> <https://gitlab.winehq.org/-/profile/notifications> · Help
> <https://gitlab.winehq.org/help>

Thanks, will take a look.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1937#note_21068