[PATCH v2 0/3] MR3396: msvcrt: Further locale cleanups.

[Victor Hermann Chiletto (＠vitorhnn)]

Fixes a memory leak and cleans up some other parts of MR !3050.

--
  v2: msvcrt: Remove uses of wcsncpy from locale_to_sname.

https://gitlab.winehq.org/wine/wine/-/merge_requests/3396

[PATCH v2 1/3] msvcrt: Remove unused struct locale_search_t member.

[Victor Chiletto]

From: Victor Chiletto

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

diff --git a/dlls/msvcrt/locale.c b/dlls/msvcrt/locale.c
index 153c4fb1c7d..b14aa73af05 100644
--- a/dlls/msvcrt/locale.c
+++ b/dlls/msvcrt/locale.c
@@ -244,7 +244,6 @@ typedef struct {
   WCHAR search_language[MAX_ELEM_LEN];
   WCHAR search_country[MAX_ELEM_LEN];
   WCHAR found_lang_sname[LOCALE_NAME_MAX_LENGTH];
-  DWORD found_codepage;
   unsigned int match_flags;
   BOOL allow_sname;
 } locale_search_t;

[PATCH v2 2/3] msvcrt: Fix memory leak in create_locinfo.

[Victor Chiletto]

From: Victor Chiletto

---
 dlls/msvcrt/locale.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/dlls/msvcrt/locale.c b/dlls/msvcrt/locale.c
index b14aa73af05..a24c0c16a48 100644
--- a/dlls/msvcrt/locale.c
+++ b/dlls/msvcrt/locale.c
@@ -1959,6 +1959,9 @@ static pthreadlocinfo create_locinfo(int category,
         InterlockedIncrement(&locinfo->lc_time_curr->refcount);
     }

+    for (i = 0; i < LC_MAX; i++)
+        free(locale_sname[i]);
+
     return locinfo;

fail:

[PATCH v2 3/3] msvcrt: Remove uses of wcsncpy from locale_to_sname.

[Victor Chiletto]

From: Victor Chiletto

---
 dlls/msvcrt/locale.c | 16 ++++++++--------
 dlls/msvcrt/mbcs.c   |  2 +-
 dlls/msvcrt/msvcrt.h |  2 +-
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/dlls/msvcrt/locale.c b/dlls/msvcrt/locale.c
index a24c0c16a48..282711062da 100644
--- a/dlls/msvcrt/locale.c
+++ b/dlls/msvcrt/locale.c
@@ -330,7 +330,7 @@ find_best_locale_proc( WCHAR *name, DWORD locale_flags, LPARAM lParam )
 }

