-- v2: msvcrt/tests: Add tests of _strnicmp_l(). include: Add _strnicmp_l() declaration. msvcrt/tests: Test _stricmp() with multiple bytes characters. msvcrt: Improve DBCS support for _strnicmp_l(). msvcrt/tests: Test _tolower_l() with DBCS. msvcrt: Improve DBCS support for _tolower_l(). msvcrt/tests: Test tolower() with DBCS.
From: Jactry Zeng jzeng@codeweavers.com
--- dlls/msvcrt/tests/string.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
diff --git a/dlls/msvcrt/tests/string.c b/dlls/msvcrt/tests/string.c index 23f4205e785..d1d3a092dd9 100644 --- a/dlls/msvcrt/tests/string.c +++ b/dlls/msvcrt/tests/string.c @@ -3387,6 +3387,30 @@ static void test_tolower(void) ret = p_tolower((unsigned char)0xD0); ok(ret == 0xF0, "ret = %x\n", ret);
+ ok(setlocale(LC_ALL, ".932") != NULL, "setlocale failed.\n"); + ret = p_tolower((signed char)0xd0); + ok(ret == 0xd0, "Got %#x.\n", ret); + ret = p_tolower((unsigned char)0xd0); + todo_wine ok(ret == (unsigned char)0xd0, "Got %#x.\n", ret); + + ok(setlocale(LC_ALL, ".936") != NULL, "setlocale failed.\n"); + ret = p_tolower((signed char)0xd0); + ok(ret == (signed char)0xd0, "Got %#x.\n", ret); + ret = p_tolower((unsigned char)0xd0); + todo_wine ok(ret == (unsigned char)0xd0, "Got %#x.\n", ret); + + ok(setlocale(LC_ALL, ".949") != NULL, "setlocale failed.\n"); + ret = p_tolower((signed char)0xd0); + ok(ret == (signed char)0xd0, "Got %#x.\n", ret); + ret = p_tolower((unsigned char)0xd0); + todo_wine ok(ret == (unsigned char)0xd0, "Got %#x.\n", ret); + + ok(setlocale(LC_ALL, ".950") != NULL, "setlocale failed.\n"); + ret = p_tolower((signed char)0xd0); + ok(ret == (signed char)0xd0, "Got %#x.\n", ret); + ret = p_tolower((unsigned char)0xd0); + todo_wine ok(ret == (unsigned char)0xd0, "Got %#x.\n", ret); + setlocale(LC_ALL, "C"); }
From: Jactry Zeng jzeng@codeweavers.com
--- dlls/msvcrt/ctype.c | 2 +- dlls/msvcrt/tests/string.c | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/dlls/msvcrt/ctype.c b/dlls/msvcrt/ctype.c index a1805324c1f..73d63057a93 100644 --- a/dlls/msvcrt/ctype.c +++ b/dlls/msvcrt/ctype.c @@ -535,7 +535,7 @@ int CDECL _tolower_l(int c, _locale_t locale) else locinfo = locale->locinfo;
- if((unsigned)c < 256) + if((unsigned)c < 256 && locinfo->mb_cur_max == 1) return locinfo->pclmap[c];
if(locinfo->pctype[(c>>8)&255] & _LEADBYTE) diff --git a/dlls/msvcrt/tests/string.c b/dlls/msvcrt/tests/string.c index d1d3a092dd9..c131c7d0442 100644 --- a/dlls/msvcrt/tests/string.c +++ b/dlls/msvcrt/tests/string.c @@ -3391,25 +3391,25 @@ static void test_tolower(void) ret = p_tolower((signed char)0xd0); ok(ret == 0xd0, "Got %#x.\n", ret); ret = p_tolower((unsigned char)0xd0); - todo_wine ok(ret == (unsigned char)0xd0, "Got %#x.\n", ret); + ok(ret == (unsigned char)0xd0, "Got %#x.\n", ret);
ok(setlocale(LC_ALL, ".936") != NULL, "setlocale failed.\n"); ret = p_tolower((signed char)0xd0); ok(ret == (signed char)0xd0, "Got %#x.\n", ret); ret = p_tolower((unsigned char)0xd0); - todo_wine ok(ret == (unsigned char)0xd0, "Got %#x.\n", ret); + ok(ret == (unsigned char)0xd0, "Got %#x.\n", ret);
ok(setlocale(LC_ALL, ".949") != NULL, "setlocale failed.\n"); ret = p_tolower((signed char)0xd0); ok(ret == (signed char)0xd0, "Got %#x.\n", ret); ret = p_tolower((unsigned char)0xd0); - todo_wine ok(ret == (unsigned char)0xd0, "Got %#x.\n", ret); + ok(ret == (unsigned char)0xd0, "Got %#x.\n", ret);
ok(setlocale(LC_ALL, ".950") != NULL, "setlocale failed.\n"); ret = p_tolower((signed char)0xd0); ok(ret == (signed char)0xd0, "Got %#x.\n", ret); ret = p_tolower((unsigned char)0xd0); - todo_wine ok(ret == (unsigned char)0xd0, "Got %#x.\n", ret); + ok(ret == (unsigned char)0xd0, "Got %#x.\n", ret);
setlocale(LC_ALL, "C"); }
From: Jactry Zeng jzeng@codeweavers.com
--- dlls/msvcrt/tests/string.c | 47 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+)
diff --git a/dlls/msvcrt/tests/string.c b/dlls/msvcrt/tests/string.c index c131c7d0442..58d8b0b5e3d 100644 --- a/dlls/msvcrt/tests/string.c +++ b/dlls/msvcrt/tests/string.c @@ -4808,6 +4808,52 @@ static void test_mbsrev(void) _setmbcp(cp); }
+static void test__tolower_l(void) +{ + int ret; + + if (!setlocale(LC_ALL, "english")) + { + win_skip("Skip _tolower_l tests.\n"); + return; + } + + ret = _tolower_l('\xa5', 0); + ok(ret == 165, "Got %d.\n", ret); + ret = _tolower_l('\xb9', 0); + ok(ret == 185, "Got %d.\n", ret); + ret = _tolower_l('a', 0); + ok(ret == 97, "Got %d.\n", ret); + + if (!setlocale(LC_ALL, ".936")) + { + win_skip("Skip _tolower_l tests.\n"); + return; + } + + ret = _tolower_l('\xa5', 0); + ok(ret == -91, "Got %d.\n", ret); + ret = _tolower_l('\xb9', 0); + ok(ret == -71, "Got %d.\n", ret); + ret = _tolower_l('a', 0); + ok(ret == 97, "Got %d.\n", ret); + + if (!setlocale(LC_ALL, "chinese-simplified")) + { + win_skip("Skip _tolower_l tests.\n"); + return; + } + + ret = _tolower_l('\xa5', 0); + ok(ret == -91, "Got %d.\n", ret); + ret = _tolower_l('\xb9', 0); + ok(ret == -71, "Got %d.\n", ret); + ret = _tolower_l('a', 0); + ok(ret == 97, "Got %d.\n", ret); + + setlocale(LC_ALL, "C"); +} + START_TEST(string) { char mem[100]; @@ -4970,4 +5016,5 @@ START_TEST(string) test__mbbtype(); test_wcsncpy(); test_mbsrev(); + test__tolower_l(); }
From: Jactry Zeng jzeng@codeweavers.com
--- dlls/msvcrt/string.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/msvcrt/string.c b/dlls/msvcrt/string.c index 571ca34b847..90538533d95 100644 --- a/dlls/msvcrt/string.c +++ b/dlls/msvcrt/string.c @@ -3338,7 +3338,7 @@ int __cdecl _strnicmp_l(const char *s1, const char *s2, c2 -= 'A' - 'a'; }while(--count && c1 && c1==c2);
- return c1-c2; + return (unsigned char)c1 - (unsigned char)c2; }
do {
From: Jactry Zeng jzeng@codeweavers.com
--- dlls/msvcrt/tests/string.c | 58 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+)
diff --git a/dlls/msvcrt/tests/string.c b/dlls/msvcrt/tests/string.c index 58d8b0b5e3d..ddd4434bb52 100644 --- a/dlls/msvcrt/tests/string.c +++ b/dlls/msvcrt/tests/string.c @@ -3515,6 +3515,10 @@ static void test__stricmp(void) ok(ret > 0, "_stricmp returned %d\n", ret); ret = _stricmp("\xa5", "\xb9"); ok(ret < 0, "_stricmp returned %d\n", ret); + ret = _stricmp("\xa5\xa1", "\xb9\xa1"); /* valid gbk characters */ + ok(ret < 0, "_stricmp returned %d\n", ret); + ret = _stricmp("abc\xa5\xa1", "abc"); + ok(ret == (unsigned char)'\xa5', "_stricmp returned %d\n", ret);
if(!setlocale(LC_ALL, "polish")) { win_skip("stricmp tests\n"); @@ -3531,6 +3535,60 @@ static void test__stricmp(void) ok(ret == 0, "_stricmp returned %d\n", ret); ret = _stricmp("a", "\xb9"); ok(ret < 0, "_stricmp returned %d\n", ret); + ret = _stricmp("\xa5\xa1", "\xb9\xa1"); /* valid gbk characters */ + ok(ret == 0, "_stricmp returned %d\n", ret); + ret = _stricmp("abc\xa5\xa1", "abc"); + ok(ret > 0, "_stricmp returned %d\n", ret); + + setlocale(LC_ALL, "C"); + + if(!setlocale(LC_ALL, ".936")) { + win_skip("stricmp tests\n"); + return; + } + + ret = _stricmp("test", "test"); + ok(ret == 0, "_stricmp returned %d\n", ret); + ret = _stricmp("a", "z"); + ok(ret < 0, "_stricmp returned %d\n", ret); + ret = _stricmp("z", "a"); + ok(ret > 0, "_stricmp returned %d\n", ret); + ret = _stricmp("\xa5", "\xb9"); + ok(ret < 0, "_stricmp returned %d\n", ret); + ret = _stricmp("a", "\xb9"); + ok(ret < 0, "_stricmp returned %d\n", ret); + ret = _stricmp("\xa5\xa1", "\xb9\xa1"); /* valid gbk characters */ + ok(ret < 0, "_stricmp returned %d\n", ret); + ret = _stricmp("\x82\xa0", "\x83\x41"); /* valid shift-jis characters */ + ok(ret < 0, "_stricmp returned %d\n", ret); + ret = _stricmp("\x81\x00", "\x81\x01"); /* invalid for gbk and shift-jis */ + ok(ret < 0, "_stricmp returned %d\n", ret); + ret = _stricmp("abc\xa5\xa1", "abc"); + ok(ret > 0, "_stricmp returned %d\n", ret); + + setlocale(LC_ALL, "C"); + + if(!setlocale(LC_ALL, ".932")) { + win_skip("stricmp tests\n"); + return; + } + + ret = _stricmp("test", "test"); + ok(ret == 0, "_stricmp returned %d\n", ret); + ret = _stricmp("a", "z"); + ok(ret < 0, "_stricmp returned %d\n", ret); + ret = _stricmp("z", "a"); + ok(ret > 0, "_stricmp returned %d\n", ret); + ret = _stricmp("\xa5", "\xb9"); + ok(ret < 0, "_stricmp returned %d\n", ret); + ret = _stricmp("\xa5\xa1", "\xb9\xa1"); /* valid gbk characters */ + ok(ret < 0, "_stricmp returned %d\n", ret); + ret = _stricmp("\x82\xa0", "\x83\x41"); /* valid shift-jis characters */ + ok(ret < 0, "_stricmp returned %d\n", ret); + ret = _stricmp("\x81\x00", "\x81\x01"); /* invalid for gbk and shift-jis */ + ok(ret < 0, "_stricmp returned %d\n", ret); + ret = _stricmp("abc\x82\xa0", "abc"); + ok(ret > 0, "_stricmp returned %d\n", ret);
setlocale(LC_ALL, "C"); }
From: Jactry Zeng jzeng@codeweavers.com
--- include/msvcrt/string.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/include/msvcrt/string.h b/include/msvcrt/string.h index 18d424b1e6a..10fbf12e99e 100644 --- a/include/msvcrt/string.h +++ b/include/msvcrt/string.h @@ -34,6 +34,7 @@ _ACRTIMP errno_t __cdecl _strlwr_s(char*,size_t); _ACRTIMP int __cdecl _strncoll(const char*, const char*, size_t); _ACRTIMP int __cdecl _strncoll_l(const char*, const char*, size_t, _locale_t); _ACRTIMP int __cdecl _strnicmp(const char*,const char*,size_t); +_ACRTIMP int __cdecl _strnicmp_l(const char*, const char*, size_t, _locale_t); _ACRTIMP int __cdecl _strnicoll(const char*, const char*, size_t); _ACRTIMP int __cdecl _strnicoll_l(const char*, const char*, size_t, _locale_t); _ACRTIMP char* __cdecl _strnset(char*,int,size_t);
From: Jactry Zeng jzeng@codeweavers.com
--- dlls/msvcrt/tests/string.c | 75 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+)
diff --git a/dlls/msvcrt/tests/string.c b/dlls/msvcrt/tests/string.c index ddd4434bb52..b67046641c7 100644 --- a/dlls/msvcrt/tests/string.c +++ b/dlls/msvcrt/tests/string.c @@ -120,6 +120,7 @@ static int (__cdecl *p__memicmp_l)(const char*, const char*, size_t, _locale_t); static size_t (__cdecl *p___strncnt)(const char*, size_t); static unsigned int (__cdecl *p_mbsnextc_l)(const unsigned char*, _locale_t); static int (__cdecl *p_mbscmp_l)(const unsigned char*, const unsigned char*, _locale_t); +static int (__cdecl *p__strnicmp_l)(const char*, const char*, size_t, _locale_t);
int CDECL __STRINGTOLD(_LDOUBLE*, char**, const char*, int);
@@ -4912,6 +4913,78 @@ static void test__tolower_l(void) setlocale(LC_ALL, "C"); }
+static void test__strnicmp_l(void) +{ + static const struct { + const char *str1; + const char *str2; + } tests[] = { + { "wine", "win" }, + /* GBK string tests */ + { "\xce\xac", "ceac" }, + { "\xce\xac\xc4\xe1", "\xce\xac" }, + { "\xce\xac""abc", "\xce\xac" }, + { "abc\xce\xac", "abc" }, + { "\xce\xac\xc4\xe1", "\xc4\xe1" }, + { "\xce\xac", "\xc4\xe1" }, + { "\xb8\xdf", "\xb8\xdb" }, + { "\xb9", "\xa5" }, + }; + _locale_t locale; + int ret, i; + + if (!p__strnicmp_l) + { + win_skip("_strnicmp_l isn't available.\n"); + return; + } + + for (i = 0; i < ARRAY_SIZE(tests); i++) + { + ret = _strnicmp_l(tests[i].str1, tests[i].str2, INT_MAX, 0); + ok(ret > 0, "tests[%d]: Got %d.\n", i, ret); + + ret = _strnicmp_l(tests[i].str2, tests[i].str1, INT_MAX, 0); + ok(ret < 0, "tests[%d]: Got %d.\n", i, ret); + } + + if (!p__create_locale) + win_skip("_create_locale isn't available.\n"); + else + { + locale = p__create_locale(LC_ALL, ".936"); + ok(locale != NULL, "Failed to create locale.\n"); + + for (i = 0; i < ARRAY_SIZE(tests); i++) + { + ret = _strnicmp_l(tests[i].str1, tests[i].str2, INT_MAX, locale); + ok(ret > 0, "tests[%d]: Got %d.\n", i, ret); + + ret = _strnicmp_l(tests[i].str2, tests[i].str1, INT_MAX, locale); + ok(ret < 0, "tests[%d]: Got %d.\n", i, ret); + } + + p__free_locale(locale); + } + + if (!setlocale(LC_ALL, ".936")) + { + win_skip("Skip testing _strnicmp_l with 936 code page.\n"); + return; + } + + for (i = 0; i < ARRAY_SIZE(tests); i++) + { + ret = _strnicmp_l(tests[i].str1, tests[i].str2, INT_MAX, 0); + ok(ret > 0, "tests[%d]: Got %d.\n", i, ret); + + ret = _strnicmp_l(tests[i].str2, tests[i].str1, INT_MAX, 0); + ok(ret < 0, "tests[%d]: Got %d.\n", i, ret); + } + + setlocale(LC_ALL, "C"); +} + START_TEST(string) { char mem[100]; @@ -4987,6 +5060,7 @@ START_TEST(string) p___strncnt = (void*)GetProcAddress(hMsvcrt, "__strncnt"); p_mbsnextc_l = (void*)GetProcAddress(hMsvcrt, "_mbsnextc_l"); p_mbscmp_l = (void*)GetProcAddress(hMsvcrt, "_mbscmp_l"); + p__strnicmp_l = (void*)GetProcAddress(hMsvcrt, "_strnicmp_l");
/* MSVCRT memcpy behaves like memmove for overlapping moves, MFC42 CString::Insert seems to rely on that behaviour */ @@ -5075,4 +5149,5 @@ START_TEST(string) test_wcsncpy(); test_mbsrev(); test__tolower_l(); + test__strnicmp_l(); }
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=149137
Your paranoid android.
=== w7u_2qxl (32 bit report) ===
msvcrt: string.c:3522: Test failed: _stricmp returned 1
=== w7u_adm (32 bit report) ===
msvcrt: string.c:3522: Test failed: _stricmp returned 1
=== w7u_el (32 bit report) ===
msvcrt: string.c:3522: Test failed: _stricmp returned 1
=== w8 (32 bit report) ===
msvcrt: string.c:3522: Test failed: _stricmp returned 1
=== w8adm (32 bit report) ===
msvcrt: string.c:3522: Test failed: _stricmp returned 1
=== w864 (32 bit report) ===
msvcrt: string.c:3522: Test failed: _stricmp returned 1
=== w1064v1507 (32 bit report) ===
msvcrt: string.c:3522: Test failed: _stricmp returned 1
=== w1064v1809 (32 bit report) ===
msvcrt: string.c:3522: Test failed: _stricmp returned 1
=== w1064_tsign (32 bit report) ===
msvcrt: string.c:3522: Test failed: _stricmp returned 1
=== w10pro64 (32 bit report) ===
msvcrt: string.c:3522: Test failed: _stricmp returned 1
=== w10pro64_en_AE_u8 (32 bit report) ===
msvcrt: string.c:3522: Test failed: _stricmp returned 1
=== w11pro64 (32 bit report) ===
msvcrt: string.c:3522: Test failed: _stricmp returned 1
On Sun Oct 20 15:44:03 2024 +0000, Piotr Caban wrote:
I think it's _tolower_l/_toupper_l that needs to be fixed (and C locale handling in strnicmp). Please also add tests with unsigned char cast:
ret = _tolower_l((unsigned char)'\xa5', 0); ok(ret == 0xa5, "Got %d.\n", ret); ret = _tolower_l((unsigned char)'\xb9', 0); ok(ret == 0xb9, "Got %d.\n", ret);
You are right, I sent another try. Thanks!
Piotr Caban (@piotr) commented about dlls/msvcrt/tests/string.c:
ret = p_tolower((unsigned char)0xD0); ok(ret == 0xF0, "ret = %x\n", ret);
- ok(setlocale(LC_ALL, ".932") != NULL, "setlocale failed.\n");
- ret = p_tolower((signed char)0xd0);
- ok(ret == 0xd0, "Got %#x.\n", ret);
- ret = p_tolower((unsigned char)0xd0);
- todo_wine ok(ret == (unsigned char)0xd0, "Got %#x.\n", ret);
```suggestion:-1+0 ret = p_tolower(0xd0); todo_wine ok(ret == 0xd0, "Got %#x.\n", ret); ```
Piotr Caban (@piotr) commented about dlls/msvcrt/tests/string.c:
ret = p_tolower((signed char)0xd0); ok(ret == 0xd0, "Got %#x.\n", ret); ret = p_tolower((unsigned char)0xd0);
- todo_wine ok(ret == (unsigned char)0xd0, "Got %#x.\n", ret);
- ok(ret == (unsigned char)0xd0, "Got %#x.\n", ret);
The implementation sets errno incorrectly - please add errno tests.
Piotr Caban (@piotr) commented about dlls/msvcrt/ctype.c:
else locinfo = locale->locinfo;
- if((unsigned)c < 256)
- if((unsigned)c < 256 && locinfo->mb_cur_max == 1)
I guess that this code should be changed to something like: ```c if((unsigned)c < 256) { if(locinfo->pctype[c] & _LEADBYTE) return c; return locinfo->pclmap[c]; } ``` but it needs more tests. Also a quick check shows that pctype is generated incorrectly for DBCS codepages - it needs to be fixed first.
Piotr Caban (@piotr) commented about dlls/msvcrt/string.c:
c2 -= 'A' - 'a'; }while(--count && c1 && c1==c2);
return c1-c2;
return (unsigned char)c1 - (unsigned char)c2;
Please change commit message - this is C locale code path.
Piotr Caban (@piotr) commented about dlls/msvcrt/tests/string.c:
win_skip("Skip _tolower_l tests.\n");
return;
- }
- ret = _tolower_l('\xa5', 0);
- ok(ret == 165, "Got %d.\n", ret);
- ret = _tolower_l('\xb9', 0);
- ok(ret == 185, "Got %d.\n", ret);
- ret = _tolower_l('a', 0);
- ok(ret == 97, "Got %d.\n", ret);
- if (!setlocale(LC_ALL, ".936"))
- {
win_skip("Skip _tolower_l tests.\n");
return;
- }
You're assuming that similar setlocale call will not fail in earlier tests. Shouldn't it be changed to `ok(setlocale(LC_ALL, ".936") != NULL, ...)`?
Piotr Caban (@piotr) commented about dlls/msvcrt/tests/string.c:
{ "\xce\xac", "\xc4\xe1" },
{ "\xb8\xdf", "\xb8\xdb" },
{ "\xb9", "\xa5" },
- };
- _locale_t locale;
- int ret, i;
- if (!p__strnicmp_l)
- {
win_skip("_strnicmp_l isn't available.\n");
return;
- }
- for (i = 0; i < ARRAY_SIZE(tests); i++)
- {
ret = _strnicmp_l(tests[i].str1, tests[i].str2, INT_MAX, 0);
```suggestion:-0+0 ret = p__strnicmp_l(tests[i].str1, tests[i].str2, INT_MAX, 0); ```
This MR is also introducing new test failure on Windows.
string.c:3522:0.016 Test failed: _stricmp returned 1 3521 | ret = _stricmp("abc\xa5\xa1", "abc"); 3522 | ok(ret == (unsigned char)'\xa5', "_stricmp returned %d\n", ret);
stricmp only documents whether it returns less than zero, zero, or greater than zero (just like POSIX strcasecmp and C memcmp). Comparing it to anything other than zero sounds likely to vary between msvcrt versions.
If the exact value is significant to something other than winetest, I'd quite like to see some comments about which exact program that is. If all known programs only care about how it compares to zero, then winetest shouldn't be more specific either.