[PATCH 0/2] MR5696: kernelbase: Handle SORTHANDLE in LCMapStringEx.
this is part of a series that'll lead to a change in wcsicmp -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5696
From: Daniel Lehman <dlehman25(a)gmail.com> --- dlls/kernel32/tests/locale.c | 101 +++++++++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/dlls/kernel32/tests/locale.c b/dlls/kernel32/tests/locale.c index bebd857dfa4..bd94e10e9a3 100644 --- a/dlls/kernel32/tests/locale.c +++ b/dlls/kernel32/tests/locale.c @@ -2872,6 +2872,9 @@ static void test_LCMapStringEx(void) { int ret; WCHAR buf[256]; + LPARAM handle_en; + LPARAM handle_tr; + LPARAM handle; if (!pLCMapStringEx) { @@ -2893,12 +2896,14 @@ static void test_LCMapStringEx(void) ok(ret, "LCMapStringEx should not fail with bad locale name\n"); /* test reserved parameters */ + memset(buf, 0, sizeof(buf)); ret = pLCMapStringEx(LOCALE_NAME_USER_DEFAULT, LCMAP_LOWERCASE, upper_case, -1, buf, ARRAY_SIZE(buf), NULL, NULL, 1); ok(ret == lstrlenW(upper_case) + 1, "ret %d, error %ld, expected value %d\n", ret, GetLastError(), lstrlenW(upper_case) + 1); ok(!lstrcmpW(buf, lower_case), "string compare mismatch\n"); + memset(buf, 0, sizeof(buf)); ret = pLCMapStringEx(LOCALE_NAME_USER_DEFAULT, LCMAP_LOWERCASE, upper_case, -1, buf, ARRAY_SIZE(buf), NULL, (void*)1, 0); ok(ret == lstrlenW(upper_case) + 1, "ret %d, error %ld, expected value %d\n", @@ -2911,6 +2916,102 @@ static void test_LCMapStringEx(void) upper_case, -1, buf, ARRAY_SIZE(buf), (void*)1, NULL, 0); test_lcmapstring_unicode(LCMapStringEx_wrapper, "LCMapStringEx:"); + + /* using SORTHANDLE */ + SetLastError(0xdeadbeef); + handle = 0xdeadbeef; + ret = pLCMapStringEx(L"en-US", LCMAP_SORTHANDLE, NULL, 0, + (LPWSTR)&handle, sizeof(handle), NULL, NULL, 0); + if(broken(!ret && GetLastError() == ERROR_INVALID_PARAMETER)) { + /* win7 */ + win_skip("LPMAP_SORTHANDLE not supported\n"); + return; + } + + ok(!!handle, "error %ld\n", GetLastError()); + todo_wine + ok(handle == ret, "ret %x, handle %#Ix, error %ld\n", ret, handle, GetLastError()); + handle_en = handle; + + SetLastError(0xdeadbeef); + handle = 0xdeadbeef; + ret = pLCMapStringEx(L"tr-TR", LCMAP_SORTHANDLE, NULL, 0, + (LPWSTR)&handle, sizeof(handle), NULL, NULL, 0); + ok(!!handle, "error %ld\n", GetLastError()); + todo_wine + ok(handle == ret, "ret %x, handle %#Ix, error %ld\n", ret, handle, GetLastError()); + handle_tr = handle; + todo_wine + ok(handle_en != handle_tr, "handle_en %#Ix, handle_tr %#Ix\n", handle_en, handle_tr); + + SetLastError(0xdeadbeef); + memset(buf, 0, sizeof(buf)); + ret = pLCMapStringEx(NULL, LCMAP_LOWERCASE|LCMAP_LINGUISTIC_CASING, + L"I", -1, buf, ARRAY_SIZE(buf), NULL, NULL, handle_en); + ok(ret == 2, "ret %d, error %ld\n", ret, GetLastError()); + ok(!lstrcmpW(buf, L"i"), "string compare mismatch\n"); + + SetLastError(0xdeadbeef); + memset(buf, 0, sizeof(buf)); + ret = pLCMapStringEx(NULL, LCMAP_LOWERCASE|LCMAP_LINGUISTIC_CASING, + L"I", -1, buf, ARRAY_SIZE(buf), NULL, NULL, handle_tr); + ok(ret == 2, "ret %d, error %ld\n", ret, GetLastError()); + todo_wine + ok(!lstrcmpW(buf, L"\x0131"), "string compare mismatch\n"); + + SetLastError(0xdeadbeef); + handle = 0xdeadbeef; + ret = pLCMapStringEx(LOCALE_NAME_USER_DEFAULT, LCMAP_SORTHANDLE, NULL, 0, + (LPWSTR)&handle, sizeof(handle), NULL, NULL, 0); + todo_wine { + ok(!!ret, "error %ld\n", GetLastError()); + ok(handle == ret, "ret %x, handle %#Ix, error %ld\n", ret, handle, GetLastError()); + } + + /* bogus parms with SORTHANDLE */ + handle = 0xdeadbeef; + ret = pLCMapStringEx(L"en-US", LCMAP_SORTHANDLE, NULL, 0, + (LPWSTR)&handle, sizeof(handle), NULL, NULL, handle_tr); + ok(handle == handle_en, "handle %#Ix, handle_en %#Ix, handle_tr %#Ix\n", + handle, handle_en, handle_tr); + + SetLastError(0xdeadbeef); + handle = 0xdeadbeef; + ret = pLCMapStringEx(L"en-US", LCMAP_SORTHANDLE, L"I", -1, + (LPWSTR)&handle, sizeof(handle), NULL, NULL, 0); + todo_wine + ok(!ret && GetLastError() == ERROR_INVALID_PARAMETER, "ret %d, error %ld\n", ret, GetLastError()); + ok(handle == 0xdeadbeef, "handle %#Ix\n", handle); + + SetLastError(0xdeadbeef); + handle = 0xdeadbeef; + ret = pLCMapStringEx(L"en-US", LCMAP_SORTHANDLE, NULL, 0, (LPWSTR)&handle, 0, NULL, NULL, 0); + ok(!ret && GetLastError() == ERROR_INVALID_PARAMETER, "ret %d, error %ld\n", ret, GetLastError()); + ok(handle == 0xdeadbeef, "handle %#Ix\n", handle); + + SetLastError(0xdeadbeef); + ret = pLCMapStringEx(L"en-US", LCMAP_SORTHANDLE, NULL, 0, NULL, sizeof(handle), NULL, NULL, 0); + ok(!ret && GetLastError() == ERROR_INVALID_PARAMETER, "ret %d, error %ld\n", ret, GetLastError()); + + SetLastError(0xdeadbeef); + memset(buf, 0, sizeof(buf)); + ret = pLCMapStringEx(L"en-US", LCMAP_LOWERCASE|LCMAP_LINGUISTIC_CASING, + L"I", -1, buf, ARRAY_SIZE(buf), NULL, NULL, handle_en); + todo_wine { + ok(!ret && GetLastError() == ERROR_INVALID_PARAMETER, + "ret %d, error %ld\n", ret, GetLastError()); + ok(!buf[0], "buf %x\n", buf[0]); + } + + SetLastError(0xdeadbeef); + memset(buf, 0, sizeof(buf)); + ret = pLCMapStringEx(NULL, LCMAP_LOWERCASE|LCMAP_LINGUISTIC_CASING, + L"I", -1, buf, ARRAY_SIZE(buf), NULL, NULL, 0xdeadbeef); + todo_wine { + ok(!ret && GetLastError() == ERROR_INVALID_PARAMETER, + "ret %d, error %ld\n", ret, GetLastError()); + ok(!buf[0], "buf %x\n", buf[0]); + } } struct neutralsublang_name_t { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5696
From: Daniel Lehman <dlehman25(a)gmail.com> --- dlls/kernel32/tests/locale.c | 11 ------- dlls/kernelbase/locale.c | 59 +++++++++++++++++++++++++++++------- 2 files changed, 48 insertions(+), 22 deletions(-) diff --git a/dlls/kernel32/tests/locale.c b/dlls/kernel32/tests/locale.c index bd94e10e9a3..53729453940 100644 --- a/dlls/kernel32/tests/locale.c +++ b/dlls/kernel32/tests/locale.c @@ -2929,7 +2929,6 @@ static void test_LCMapStringEx(void) } ok(!!handle, "error %ld\n", GetLastError()); - todo_wine ok(handle == ret, "ret %x, handle %#Ix, error %ld\n", ret, handle, GetLastError()); handle_en = handle; @@ -2938,10 +2937,8 @@ static void test_LCMapStringEx(void) ret = pLCMapStringEx(L"tr-TR", LCMAP_SORTHANDLE, NULL, 0, (LPWSTR)&handle, sizeof(handle), NULL, NULL, 0); ok(!!handle, "error %ld\n", GetLastError()); - todo_wine ok(handle == ret, "ret %x, handle %#Ix, error %ld\n", ret, handle, GetLastError()); handle_tr = handle; - todo_wine ok(handle_en != handle_tr, "handle_en %#Ix, handle_tr %#Ix\n", handle_en, handle_tr); SetLastError(0xdeadbeef); @@ -2956,17 +2953,14 @@ static void test_LCMapStringEx(void) ret = pLCMapStringEx(NULL, LCMAP_LOWERCASE|LCMAP_LINGUISTIC_CASING, L"I", -1, buf, ARRAY_SIZE(buf), NULL, NULL, handle_tr); ok(ret == 2, "ret %d, error %ld\n", ret, GetLastError()); - todo_wine ok(!lstrcmpW(buf, L"\x0131"), "string compare mismatch\n"); SetLastError(0xdeadbeef); handle = 0xdeadbeef; ret = pLCMapStringEx(LOCALE_NAME_USER_DEFAULT, LCMAP_SORTHANDLE, NULL, 0, (LPWSTR)&handle, sizeof(handle), NULL, NULL, 0); - todo_wine { ok(!!ret, "error %ld\n", GetLastError()); ok(handle == ret, "ret %x, handle %#Ix, error %ld\n", ret, handle, GetLastError()); - } /* bogus parms with SORTHANDLE */ handle = 0xdeadbeef; @@ -2979,7 +2973,6 @@ static void test_LCMapStringEx(void) handle = 0xdeadbeef; ret = pLCMapStringEx(L"en-US", LCMAP_SORTHANDLE, L"I", -1, (LPWSTR)&handle, sizeof(handle), NULL, NULL, 0); - todo_wine ok(!ret && GetLastError() == ERROR_INVALID_PARAMETER, "ret %d, error %ld\n", ret, GetLastError()); ok(handle == 0xdeadbeef, "handle %#Ix\n", handle); @@ -2997,21 +2990,17 @@ static void test_LCMapStringEx(void) memset(buf, 0, sizeof(buf)); ret = pLCMapStringEx(L"en-US", LCMAP_LOWERCASE|LCMAP_LINGUISTIC_CASING, L"I", -1, buf, ARRAY_SIZE(buf), NULL, NULL, handle_en); - todo_wine { ok(!ret && GetLastError() == ERROR_INVALID_PARAMETER, "ret %d, error %ld\n", ret, GetLastError()); ok(!buf[0], "buf %x\n", buf[0]); - } SetLastError(0xdeadbeef); memset(buf, 0, sizeof(buf)); ret = pLCMapStringEx(NULL, LCMAP_LOWERCASE|LCMAP_LINGUISTIC_CASING, L"I", -1, buf, ARRAY_SIZE(buf), NULL, NULL, 0xdeadbeef); - todo_wine { ok(!ret && GetLastError() == ERROR_INVALID_PARAMETER, "ret %d, error %ld\n", ret, GetLastError()); ok(!buf[0], "buf %x\n", buf[0]); - } } struct neutralsublang_name_t { diff --git a/dlls/kernelbase/locale.c b/dlls/kernelbase/locale.c index 44af29f163d..33452edb7f5 100644 --- a/dlls/kernelbase/locale.c +++ b/dlls/kernelbase/locale.c @@ -654,7 +654,7 @@ static const NLS_LOCALE_DATA *get_locale_by_name( const WCHAR *name, LCID *lcid } -static const struct sortguid *get_language_sort( const WCHAR *name ) +static const struct sortguid *get_language_sort_index( const WCHAR *name, LPARAM *index ) { const NLS_LOCALE_LCNAME_INDEX *entry; const NLS_LOCALE_DATA *locale; @@ -678,7 +678,11 @@ static const struct sortguid *get_language_sort( const WCHAR *name ) SetLastError( ERROR_INVALID_PARAMETER ); return NULL; } - if ((ret = locale_sorts[entry - lcnames_index])) return ret; + if ((ret = locale_sorts[entry - lcnames_index])) + { + *index = ret - &sort.guids[0]; + return ret; + } lcid = entry->id; name = locale_strings + entry->name + 1; @@ -701,10 +705,16 @@ static const struct sortguid *get_language_sort( const WCHAR *name ) RegCloseKey( key ); } if (!ret) ret = &sort.guids[0]; + *index = ret - &sort.guids[0]; locale_sorts[entry - lcnames_index] = ret; return ret; } +static inline const struct sortguid *get_language_sort( const WCHAR *name ) +{ + LPARAM dummy; + return get_language_sort_index( name, &dummy ); +} /****************************************************************************** * NlsValidateLocale (kernelbase.@) @@ -6613,10 +6623,25 @@ INT WINAPI DECLSPEC_HOTPATCH LCMapStringEx( const WCHAR *locale, DWORD flags, co if (version) FIXME( "unsupported version structure %p\n", version ); if (reserved) FIXME( "unsupported reserved pointer %p\n", reserved ); - if (handle) + + if (flags & LCMAP_SORTHANDLE) { - static int once; - if (!once++) FIXME( "unsupported lparam %Ix\n", handle ); + if (src || srclen || !dst || dstlen < sizeof(LPARAM)) + { + SetLastError( ERROR_INVALID_PARAMETER ); + return 0; + } + + handle = 0; + if (!(get_language_sort_index( locale, &handle ))) + { + SetLastError( ERROR_INVALID_PARAMETER ); + return 0; + } + + handle = MAKELONG(handle, 1); + *(LPARAM *)dst = handle; + return handle; } if (!src || !srclen || dstlen < 0) @@ -6639,18 +6664,30 @@ INT WINAPI DECLSPEC_HOTPATCH LCMapStringEx( const WCHAR *locale, DWORD flags, co } if (flags & (LCMAP_LOWERCASE | LCMAP_UPPERCASE | LCMAP_SORTKEY)) { - if (!(sortid = get_language_sort( locale ))) return 0; + if (handle) + { + if (locale) + { + SetLastError( ERROR_INVALID_PARAMETER ); + return 0; + } + + handle = LOWORD(handle); + if (handle < 0 || handle >= sort.guid_count) + { + SetLastError( ERROR_INVALID_PARAMETER ); + return 0; + } + + sortid = &sort.guids[handle]; + } + else if (!(sortid = get_language_sort( locale ))) return 0; } if (flags & LCMAP_HASH) { FIXME( "LCMAP_HASH %s not supported\n", debugstr_wn( src, srclen )); return 0; } - if (flags & LCMAP_SORTHANDLE) - { - FIXME( "LCMAP_SORTHANDLE not supported\n" ); - return 0; - } if (flags & LCMAP_SORTKEY) return get_sortkey( sortid, flags, src, srclen, (BYTE *)dst, dstlen ); return lcmap_string( sortid, flags, src, srclen, dst, dstlen ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5696
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=145729 Your paranoid android. === debian11b (64 bit WoW report) === user32: input.c:3986: Test succeeded inside todo block: button_down_hwnd_todo 1: got MSG_TEST_WIN hwnd 00000000013F00E6, msg WM_LBUTTONDOWN, wparam 0x1, lparam 0x320032
Using SORTHANDLE is discouraged, why do you need it? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5696#note_71001
an optimization for _wcsicmp_l it's meant to be used internally, but i added tests to try to match windows behavior 24a2b625545f1875b5c3177f2b9da1b7299b864f degraded performance. perf showed most time was spent in get_language_sort. _wcsicmp_l is being used heavily for std::map/set _wcsicmp_l calls _towlower_l for each letter of both strings, which in turn calls: towlower_l -> LCMapStringW -> LCMapStringEx -> get_language_sort the sortguid is the same since the same locale is passed. using SORTHANDLE gets the performance roughly where it was here are all 5 commits, if you want to see where it's going: https://gitlab.winehq.org/dlehman25/wine/-/tree/wcsicmp4 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5696#note_71098
The docs say that SORTHANDLE is discouraged because it makes essentially no difference. Which means that the normal non-SORTHANDLE case should be made faster instead. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5696#note_71212
This merge request was closed by Daniel Lehman. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5696
ok, will find another way -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5696#note_71256
participants (4)
-
Alexandre Julliard (＠julliard) -
Daniel Lehman
-
Daniel Lehman (＠dlehman25)
-
Marvin