[PATCH v2 0/3] MR3396: msvcrt: Further locale cleanups.
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
From: Victor Chiletto <vchiletto(a)codeweavers.com> --- 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; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3396
From: Victor Chiletto <vchiletto(a)codeweavers.com> --- 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: -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3396
From: Victor Chiletto <vchiletto(a)codeweavers.com> --- 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; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3396
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
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?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3396#note_40252
v2: Removed checks on the sname buffer size in locale_to_sname. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3396#note_40253
This merge request was approved by Piotr Caban. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3396
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.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3396#note_40257
participants (4)
-
Marvin -
Piotr Caban (＠piotr) -
Victor Chiletto
-
Victor Hermann Chiletto (＠vitorhnn)