[PATCH v2 0/3] MR6680: msvcp140: Use current locale to convert path in _Fiopen.
-- v2: msvcp140: Use current locale to convert path in _Fiopen. msvcp120/tests: Add tests for _Fiopen. msvcp140/tests: Add tests for _Fiopen. https://gitlab.winehq.org/wine/wine/-/merge_requests/6680
From: Daniel Lehman <dlehman25(a)gmail.com> --- dlls/msvcp140/tests/msvcp140.c | 64 ++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/dlls/msvcp140/tests/msvcp140.c b/dlls/msvcp140/tests/msvcp140.c index faf4c214801..44f999fb838 100644 --- a/dlls/msvcp140/tests/msvcp140.c +++ b/dlls/msvcp140/tests/msvcp140.c @@ -22,6 +22,8 @@ #include "windef.h" #include "winbase.h" #include "winnls.h" +#include "share.h" +#include "locale.h" #include "wine/test.h" #include "winbase.h" @@ -253,6 +255,23 @@ static ULONG (__cdecl *p__Winerror_message)(ULONG, char*, ULONG); static int (__cdecl *p__Winerror_map)(int); static const char* (__cdecl *p__Syserror_map)(int err); +typedef enum { + OPENMODE_in = 0x01, + OPENMODE_out = 0x02, + OPENMODE_ate = 0x04, + OPENMODE_app = 0x08, + OPENMODE_trunc = 0x10, + OPENMODE__Nocreate = 0x40, + OPENMODE__Noreplace = 0x80, + OPENMODE_binary = 0x20, + OPENMODE_mask = 0xff +} IOSB_openmode; +static FILE* (__cdecl *p__Fiopen_wchar)(const wchar_t*, int, int); +static FILE* (__cdecl *p__Fiopen)(const char*, int, int); + +static char* (__cdecl *p_setlocale)(int, const char*); +static int (__cdecl *p_fclose)(FILE*); + static BOOLEAN (WINAPI *pCreateSymbolicLinkW)(const WCHAR *, const WCHAR *, DWORD); static HMODULE msvcp; @@ -291,6 +310,9 @@ static BOOL init(void) SET(p__Release_chore, "?_Release_chore(a)details@Concurrency@@YAXPEAU_Threadpool_chore(a)12@@Z"); SET(p__Winerror_message, "?_Winerror_message(a)std@@YAKKPEADK(a)Z"); SET(p__Syserror_map, "?_Syserror_map(a)std@@YAPEBDH(a)Z"); + + SET(p__Fiopen_wchar, "?_Fiopen(a)std@@YAPEAU_iobuf@@PEB_WHH(a)Z"); + SET(p__Fiopen, "?_Fiopen(a)std@@YAPEAU_iobuf@@PEBDHH(a)Z"); } else { #ifdef __arm__ SET(p_task_continuation_context_ctor, "??0task_continuation_context(a)Concurrency@@AAA(a)XZ"); @@ -322,6 +344,9 @@ static BOOL init(void) SET(p__Release_chore, "?_Release_chore(a)details@Concurrency@@YAXPAU_Threadpool_chore(a)12@@Z"); SET(p__Winerror_message, "?_Winerror_message(a)std@@YAKKPADK(a)Z"); SET(p__Syserror_map, "?_Syserror_map(a)std@@YAPBDH(a)Z"); + + SET(p__Fiopen_wchar, "?_Fiopen(a)std@@YAPAU_iobuf@@PB_WHH(a)Z"); + SET(p__Fiopen, "?_Fiopen(a)std@@YAPAU_iobuf@@PBDHH(a)Z"); } SET(p__Mtx_init, "_Mtx_init"); @@ -362,6 +387,10 @@ static BOOL init(void) SET(p_To_wide, "_To_wide"); SET(p_Unlink, "_Unlink"); + hdll = GetModuleHandleA("ucrtbase.dll"); + p_setlocale = (void*)GetProcAddress(hdll, "setlocale"); + p_fclose = (void*)GetProcAddress(hdll, "fclose"); + hdll = GetModuleHandleA("kernel32.dll"); pCreateSymbolicLinkW = (void*)GetProcAddress(hdll, "CreateSymbolicLinkW"); @@ -1670,6 +1699,40 @@ static void test__Mtx(void) p__Mtx_destroy(mtx); } +static void test__Fiopen(void) +{ + int i; + FILE *f; + char *oldloc; + static const struct { + const char *loc; + const WCHAR *wpath; + const char *apath; + int is_todo; + } tests[] = { + { "C", L"utf_\x00e4\x00cf\x00f6\x00df.txt", "utf_\xe4\xcf\xf6\xdf.txt" }, + { "de_DE", L"utf_\x00e4\x00cf\x00f6\x00df.txt", "utf_\xe4\xcf\xf6\xdf.txt" }, + { "de_DE.utf8", L"utf_\x00e4\x00cf\x00f6\x00df.txt", "utf_\xc3\xa4\xc3\x8f\xc3\xb6\xc3\x9f.txt", TRUE }, + }; + + oldloc = p_setlocale(LC_ALL, NULL); + for(i=0; i<ARRAY_SIZE(tests); i++) { + if(!p_setlocale(LC_ALL, tests[i].loc)) { + win_skip("skipping locale %s\n", tests[i].loc); + continue; + } + f = p__Fiopen_wchar(tests[i].wpath, OPENMODE_out, SH_DENYNO); + ok(!!f, "failed to create %s with locale %s\n", wine_dbgstr_w(tests[i].wpath), tests[i].loc); + if(f) p_fclose(f); + f = p__Fiopen(tests[i].apath, OPENMODE_in, SH_DENYNO); + todo_wine_if(tests[i].is_todo) + ok(!!f, "failed to read %s with locale %s\n", tests[i].apath, tests[i].loc); + if(f) p_fclose(f); + DeleteFileW(tests[i].wpath); + } + p_setlocale(LC_ALL, oldloc); +} + START_TEST(msvcp140) { if(!init()) return; @@ -1698,5 +1761,6 @@ START_TEST(msvcp140) test_cnd(); test_Copy_file(); test__Mtx(); + test__Fiopen(); FreeLibrary(msvcp); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6680
From: Daniel Lehman <dlehman25(a)gmail.com> --- dlls/msvcp120/tests/msvcp120.c | 58 ++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/dlls/msvcp120/tests/msvcp120.c b/dlls/msvcp120/tests/msvcp120.c index c7de814a384..04e22bebe65 100644 --- a/dlls/msvcp120/tests/msvcp120.c +++ b/dlls/msvcp120/tests/msvcp120.c @@ -20,6 +20,7 @@ #include <stdio.h> #include <math.h> #include <limits.h> +#include <share.h> #include "wine/test.h" #include "winbase.h" @@ -210,6 +211,7 @@ static BOOL compare_float(float f, float g, unsigned int ulps) static char* (__cdecl *p_setlocale)(int, const char*); static int (__cdecl *p__setmbcp)(int); static int (__cdecl *p__ismbblead)(unsigned int); +static int (__cdecl *p_fclose)(FILE*); static MSVCRT_long (__cdecl *p__Xtime_diff_to_millis2)(const xtime*, const xtime*); static int (__cdecl *p_xtime_get)(xtime*, int); @@ -423,6 +425,20 @@ static void (__thiscall *p_vector_base_v4__Internal_resize)( static const BYTE *p_byte_reverse_table; +typedef enum { + OPENMODE_in = 0x01, + OPENMODE_out = 0x02, + OPENMODE_ate = 0x04, + OPENMODE_app = 0x08, + OPENMODE_trunc = 0x10, + OPENMODE__Nocreate = 0x40, + OPENMODE__Noreplace = 0x80, + OPENMODE_binary = 0x20, + OPENMODE_mask = 0xff +} IOSB_openmode; +static FILE* (__cdecl *p__Fiopen_wchar)(const wchar_t*, int, int); +static FILE* (__cdecl *p__Fiopen)(const char*, int, int); + static HMODULE msvcp; #define SETNOFAIL(x,y) x = (void*)GetProcAddress(msvcp,y) #define SET(x,y) do { SETNOFAIL(x,y); ok(x != NULL, "Export '%s' not found\n", y); } while(0) @@ -584,6 +600,10 @@ static BOOL init(void) "?_Internal_resize(a)_Concurrent_vector_base_v4@details(a)Concurrency@@IEAAX_K00P6AXPEAX0(a)ZP6AX1PEBX0@Z3(a)Z"); SET(p__Syserror_map, "?_Syserror_map(a)std@@YAPEBDH(a)Z"); + SET(p__Fiopen_wchar, + "?_Fiopen(a)std@@YAPEAU_iobuf@@PEB_WHH(a)Z"); + SET(p__Fiopen, + "?_Fiopen(a)std@@YAPEAU_iobuf@@PEBDHH(a)Z"); } else { SET(p_tr2_sys__File_size, "?_File_size(a)sys@tr2(a)std@@YA_KPBD(a)Z"); @@ -659,6 +679,10 @@ static BOOL init(void) "?_Segment_index_of(a)_Concurrent_vector_base_v4@details(a)Concurrency@@KAII(a)Z"); SET(p__Syserror_map, "?_Syserror_map(a)std@@YAPBDH(a)Z"); + SET(p__Fiopen_wchar, + "?_Fiopen(a)std@@YAPAU_iobuf@@PB_WHH(a)Z"); + SET(p__Fiopen, + "?_Fiopen(a)std@@YAPAU_iobuf@@PBDHH(a)Z"); #ifdef __i386__ SET(p_i386_Thrd_current, "_Thrd_current"); @@ -821,6 +845,7 @@ static BOOL init(void) p_setlocale = (void*)GetProcAddress(hdll, "setlocale"); p__setmbcp = (void*)GetProcAddress(hdll, "_setmbcp"); p__ismbblead = (void*)GetProcAddress(hdll, "_ismbblead"); + p_fclose = (void*)GetProcAddress(hdll, "fclose"); hdll = GetModuleHandleA("kernel32.dll"); pCreateSymbolicLinkA = (void*)GetProcAddress(hdll, "CreateSymbolicLinkA"); @@ -3343,6 +3368,37 @@ static void test_data_exports(void) } } +static void test__Fiopen(void) +{ + int i; + FILE *f; + char *oldloc; + static const struct { + const char *loc; + const WCHAR *wpath; + const char *apath; + } tests[] = { + { "C", L"utf_\x00e4\x00cf\x00f6\x00df.txt", "utf_\xe4\xcf\xf6\xdf.txt" }, + { "de_DE", L"utf_\x00e4\x00cf\x00f6\x00df.txt", "utf_\xe4\xcf\xf6\xdf.txt" }, + }; + + oldloc = p_setlocale(LC_ALL, NULL); + for(i=0; i<ARRAY_SIZE(tests); i++) { + if(!p_setlocale(LC_ALL, tests[i].loc)) { + win_skip("skipping locale %s\n", tests[i].loc); + continue; + } + f = p__Fiopen_wchar(tests[i].wpath, OPENMODE_out, SH_DENYNO); + ok(!!f, "failed to create %s with locale %s\n", wine_dbgstr_w(tests[i].wpath), tests[i].loc); + if(f) p_fclose(f); + f = p__Fiopen(tests[i].apath, OPENMODE_in, SH_DENYNO); + ok(!!f, "failed to read %s with locale %s\n", tests[i].apath, tests[i].loc); + if(f) p_fclose(f); + DeleteFileW(tests[i].wpath); + } + p_setlocale(LC_ALL, oldloc); +} + START_TEST(msvcp120) { if(!init()) return; @@ -3390,6 +3446,8 @@ START_TEST(msvcp120) test_data_exports(); + test__Fiopen(); + free_expect_struct(); TlsFree(expect_idx); FreeLibrary(msvcp); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6680
From: Daniel Lehman <dlehman25(a)gmail.com> --- dlls/msvcp140/tests/msvcp140.c | 4 +--- dlls/msvcp90/ios.c | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/dlls/msvcp140/tests/msvcp140.c b/dlls/msvcp140/tests/msvcp140.c index 44f999fb838..34d926fa27c 100644 --- a/dlls/msvcp140/tests/msvcp140.c +++ b/dlls/msvcp140/tests/msvcp140.c @@ -1708,11 +1708,10 @@ static void test__Fiopen(void) const char *loc; const WCHAR *wpath; const char *apath; - int is_todo; } tests[] = { { "C", L"utf_\x00e4\x00cf\x00f6\x00df.txt", "utf_\xe4\xcf\xf6\xdf.txt" }, { "de_DE", L"utf_\x00e4\x00cf\x00f6\x00df.txt", "utf_\xe4\xcf\xf6\xdf.txt" }, - { "de_DE.utf8", L"utf_\x00e4\x00cf\x00f6\x00df.txt", "utf_\xc3\xa4\xc3\x8f\xc3\xb6\xc3\x9f.txt", TRUE }, + { "de_DE.utf8", L"utf_\x00e4\x00cf\x00f6\x00df.txt", "utf_\xc3\xa4\xc3\x8f\xc3\xb6\xc3\x9f.txt" }, }; oldloc = p_setlocale(LC_ALL, NULL); @@ -1725,7 +1724,6 @@ static void test__Fiopen(void) ok(!!f, "failed to create %s with locale %s\n", wine_dbgstr_w(tests[i].wpath), tests[i].loc); if(f) p_fclose(f); f = p__Fiopen(tests[i].apath, OPENMODE_in, SH_DENYNO); - todo_wine_if(tests[i].is_todo) ok(!!f, "failed to read %s with locale %s\n", tests[i].apath, tests[i].loc); if(f) p_fclose(f); DeleteFileW(tests[i].wpath); diff --git a/dlls/msvcp90/ios.c b/dlls/msvcp90/ios.c index 5b791cd4345..50421ecc15b 100644 --- a/dlls/msvcp90/ios.c +++ b/dlls/msvcp90/ios.c @@ -3272,7 +3272,7 @@ FILE* __cdecl _Fiopen(const char *name, int mode, int prot) TRACE("(%s %d %d)\n", name, mode, prot); -#if _MSVCP_VER >= 80 && _MSVCP_VER <= 90 +#if (_MSVCP_VER >= 80 && _MSVCP_VER <= 90) || _MSVCP_VER >= 140 if(mbstowcs_s(NULL, nameW, FILENAME_MAX, name, FILENAME_MAX-1) != 0) return NULL; #else -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6680
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=149053 Your paranoid android. === w7u_el (32 bit report) === msvcp120: msvcp120.c:3395: Test failed: failed to read utf_äÏöß.txt with locale C === w10pro64_en_AE_u8 (32 bit report) === msvcp120: msvcp120.c:3395: Test failed: failed to read utf_äÏöß.txt with locale C msvcp120.c:3395: Test failed: failed to read utf_äÏöß.txt with locale de_DE === w10pro64_ar (64 bit report) === msvcp120: msvcp120.c:3395: Test failed: failed to read utf_äÏöß.txt with locale C msvcp120.c:3395: Test failed: failed to read utf_äÏöß.txt with locale de_DE === w10pro64_ja (64 bit report) === msvcp120: msvcp120.c:3395: Test failed: failed to read utf_äÏöß.txt with locale C msvcp120.c:3395: Test failed: failed to read utf_äÏöß.txt with locale de_DE === w10pro64_zh_CN (64 bit report) === msvcp120: msvcp120.c:3395: Test failed: failed to read utf_äÏöß.txt with locale C msvcp120.c:3395: Test failed: failed to read utf_äÏöß.txt with locale de_DE === w7u_el (32 bit report) === msvcp140: msvcp140.c:1727: Test failed: failed to read utf_äÏöß.txt with locale C === w1064v1809 (32 bit report) === msvcp140: msvcp140.c:1727: Test failed: failed to read utf_äÏöß.txt with locale de_DE === w10pro64_en_AE_u8 (32 bit report) === msvcp140: msvcp140.c:1727: Test failed: failed to read utf_äÏöß.txt with locale C msvcp140.c:1727: Test failed: failed to read utf_äÏöß.txt with locale de_DE === w1064v1809 (64 bit report) === msvcp140: msvcp140.c:1727: Test failed: failed to read utf_äÏöß.txt with locale de_DE === w10pro64_ar (64 bit report) === msvcp140: msvcp140.c:1727: Test failed: failed to read utf_äÏöß.txt with locale C msvcp140.c:1727: Test failed: failed to read utf_äÏöß.txt with locale de_DE === w10pro64_ja (64 bit report) === msvcp140: msvcp140.c:1727: Test failed: failed to read utf_äÏöß.txt with locale C msvcp140.c:1727: Test failed: failed to read utf_äÏöß.txt with locale de_DE === w10pro64_zh_CN (64 bit report) === msvcp140: msvcp140.c:1727: Test failed: failed to read utf_äÏöß.txt with locale C msvcp140.c:1727: Test failed: failed to read utf_äÏöß.txt with locale de_DE === debian11 (32 bit ar:MA report) === msvcp120: msvcp120.c:3395: Test failed: failed to read utf_äÏöß.txt with locale C msvcp120.c:3395: Test failed: failed to read utf_äÏöß.txt with locale de_DE === debian11 (32 bit he:IL report) === msvcp120: msvcp120.c:3395: Test failed: failed to read utf_äÏöß.txt with locale C msvcp120.c:3395: Test failed: failed to read utf_äÏöß.txt with locale de_DE === debian11 (32 bit hi:IN report) === msvcp120: msvcp120.c:3395: Test failed: failed to read utf_äÏöß.txt with locale C msvcp120.c:3395: Test failed: failed to read utf_äÏöß.txt with locale de_DE === debian11 (32 bit ja:JP report) === msvcp120: msvcp120.c:3395: Test failed: failed to read utf_äÏöß.txt with locale C msvcp120.c:3395: Test failed: failed to read utf_äÏöß.txt with locale de_DE === debian11 (32 bit zh:CN report) === msvcp120: msvcp120.c:3395: Test failed: failed to read utf_äÏöß.txt with locale C msvcp120.c:3395: Test failed: failed to read utf_äÏöß.txt with locale de_DE
Piotr Caban (@piotr) commented about dlls/msvcp120/tests/msvcp120.c:
} }
+static void test__Fiopen(void) +{ + int i; + FILE *f; + char *oldloc; + static const struct { + const char *loc; + const WCHAR *wpath; + const char *apath; + } tests[] = { + { "C", L"utf_\x00e4\x00cf\x00f6\x00df.txt", "utf_\xe4\xcf\xf6\xdf.txt" }, + { "de_DE", L"utf_\x00e4\x00cf\x00f6\x00df.txt", "utf_\xe4\xcf\xf6\xdf.txt" },
The implementation uses CP_ACP to convert between wchar and char version. You will need to do the same in the tests, otherwise the tests may fail depending on system settings. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6680#note_85479
Piotr Caban (@piotr) commented about dlls/msvcp140/tests/msvcp140.c:
+ int is_todo; + } tests[] = { + { "C", L"utf_\x00e4\x00cf\x00f6\x00df.txt", "utf_\xe4\xcf\xf6\xdf.txt" }, + { "de_DE", L"utf_\x00e4\x00cf\x00f6\x00df.txt", "utf_\xe4\xcf\xf6\xdf.txt" }, + { "de_DE.utf8", L"utf_\x00e4\x00cf\x00f6\x00df.txt", "utf_\xc3\xa4\xc3\x8f\xc3\xb6\xc3\x9f.txt", TRUE }, + }; + + oldloc = p_setlocale(LC_ALL, NULL); + for(i=0; i<ARRAY_SIZE(tests); i++) { + if(!p_setlocale(LC_ALL, tests[i].loc)) { + win_skip("skipping locale %s\n", tests[i].loc); + continue; + } + f = p__Fiopen_wchar(tests[i].wpath, OPENMODE_out, SH_DENYNO); + ok(!!f, "failed to create %s with locale %s\n", wine_dbgstr_w(tests[i].wpath), tests[i].loc); + if(f) p_fclose(f);
p_fclose(f);
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6680#note_85478
Piotr Caban (@piotr) commented about dlls/msvcp140/tests/msvcp140.c:
+{ + int i; + FILE *f; + char *oldloc; + static const struct { + const char *loc; + const WCHAR *wpath; + const char *apath; + int is_todo; + } tests[] = { + { "C", L"utf_\x00e4\x00cf\x00f6\x00df.txt", "utf_\xe4\xcf\xf6\xdf.txt" }, + { "de_DE", L"utf_\x00e4\x00cf\x00f6\x00df.txt", "utf_\xe4\xcf\xf6\xdf.txt" }, + { "de_DE.utf8", L"utf_\x00e4\x00cf\x00f6\x00df.txt", "utf_\xc3\xa4\xc3\x8f\xc3\xb6\xc3\x9f.txt", TRUE }, + }; + + oldloc = p_setlocale(LC_ALL, NULL); You can't store setlocale return like this - it's get invalidated when locale is changed. Please restore to C locale instead.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6680#note_85477
Piotr Caban (@piotr) commented about dlls/msvcp90/ios.c:
TRACE("(%s %d %d)\n", name, mode, prot);
-#if _MSVCP_VER >= 80 && _MSVCP_VER <= 90 +#if (_MSVCP_VER >= 80 && _MSVCP_VER <= 90) || _MSVCP_VER >= 140
Following test fails on Windows but succeeds in Wine: { "pl_PL", L"t\x0119\x015b\x0107.txt", "t\xea\x9c\xe6.txt" }, -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6680#note_85476
participants (4)
-
Daniel Lehman -
Daniel Lehman (＠dlehman25)
-
Marvin -
Piotr Caban (＠piotr)