[PATCH 0/3] MR3375: msvcrt: setlocale fixes

[PATCH 1/3] msvcrt: Pass correct buffer to locale_to_sname helper in create_locinfo.

[Piotr Caban]

From: Piotr Caban

---
 dlls/msvcrt/locale.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/msvcrt/locale.c b/dlls/msvcrt/locale.c
index 10682990bfc..7e94d7bf2a8 100644
--- a/dlls/msvcrt/locale.c
+++ b/dlls/msvcrt/locale.c
@@ -1339,7 +1339,7 @@ static pthreadlocinfo create_locinfo(int category,
                         locale_len[i] = p-locale;
                     }
                 } else {
-                    locale_found = locale_to_sname(buf, &cp[i], &sname_match, wbuf, LOCALE_NAME_MAX_LENGTH);
+                    locale_found = locale_to_sname(locale, &cp[i], &sname_match, wbuf, LOCALE_NAME_MAX_LENGTH);
                     locale_sname[i] = wcsdup(wbuf);
                     if(sname_match) {
                         locale_name[i] = locale;

[PATCH 2/3] msvcrt: Improve locale_to_sname error handling.

[Piotr Caban]

From: Piotr Caban

---
 dlls/msvcrt/locale.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/dlls/msvcrt/locale.c b/dlls/msvcrt/locale.c
index 7e94d7bf2a8..854ec6e7158 100644
--- a/dlls/msvcrt/locale.c
+++ b/dlls/msvcrt/locale.c
@@ -1333,22 +1333,16 @@ static pthreadlocinfo create_locinfo(int category,
                     memcpy(buf, locale, p-locale);
                     buf[p-locale] = '\0';
                     locale_found = locale_to_sname(buf, &cp[i], &sname_match, wbuf, LOCALE_NAME_MAX_LENGTH);
-                    locale_sname[i] = wcsdup(wbuf);
-                    if(sname_match) {
-                        locale_name[i] = locale;
-                        locale_len[i] = p-locale;
-                    }
                 } else {
                     locale_found = locale_to_sname(locale, &cp[i], &sname_match, wbuf, LOCALE_NAME_MAX_LENGTH);
-                    locale_sname[i] = wcsdup(wbuf);
-                    if(sname_match) {
-                        locale_name[i] = locale;
-                        locale_len[i] = strlen(locale);
-                    }
                 }

-                if(!locale_found || !locale_sname[i])
+                if(!locale_found || !(locale_sname[i] = wcsdup(wbuf)))
                     goto fail;
+                if(sname_match) {
+                    locale_name[i] = locale;
+                    locale_len[i] = p ? p-locale : strlen(locale);
+                }
             }

if(!p || *(p+1)!='L' || *(p+2)!='C' || *(p+3)!='_')

[PATCH 3/3] msvcrt: Set sname_match in locale_to_sname when returning cached result.

[Piotr Caban]

From: Piotr Caban

---
 dlls/msvcrt/locale.c | 6 ++++--
 dlls/msvcrt/msvcrt.h | 1 +
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/dlls/msvcrt/locale.c b/dlls/msvcrt/locale.c
index 854ec6e7158..153c4fb1c7d 100644
--- a/dlls/msvcrt/locale.c
+++ b/dlls/msvcrt/locale.c
@@ -341,8 +341,9 @@ BOOL locale_to_sname(const char *locale, unsigned short *codepage, BOOL *sname_m
     if (!strcmp(locale, data->cached_locale)) {
         if (codepage)
             *codepage = data->cached_cp;
-        if (sname)
-            wcsncpy(sname, data->cached_sname, sname_size);
+        if (sname_match)
+            *sname_match = data->cached_sname_match;
+        wcsncpy(sname, data->cached_sname, sname_size);
         return TRUE;
     }

@@ -441,6 +442,7 @@ BOOL locale_to_sname(const char *locale, unsigned short *codepage, BOOL *sname_m
     if (strlen(locale) < sizeof(data->cached_locale)) {
         strcpy(data->cached_locale, locale);
         data->cached_cp = locale_cp;
+        data->cached_sname_match = is_sname;
         wcscpy(data->cached_sname, sname);
     }

diff --git a/dlls/msvcrt/msvcrt.h b/dlls/msvcrt/msvcrt.h
index 42c41c2200c..155db86bf02 100644
--- a/dlls/msvcrt/msvcrt.h
+++ b/dlls/msvcrt/msvcrt.h
@@ -170,6 +170,7 @@ struct __thread_data {
     int                             processing_throw;
     frame_info                     *frame_info_head;
     void                           *unk8[6];
+    BOOL                            cached_sname_match;
     WCHAR                           cached_sname[LOCALE_NAME_MAX_LENGTH];
     int                             unk9[2];
     DWORD                           cached_cp;

[Victor Hermann Chiletto (＠vitorhnn)]

Victor Hermann Chiletto (@vitorhnn) commented about dlls/msvcrt/locale.c:
>  if (!strcmp(locale, data->cached_locale)) {
>      if (codepage)
>          *codepage = data->cached_cp;
> -
>    if (sname)
> -
>        wcsncpy(sname, data->cached_sname, sname_size);
> -
>    if (sname_match)
> -
>        *sname_match = data->cached_sname_match;
> -
>    wcsncpy(sname, data->cached_sname, sname_size);

I think we should replace wcsncpy with lstrcpynW here and on lines 392 and 408.

[Victor Hermann Chiletto (＠vitorhnn)]

Seems fine, but I'm going to go over my entire MR again, considering the two bugs it has introduced so far.

[Piotr Caban (＠piotr)]

On Fri Jul 21 17:03:58 2023 +0000, Victor Hermann Chiletto wrote:
> I think we should replace wcsncpy with lstrcpynW here and on lines 392
> and 408.

I think it can go in another MR.

I don't think we should use lstrcpynW here (but we also shouldn't use wcsncpy). I think that the correct solution is to reject locale if it doesn't fit into the buffer. I don't think things will work with truncated locale name.

[Victor Hermann Chiletto (＠vitorhnn)]

On Fri Jul 21 20:25:24 2023 +0000, Victor Hermann Chiletto wrote:
> Seems fine, but I'm going to go over my entire MR again, considering the
> multiple bugs it has introduced so far.

Couldn't find anything _broken_, but there's definitely room for improvement. Working on a follow-up.