/* Internal: Find the sname for a locale specification */
-BOOL locale_to_sname(const char *locale, unsigned short *codepage, BOOL *sname_match, WCHAR *sname, int sname_size)
+BOOL locale_to_sname(const char *locale, unsigned short *codepage, BOOL *sname_match, WCHAR *sname)
 {
     thread_data_t *data = msvcrt_get_thread_data();
     const char *cp, *region;
@@ -342,7 +342,7 @@ BOOL locale_to_sname(const char *locale, unsigned short *codepage, BOOL *sname_m
             *codepage = data->cached_cp;
         if (sname_match)
             *sname_match = data->cached_sname_match;
-        wcsncpy(sname, data->cached_sname, sname_size);
+        wcscpy(sname, data->cached_sname);
         return TRUE;
     }

@@ -350,7 +350,7 @@ BOOL locale_to_sname(const char *locale, unsigned short *codepage, BOOL *sname_m
     region = strchr(locale, '_');

if(!locale[0] || (cp == locale && !region)) {
-        GetUserDefaultLocaleName(sname, sname_size);
+        GetUserDefaultLocaleName(sname, LOCALE_NAME_MAX_LENGTH);
     } else {
         char search_language_buf[MAX_ELEM_LEN] = { 0 }, search_country_buf[MAX_ELEM_LEN] = { 0 };
         locale_search_t search;
@@ -388,7 +388,7 @@ BOOL locale_to_sname(const char *locale, unsigned short *codepage, BOOL *sname_m
         if (search.allow_sname && IsValidLocaleName(search.search_language))
         {
             search.match_flags = FOUND_SNAME;
-            wcsncpy(sname, search.search_language, sname_size);
+            wcscpy(sname, search.search_language);
         }
         else
         {
@@ -404,7 +404,7 @@ BOOL locale_to_sname(const char *locale, unsigned short *codepage, BOOL *sname_m
             if (search.search_country[0] && !(search.match_flags & FOUND_COUNTRY))
                 return FALSE;

-            wcsncpy(sname, search.found_lang_sname, sname_size);
+            wcscpy(sname, search.found_lang_sname);
         }

is_sname = !remapped && (search.match_flags & FOUND_SNAME) != 0;
@@ -1333,9 +1333,9 @@ static pthreadlocinfo create_locinfo(int category,
                 if(p) {
                     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_found = locale_to_sname(buf, &cp[i], &sname_match, wbuf);
                 } else {
-                    locale_found = locale_to_sname(locale, &cp[i], &sname_match, wbuf, LOCALE_NAME_MAX_LENGTH);
+                    locale_found = locale_to_sname(locale, &cp[i], &sname_match, wbuf);
                 }

if(!locale_found || !(locale_sname[i] = wcsdup(wbuf)))
@@ -1352,7 +1352,7 @@ static pthreadlocinfo create_locinfo(int category,
             locale = p+1;
         }
     } else {
-        BOOL locale_found = locale_to_sname(locale, &cp[0], &sname_match, wbuf, LOCALE_NAME_MAX_LENGTH);
+        BOOL locale_found = locale_to_sname(locale, &cp[0], &sname_match, wbuf);

if(!locale_found)
             return NULL;
diff --git a/dlls/msvcrt/mbcs.c b/dlls/msvcrt/mbcs.c
index 8598bceb029..c8390288d4a 100644
--- a/dlls/msvcrt/mbcs.c
+++ b/dlls/msvcrt/mbcs.c
@@ -254,7 +254,7 @@ threadmbcinfo* create_mbcinfo(int cp, LCID lcid, threadmbcinfo *old_mbcinfo)
   if(lcid == -1) {
     WCHAR wbuf[LOCALE_NAME_MAX_LENGTH];
     sprintf(bufA, ".%d", newcp);
-    mbcinfo->mblcid = locale_to_sname(bufA, NULL, NULL, wbuf, LOCALE_NAME_MAX_LENGTH) ? LocaleNameToLCID(wbuf, LOCALE_ALLOW_NEUTRAL_NAMES) : -1;
+    mbcinfo->mblcid = locale_to_sname(bufA, NULL, NULL, wbuf) ? LocaleNameToLCID(wbuf, LOCALE_ALLOW_NEUTRAL_NAMES) : -1;
   } else {
     mbcinfo->mblcid = lcid;
   }
diff --git a/dlls/msvcrt/msvcrt.h b/dlls/msvcrt/msvcrt.h
index 155db86bf02..9f253d45579 100644
--- a/dlls/msvcrt/msvcrt.h
+++ b/dlls/msvcrt/msvcrt.h
@@ -186,7 +186,7 @@ typedef struct __thread_data thread_data_t;

extern thread_data_t *CDECL msvcrt_get_thread_data(void) DECLSPEC_HIDDEN;

-BOOL locale_to_sname(const char*, unsigned short*, BOOL*, WCHAR*, int) DECLSPEC_HIDDEN;
+BOOL locale_to_sname(const char*, unsigned short*, BOOL*, WCHAR*) DECLSPEC_HIDDEN;
 extern _locale_t MSVCRT_locale DECLSPEC_HIDDEN;
 extern __lc_time_data cloc_time_data DECLSPEC_HIDDEN;
 extern unsigned int MSVCRT___lc_codepage;

[PATCH v2 3/3] msvcrt: Remove uses of wcsncpy from locale_to_sname.

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=135263

Your paranoid android.

=== debian11b (64 bit WoW report) ===

shell32:
progman_dde.c:368: Test failed: window not created

[Victor Hermann Chiletto (＠vitorhnn)]

On Tue Jul 25 11:30:17 2023 +0000, Piotr Caban wrote:
> Isn't it better to remove sname_size parameter and assume that passed
> buffer length is LOCALE_NAME_MAX_LENGTH? The size checks should not be
> needed since locale name always fits into LOCALE_NAME_MAX_LENGTH buffer.

Done. Should I add a comment to the top of locale_to_sname indicating that it expects the buffer to be at least LOCALE_NAME_MAX_LENGTH?

[Victor Hermann Chiletto (＠vitorhnn)]

v2: Removed checks on the sname buffer size in locale_to_sname.

[PATCH v2 0/3] MR3396: msvcrt: Further locale cleanups. - approved

[Piotr Caban (＠piotr)]

This merge request was approved by Piotr Caban.

[Piotr Caban (＠piotr)]

On Wed Jul 26 06:26:25 2023 +0000, Victor Hermann Chiletto wrote:
> Done. Should I add a comment to the top of locale_to_sname indicating
> that it expects the buffer to be at least LOCALE_NAME_MAX_LENGTH?

Yes, please add the comment.