[PATCH v2 1/4] msvcrt: Make locale and thread helper functions CDECL.
This prevent callers from having to save AVX registers to the stack. It is for instance the case in MSVCRT__towlower_l, which is called on every character by MSVCRT__wcsicmp_l. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/msvcrt/locale.c | 4 ++-- dlls/msvcrt/msvcrt.h | 6 +++--- dlls/msvcrt/thread.c | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/dlls/msvcrt/locale.c b/dlls/msvcrt/locale.c index 7562f70eb0b..f25c1228d2f 100644 --- a/dlls/msvcrt/locale.c +++ b/dlls/msvcrt/locale.c @@ -452,7 +452,7 @@ static inline void swap_pointers(void **p1, void **p2) { } /* INTERNAL: returns pthreadlocinfo struct */ -MSVCRT_pthreadlocinfo get_locinfo(void) { +MSVCRT_pthreadlocinfo CDECL get_locinfo(void) { thread_data_t *data = msvcrt_get_thread_data(); if(!data || !data->have_locale) @@ -462,7 +462,7 @@ MSVCRT_pthreadlocinfo get_locinfo(void) { } /* INTERNAL: returns pthreadlocinfo struct */ -MSVCRT_pthreadmbcinfo get_mbcinfo(void) { +MSVCRT_pthreadmbcinfo CDECL get_mbcinfo(void) { thread_data_t *data = msvcrt_get_thread_data(); if(!data || !data->have_locale) diff --git a/dlls/msvcrt/msvcrt.h b/dlls/msvcrt/msvcrt.h index 4f79778f7cc..f73117ae5e2 100644 --- a/dlls/msvcrt/msvcrt.h +++ b/dlls/msvcrt/msvcrt.h @@ -308,7 +308,7 @@ struct __thread_data { typedef struct __thread_data thread_data_t; -extern thread_data_t *msvcrt_get_thread_data(void) DECLSPEC_HIDDEN; +extern thread_data_t *CDECL msvcrt_get_thread_data(void) DECLSPEC_HIDDEN; LCID MSVCRT_locale_to_LCID(const char*, unsigned short*, BOOL*) DECLSPEC_HIDDEN; extern MSVCRT__locale_t MSVCRT_locale DECLSPEC_HIDDEN; @@ -1128,8 +1128,8 @@ int __cdecl MSVCRT__set_printf_count_output(int); #define MSVCRT__DISABLE_PER_THREAD_LOCALE 2 extern MSVCRT__locale_t MSVCRT_locale; -MSVCRT_pthreadlocinfo get_locinfo(void) DECLSPEC_HIDDEN; -MSVCRT_pthreadmbcinfo get_mbcinfo(void) DECLSPEC_HIDDEN; +MSVCRT_pthreadlocinfo CDECL get_locinfo(void) DECLSPEC_HIDDEN; +MSVCRT_pthreadmbcinfo CDECL get_mbcinfo(void) DECLSPEC_HIDDEN; void __cdecl MSVCRT__free_locale(MSVCRT__locale_t); void free_locinfo(MSVCRT_pthreadlocinfo) DECLSPEC_HIDDEN; void free_mbcinfo(MSVCRT_pthreadmbcinfo) DECLSPEC_HIDDEN; diff --git a/dlls/msvcrt/thread.c b/dlls/msvcrt/thread.c index af31534adbd..11573892e95 100644 --- a/dlls/msvcrt/thread.c +++ b/dlls/msvcrt/thread.c @@ -35,7 +35,7 @@ typedef struct { * * Return the thread local storage structure. */ -thread_data_t *msvcrt_get_thread_data(void) +thread_data_t *CDECL msvcrt_get_thread_data(void) { thread_data_t *ptr; DWORD err = GetLastError(); /* need to preserve last error */ -- 2.29.2
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/msvcrt/locale.c | 32 ++++++++++++++++++++++++++++++-- dlls/msvcrt/msvcrt.h | 2 ++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/dlls/msvcrt/locale.c b/dlls/msvcrt/locale.c index f25c1228d2f..0b423387368 100644 --- a/dlls/msvcrt/locale.c +++ b/dlls/msvcrt/locale.c @@ -953,6 +953,34 @@ void free_mbcinfo(MSVCRT_pthreadmbcinfo mbcinfo) MSVCRT_free(mbcinfo); } +MSVCRT__locale_t CDECL get_current_locale_noalloc(MSVCRT__locale_t locale) +{ + thread_data_t *data = msvcrt_get_thread_data(); + + if(!locale) + return NULL; + + if(!data || !data->have_locale) + { + locale->locinfo = MSVCRT_locale->locinfo; + locale->mbcinfo = MSVCRT_locale->mbcinfo; + /* FIXME: update locinfo/mbcinfo refcount */ + } + else + { + locale->locinfo = data->locinfo; + locale->mbcinfo = data->mbcinfo; + } + return locale; +} + +void CDECL free_locale_noalloc(MSVCRT__locale_t locale) +{ + if (!locale) + return; + /* FIXME: update locinfo/mbcinfo refcount */ +} + /********************************************************************* * _get_current_locale (MSVCRT.@) */ @@ -962,8 +990,7 @@ MSVCRT__locale_t CDECL MSVCRT__get_current_locale(void) if(!loc) return NULL; - loc->locinfo = get_locinfo(); - loc->mbcinfo = get_mbcinfo(); + get_current_locale_noalloc(loc); InterlockedIncrement(&loc->locinfo->refcount); InterlockedIncrement(&loc->mbcinfo->refcount); return loc; @@ -977,6 +1004,7 @@ void CDECL MSVCRT__free_locale(MSVCRT__locale_t locale) if (!locale) return; + free_locale_noalloc(locale); free_locinfo(locale->locinfo); free_mbcinfo(locale->mbcinfo); MSVCRT_free(locale); diff --git a/dlls/msvcrt/msvcrt.h b/dlls/msvcrt/msvcrt.h index f73117ae5e2..07a2f3d33e8 100644 --- a/dlls/msvcrt/msvcrt.h +++ b/dlls/msvcrt/msvcrt.h @@ -1128,6 +1128,8 @@ int __cdecl MSVCRT__set_printf_count_output(int); #define MSVCRT__DISABLE_PER_THREAD_LOCALE 2 extern MSVCRT__locale_t MSVCRT_locale; +MSVCRT__locale_t CDECL get_current_locale_noalloc(MSVCRT__locale_t locale) DECLSPEC_HIDDEN; +void CDECL free_locale_noalloc(MSVCRT__locale_t locale) DECLSPEC_HIDDEN; MSVCRT_pthreadlocinfo CDECL get_locinfo(void) DECLSPEC_HIDDEN; MSVCRT_pthreadmbcinfo CDECL get_mbcinfo(void) DECLSPEC_HIDDEN; void __cdecl MSVCRT__free_locale(MSVCRT__locale_t); -- 2.29.2
When not provided, instead of calling get_locinfo on every character. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/msvcrt/wcs.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/dlls/msvcrt/wcs.c b/dlls/msvcrt/wcs.c index 858ecbd7ed0..793c37a343f 100644 --- a/dlls/msvcrt/wcs.c +++ b/dlls/msvcrt/wcs.c @@ -119,16 +119,22 @@ int CDECL MSVCRT_towlower(MSVCRT_wint_t c) INT CDECL MSVCRT__wcsicmp_l(const MSVCRT_wchar_t *str1, const MSVCRT_wchar_t *str2, MSVCRT__locale_t locale) { + MSVCRT__locale_tstruct tmp = {}; MSVCRT_wchar_t c1, c2; if(!MSVCRT_CHECK_PMT(str1 != NULL) || !MSVCRT_CHECK_PMT(str2 != NULL)) return MSVCRT__NLSCMPERROR; + if(!locale) + locale = get_current_locale_noalloc(&tmp); + do { c1 = MSVCRT__towlower_l(*str1++, locale); c2 = MSVCRT__towlower_l(*str2++, locale); } while(c1 && (c1 == c2)); + + free_locale_noalloc(&tmp); return c1 - c2; } @@ -146,6 +152,7 @@ INT CDECL MSVCRT__wcsicmp( const MSVCRT_wchar_t* str1, const MSVCRT_wchar_t* str INT CDECL MSVCRT__wcsnicmp_l(const MSVCRT_wchar_t *str1, const MSVCRT_wchar_t *str2, MSVCRT_size_t n, MSVCRT__locale_t locale) { + MSVCRT__locale_tstruct tmp = {}; MSVCRT_wchar_t c1, c2; if (!n) @@ -154,11 +161,16 @@ INT CDECL MSVCRT__wcsnicmp_l(const MSVCRT_wchar_t *str1, const MSVCRT_wchar_t *s if(!MSVCRT_CHECK_PMT(str1 != NULL) || !MSVCRT_CHECK_PMT(str2 != NULL)) return MSVCRT__NLSCMPERROR; + if(!locale) + locale = get_current_locale_noalloc(&tmp); + do { c1 = MSVCRT__towlower_l(*str1++, locale); c2 = MSVCRT__towlower_l(*str2++, locale); } while(--n && c1 && (c1 == c2)); + + free_locale_noalloc(&tmp); return c1 - c2; } @@ -397,6 +409,7 @@ MSVCRT_wchar_t* CDECL MSVCRT__wcsupr( MSVCRT_wchar_t *str ) */ int CDECL MSVCRT__wcslwr_s_l( MSVCRT_wchar_t* str, MSVCRT_size_t n, MSVCRT__locale_t locale ) { + MSVCRT__locale_tstruct tmp = {}; MSVCRT_wchar_t* ptr = str; if (!str || !n) @@ -406,6 +419,9 @@ int CDECL MSVCRT__wcslwr_s_l( MSVCRT_wchar_t* str, MSVCRT_size_t n, MSVCRT__loca return MSVCRT_EINVAL; } + if(!locale) + locale = get_current_locale_noalloc(&tmp); + while (n--) { if (!*ptr) return 0; @@ -413,6 +429,8 @@ int CDECL MSVCRT__wcslwr_s_l( MSVCRT_wchar_t* str, MSVCRT_size_t n, MSVCRT__loca ptr++; } + free_locale_noalloc(&tmp); + /* MSDN claims that the function should return and set errno to * ERANGE, which doesn't seem to be true based on the tests. */ *str = '\0'; -- 2.29.2
On 11/9/20 3:32 PM, Rémi Bernon wrote:
@@ -406,6 +419,9 @@ int CDECL MSVCRT__wcslwr_s_l( MSVCRT_wchar_t* str, MSVCRT_size_t n, MSVCRT__loca return MSVCRT_EINVAL; }
+ if(!locale) + locale = get_current_locale_noalloc(&tmp); + while (n--) { if (!*ptr) return 0; Oops, I missed that return 0. sorry I'll send another version. -- Rémi Bernon <rbernon(a)codeweavers.com>
When not provided, instead of calling get_locinfo on every character. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/msvcrt/wcs.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/dlls/msvcrt/wcs.c b/dlls/msvcrt/wcs.c index 793c37a343f..41c70fdad12 100644 --- a/dlls/msvcrt/wcs.c +++ b/dlls/msvcrt/wcs.c @@ -355,6 +355,7 @@ MSVCRT_wchar_t* CDECL MSVCRT__wcsset( MSVCRT_wchar_t* str, MSVCRT_wchar_t c ) int CDECL MSVCRT__wcsupr_s_l( MSVCRT_wchar_t* str, MSVCRT_size_t n, MSVCRT__locale_t locale ) { + MSVCRT__locale_tstruct tmp = {}; MSVCRT_wchar_t* ptr = str; if (!str || !n) @@ -364,13 +365,19 @@ int CDECL MSVCRT__wcsupr_s_l( MSVCRT_wchar_t* str, MSVCRT_size_t n, return MSVCRT_EINVAL; } + if(!locale) + locale = get_current_locale_noalloc(&tmp); + while (n--) { - if (!*ptr) return 0; + if (!*ptr) break; *ptr = MSVCRT__towupper_l(*ptr, locale); ptr++; } + free_locale_noalloc(&tmp); + if (!*ptr) return 0; + /* MSDN claims that the function should return and set errno to * ERANGE, which doesn't seem to be true based on the tests. */ *str = '\0'; -- 2.29.2
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=81614 Your paranoid android. === debiant (32 bit report) === msvcrt: string.c:1264: Test failed: Expected _wcsupr_s to fail with EINVAL, got 0 string.c:1265: Test failed: Expected errno to be EINVAL, got 9 string.c:1266: Test failed: Expected the first buffer character to be null string.c:1286: Test failed: Expected _wcsupr_s to fail with EINVAL, got 0 string.c:1287: Test failed: Expected errno to be EINVAL, got 9 string.c:1288: Test failed: Expected the first buffer character to be null === debiant (32 bit Chinese:China report) === msvcrt: string.c:1264: Test failed: Expected _wcsupr_s to fail with EINVAL, got 0 string.c:1265: Test failed: Expected errno to be EINVAL, got 9 string.c:1266: Test failed: Expected the first buffer character to be null string.c:1286: Test failed: Expected _wcsupr_s to fail with EINVAL, got 0 string.c:1287: Test failed: Expected errno to be EINVAL, got 9 string.c:1288: Test failed: Expected the first buffer character to be null === debiant (32 bit WoW report) === msvcrt: string.c:1264: Test failed: Expected _wcsupr_s to fail with EINVAL, got 0 string.c:1265: Test failed: Expected errno to be EINVAL, got 9 string.c:1266: Test failed: Expected the first buffer character to be null string.c:1286: Test failed: Expected _wcsupr_s to fail with EINVAL, got 0 string.c:1287: Test failed: Expected errno to be EINVAL, got 9 string.c:1288: Test failed: Expected the first buffer character to be null === debiant (64 bit WoW report) === msvcrt: string.c:1264: Test failed: Expected _wcsupr_s to fail with EINVAL, got 0 string.c:1265: Test failed: Expected errno to be EINVAL, got 9 string.c:1266: Test failed: Expected the first buffer character to be null string.c:1286: Test failed: Expected _wcsupr_s to fail with EINVAL, got 0 string.c:1287: Test failed: Expected errno to be EINVAL, got 9 string.c:1288: Test failed: Expected the first buffer character to be null
participants (2)
-
Marvin -
Rémi Bernon