[PATCH v2 0/6] MR6937: msvcrt: Prepare directory related functions to handle utf-8 encoded file names.
The idea is to use common helper to convert between CP_ACP and WCHAR strings first. Later all functions will be switched to support UTF-8 at the same time. -- v2: msvcrt: Call _wfullpath in _fullpath function. msvcrt: Call _wgetdcwd in _getdcwd function. msvcrt: Call _wgetcwd in _getcwd function. msvcrt: Call _wchdir in _chdir function. msvcrt: Call _wrmdir in _rmdir function. msvcrt: Call _wmkdir in _mkdir function. https://gitlab.winehq.org/wine/wine/-/merge_requests/6937
From: Piotr Caban <piotr(a)codeweavers.com> --- dlls/msvcrt/dir.c | 11 +++++++---- dlls/msvcrt/msvcrt.h | 18 ++++++++++++++++++ dlls/ucrtbase/tests/file.c | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 4 deletions(-) diff --git a/dlls/msvcrt/dir.c b/dlls/msvcrt/dir.c index 546e8d49a7b..648c8d80884 100644 --- a/dlls/msvcrt/dir.c +++ b/dlls/msvcrt/dir.c @@ -1004,10 +1004,13 @@ unsigned int CDECL _getdiskfree(unsigned int disk, struct _diskfree_t * d) */ int CDECL _mkdir(const char * newdir) { - if (CreateDirectoryA(newdir,NULL)) - return 0; - msvcrt_set_errno(GetLastError()); - return -1; + wchar_t *newdirW = NULL; + int ret; + + if (newdir && !(newdirW = wstrdupa_utf8(newdir))) return -1; + ret = _wmkdir(newdirW); + free(newdirW); + return ret; } /********************************************************************* diff --git a/dlls/msvcrt/msvcrt.h b/dlls/msvcrt/msvcrt.h index eaaf8b0034d..71b0f9039f1 100644 --- a/dlls/msvcrt/msvcrt.h +++ b/dlls/msvcrt/msvcrt.h @@ -62,6 +62,7 @@ typedef struct _iobuf #include "windef.h" #include "winbase.h" +#include "winnls.h" #undef strncpy #undef wcsncpy @@ -411,4 +412,21 @@ extern char* __cdecl __unDName(char *,const char*,int,malloc_func_t,free_func_t, #define INHERIT_THREAD_PRIORITY 0xF000 +static inline int convert_acp_utf8_to_wcs(const char *str, wchar_t *wstr, int len) +{ + return MultiByteToWideChar(CP_ACP, MB_PRECOMPOSED, str, -1, wstr, len); +} + +static inline wchar_t* wstrdupa_utf8(const char *str) +{ + int len = convert_acp_utf8_to_wcs(str, NULL, 0); + wchar_t *wstr; + + if (!len) return NULL; + wstr = malloc(len * sizeof(wchar_t)); + if (!wstr) return NULL; + convert_acp_utf8_to_wcs(str, wstr, len); + return wstr; +} + #endif /* __WINE_MSVCRT_H */ diff --git a/dlls/ucrtbase/tests/file.c b/dlls/ucrtbase/tests/file.c index 99372a391e3..ef62db88062 100644 --- a/dlls/ucrtbase/tests/file.c +++ b/dlls/ucrtbase/tests/file.c @@ -18,6 +18,8 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA */ +#include <errno.h> +#include <direct.h> #include <stdarg.h> #include <locale.h> #include <share.h> @@ -216,10 +218,43 @@ static void test_fopen(void) setlocale(LC_ALL, "C"); } +static void test_utf8(void) +{ + const char dir[] = "dir\xc4\x99\xc5\x9b\xc4\x87"; + const WCHAR dirW[] = L"dir\x0119\x015b\x0107"; + + int ret; + + if (!setlocale(LC_ALL, ".utf8")) + { + win_skip("utf-8 tests\n"); + return; + } + + ret = _mkdir(dir); + if (ret == -1 && errno == ENOENT) + { + skip("can't create test environment\n"); + return; + } + ok(!ret, "_mkdir returned %d, error %d\n", ret, errno); + + ret = _wrmdir(dirW); + todo_wine_if(GetACP() != CP_UTF8) ok(!ret, "_wrmdir returned %d, errno %d\n", ret, errno); + if (ret) + { + ret = _rmdir(dir); + ok(!ret, "_rmdir returned %d, errno %d\n", ret, errno); + } + + setlocale(LC_ALL, "C"); +} + START_TEST(file) { test_std_stream_buffering(); test_iobuf_layout(); test_std_stream_open(); test_fopen(); + test_utf8(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6937
From: Piotr Caban <piotr(a)codeweavers.com> --- dlls/msvcrt/dir.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/dlls/msvcrt/dir.c b/dlls/msvcrt/dir.c index 648c8d80884..4fd0d200500 100644 --- a/dlls/msvcrt/dir.c +++ b/dlls/msvcrt/dir.c @@ -1043,10 +1043,13 @@ int CDECL _wmkdir(const wchar_t* newdir) */ int CDECL _rmdir(const char * dir) { - if (RemoveDirectoryA(dir)) - return 0; - msvcrt_set_errno(GetLastError()); - return -1; + wchar_t *dirW = NULL; + int ret; + + if (dir && !(dirW = wstrdupa_utf8(dir))) return -1; + ret = _wrmdir(dirW); + free(dirW); + return ret; } /********************************************************************* -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6937
From: Piotr Caban <piotr(a)codeweavers.com> --- dlls/msvcrt/dir.c | 13 +++++++------ dlls/ucrtbase/tests/file.c | 5 +++++ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/dlls/msvcrt/dir.c b/dlls/msvcrt/dir.c index 4fd0d200500..d574a4ae22e 100644 --- a/dlls/msvcrt/dir.c +++ b/dlls/msvcrt/dir.c @@ -274,12 +274,13 @@ static void msvcrt_wfttofd64i32( const WIN32_FIND_DATAW *fd, struct _wfinddata64 */ int CDECL _chdir(const char * newdir) { - if (!SetCurrentDirectoryA(newdir)) - { - msvcrt_set_errno(newdir?GetLastError():0); - return -1; - } - return 0; + wchar_t *newdirW = NULL; + int ret; + + if (newdir && !(newdirW = wstrdupa_utf8(newdir))) return -1; + ret = _wchdir(newdirW); + free(newdirW); + return ret; } /********************************************************************* diff --git a/dlls/ucrtbase/tests/file.c b/dlls/ucrtbase/tests/file.c index ef62db88062..bb01a8a7b8f 100644 --- a/dlls/ucrtbase/tests/file.c +++ b/dlls/ucrtbase/tests/file.c @@ -239,6 +239,11 @@ static void test_utf8(void) } ok(!ret, "_mkdir returned %d, error %d\n", ret, errno); + ret = _chdir(dir); + ok(!ret, "_chdir returned %d, error %d\n", ret, errno); + ret = _chdir(".."); + ok(!ret, "_chdir returned %d, error %d\n", ret, errno); + ret = _wrmdir(dirW); todo_wine_if(GetACP() != CP_UTF8) ok(!ret, "_wrmdir returned %d, errno %d\n", ret, errno); if (ret) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6937
From: Piotr Caban <piotr(a)codeweavers.com> --- dlls/msvcrt/dir.c | 29 +++++++++++++---------------- dlls/msvcrt/msvcrt.h | 17 +++++++++++++++++ dlls/ucrtbase/tests/file.c | 8 ++++++++ 3 files changed, 38 insertions(+), 16 deletions(-) diff --git a/dlls/msvcrt/dir.c b/dlls/msvcrt/dir.c index d574a4ae22e..abb3bc21388 100644 --- a/dlls/msvcrt/dir.c +++ b/dlls/msvcrt/dir.c @@ -787,24 +787,21 @@ int CDECL _wfindnext64i32(intptr_t hand, struct _wfinddata64i32_t * ft) */ char* CDECL _getcwd(char * buf, int size) { - char dir[MAX_PATH]; - int dir_len = GetCurrentDirectoryA(MAX_PATH,dir); + wchar_t dirW[MAX_PATH]; + int len; - if (dir_len < 1) - return NULL; /* FIXME: Real return value untested */ + if (!_wgetcwd(dirW, ARRAY_SIZE(dirW))) return NULL; - if (!buf) - { - if (size <= dir_len) size = dir_len + 1; - if (!(buf = malloc( size ))) return NULL; - } - else if (dir_len >= size) - { - *_errno() = ERANGE; - return NULL; /* buf too small */ - } - strcpy(buf,dir); - return buf; + if (!buf) return astrdupw_utf8(dirW); + len = convert_wcs_to_acp_utf8(dirW, NULL, 0); + if (!len) return NULL; + if (len > size) + { + *_errno() = ERANGE; + return NULL; + } + convert_wcs_to_acp_utf8(dirW, buf, size); + return buf; } /********************************************************************* diff --git a/dlls/msvcrt/msvcrt.h b/dlls/msvcrt/msvcrt.h index 71b0f9039f1..39349345de0 100644 --- a/dlls/msvcrt/msvcrt.h +++ b/dlls/msvcrt/msvcrt.h @@ -417,6 +417,11 @@ static inline int convert_acp_utf8_to_wcs(const char *str, wchar_t *wstr, int le return MultiByteToWideChar(CP_ACP, MB_PRECOMPOSED, str, -1, wstr, len); } +static inline int convert_wcs_to_acp_utf8(const wchar_t *wstr, char *str, int len) +{ + return WideCharToMultiByte(CP_ACP, 0, wstr, -1, str, len, NULL, NULL); +} + static inline wchar_t* wstrdupa_utf8(const char *str) { int len = convert_acp_utf8_to_wcs(str, NULL, 0); @@ -429,4 +434,16 @@ static inline wchar_t* wstrdupa_utf8(const char *str) return wstr; } +static inline char* astrdupw_utf8(const wchar_t *wstr) +{ + int len = convert_wcs_to_acp_utf8(wstr, NULL, 0); + char *str; + + if (!len) return NULL; + str = malloc(len * sizeof(char)); + if (!str) return NULL; + convert_wcs_to_acp_utf8(wstr, str, len); + return str; +} + #endif /* __WINE_MSVCRT_H */ diff --git a/dlls/ucrtbase/tests/file.c b/dlls/ucrtbase/tests/file.c index bb01a8a7b8f..1bae5e4263d 100644 --- a/dlls/ucrtbase/tests/file.c +++ b/dlls/ucrtbase/tests/file.c @@ -223,6 +223,7 @@ static void test_utf8(void) const char dir[] = "dir\xc4\x99\xc5\x9b\xc4\x87"; const WCHAR dirW[] = L"dir\x0119\x015b\x0107"; + char buf[256], *p; int ret; if (!setlocale(LC_ALL, ".utf8")) @@ -241,6 +242,13 @@ static void test_utf8(void) ret = _chdir(dir); ok(!ret, "_chdir returned %d, error %d\n", ret, errno); + + p = _getcwd(buf, sizeof(buf)); + ok(p == buf, "_getcwd returned %p, errno %d\n", p, errno); + p = strrchr(p, '\\'); + ok(!!p, "strrchr returned NULL, buf = %s\n", debugstr_a(buf)); + ok(!strcmp(p + 1, dir), "unexpected working directory: %s\n", debugstr_a(buf)); + ret = _chdir(".."); ok(!ret, "_chdir returned %d, error %d\n", ret, errno); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6937
From: Piotr Caban <piotr(a)codeweavers.com> --- dlls/msvcrt/dir.c | 40 +++++++++++--------------------------- dlls/ucrtbase/tests/file.c | 6 ++++++ 2 files changed, 17 insertions(+), 29 deletions(-) diff --git a/dlls/msvcrt/dir.c b/dlls/msvcrt/dir.c index abb3bc21388..b878a5ab1a9 100644 --- a/dlls/msvcrt/dir.c +++ b/dlls/msvcrt/dir.c @@ -869,39 +869,21 @@ int CDECL _getdrive(void) */ char* CDECL _getdcwd(int drive, char * buf, int size) { - static char* dummy; - - TRACE(":drive %d(%c), size %d\n",drive, drive + 'A' - 1, size); - - if (!drive || drive == _getdrive()) - return _getcwd(buf,size); /* current */ - else - { - char dir[MAX_PATH]; - char drivespec[] = "A:"; - int dir_len; + wchar_t dirW[MAX_PATH]; + int len; - drivespec[0] += drive - 1; - if (GetDriveTypeA(drivespec) < DRIVE_REMOVABLE) - { - *_errno() = EACCES; - return NULL; - } + if (!_wgetdcwd(drive, dirW, ARRAY_SIZE(dirW))) return NULL; - dir_len = GetFullPathNameA(drivespec,MAX_PATH,dir,&dummy); - if (dir_len >= size || dir_len < 1) + if (!buf) return astrdupw_utf8(dirW); + len = convert_wcs_to_acp_utf8(dirW, NULL, 0); + if (!len) return NULL; + if (len > size) { - *_errno() = ERANGE; - return NULL; /* buf too small */ + *_errno() = ERANGE; + return NULL; } - - TRACE(":returning '%s'\n", dir); - if (!buf) - return _strdup(dir); /* allocate */ - - strcpy(buf,dir); - } - return buf; + convert_wcs_to_acp_utf8(dirW, buf, size); + return buf; } /********************************************************************* diff --git a/dlls/ucrtbase/tests/file.c b/dlls/ucrtbase/tests/file.c index 1bae5e4263d..620745be1ef 100644 --- a/dlls/ucrtbase/tests/file.c +++ b/dlls/ucrtbase/tests/file.c @@ -249,6 +249,12 @@ static void test_utf8(void) ok(!!p, "strrchr returned NULL, buf = %s\n", debugstr_a(buf)); ok(!strcmp(p + 1, dir), "unexpected working directory: %s\n", debugstr_a(buf)); + p = _getdcwd(_getdrive(), buf, sizeof(buf)); + ok(p == buf, "_getcwd returned %p, errno %d\n", p, errno); + p = strrchr(p, '\\'); + ok(!!p, "strrchr returned NULL, buf = %s\n", debugstr_a(buf)); + ok(!strcmp(p + 1, dir), "unexpected working directory: %s\n", debugstr_a(buf)); + ret = _chdir(".."); ok(!ret, "_chdir returned %d, error %d\n", ret, errno); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6937
From: Piotr Caban <piotr(a)codeweavers.com> --- dlls/msvcrt/dir.c | 49 +++++++++++++------------------------- dlls/ucrtbase/tests/file.c | 6 +++++ 2 files changed, 22 insertions(+), 33 deletions(-) diff --git a/dlls/msvcrt/dir.c b/dlls/msvcrt/dir.c index b878a5ab1a9..ff26a0ce2b8 100644 --- a/dlls/msvcrt/dir.c +++ b/dlls/msvcrt/dir.c @@ -1296,43 +1296,26 @@ wchar_t * CDECL _wfullpath(wchar_t * absPath, const wchar_t* relPath, size_t siz * Otherwise populates absPath with the path and returns it. * Failure: NULL. errno indicates the error. */ -char * CDECL _fullpath(char * absPath, const char* relPath, size_t size) +char * CDECL _fullpath(char *abs_path, const char *rel_path, size_t size) { - DWORD rc; - char* lastpart; - char* buffer; - BOOL alloced = FALSE; - - if (!relPath || !*relPath) - return _getcwd(absPath, size); - - if (absPath == NULL) - { - buffer = malloc(MAX_PATH); - size = MAX_PATH; - alloced = TRUE; - } - else - buffer = absPath; - - if (size < 4) - { - *_errno() = ERANGE; - return NULL; - } - - TRACE(":resolving relative path '%s'\n",relPath); + wchar_t abs_pathW[MAX_PATH], *rel_pathW = NULL, *retW; + size_t len; - rc = GetFullPathNameA(relPath,size,buffer,&lastpart); + if (rel_path && !(rel_pathW = wstrdupa_utf8(rel_path))) return NULL; + retW = _wfullpath(abs_pathW, rel_pathW, ARRAY_SIZE(abs_pathW)); + free(rel_pathW); + if (!retW) return NULL; - if (rc > 0 && rc <= size) - return buffer; - else - { - if (alloced) - free(buffer); + if (!abs_path) return astrdupw_utf8(abs_pathW); + len = convert_wcs_to_acp_utf8(abs_pathW, NULL, 0); + if (!len) return NULL; + if (len > size) + { + *_errno() = ERANGE; return NULL; - } + } + convert_wcs_to_acp_utf8(abs_pathW, abs_path, size); + return abs_path; } /********************************************************************* diff --git a/dlls/ucrtbase/tests/file.c b/dlls/ucrtbase/tests/file.c index 620745be1ef..a81f23a549d 100644 --- a/dlls/ucrtbase/tests/file.c +++ b/dlls/ucrtbase/tests/file.c @@ -255,6 +255,12 @@ static void test_utf8(void) ok(!!p, "strrchr returned NULL, buf = %s\n", debugstr_a(buf)); ok(!strcmp(p + 1, dir), "unexpected working directory: %s\n", debugstr_a(buf)); + p = _fullpath(buf, NULL, sizeof(buf)); + ok(p == buf, "_fulpath returned %p, errno %d\n", p, errno); + p = strrchr(p, '\\'); + ok(!!p, "strrchr returned NULL, buf = %s\n", debugstr_a(buf)); + ok(!strcmp(p + 1, dir), "unexpected working directory: %s\n", debugstr_a(buf)); + ret = _chdir(".."); ok(!ret, "_chdir returned %d, error %d\n", ret, errno); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6937
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=150127 Your paranoid android. === debian11 (32 bit ja:JP report) === ucrtbase: file.c:250: Test failed: unexpected working directory: "C:\\users\\winetest\\AppData\\Local\\Temp\\wct\\dir\xc4\x99\xc5\x9b\xc4\x81E" file.c:256: Test failed: unexpected working directory: "C:\\users\\winetest\\AppData\\Local\\Temp\\wct\\dir\xc4\x99\xc5\x9b\xc4\x81E" file.c:262: Test failed: unexpected working directory: "C:\\users\\winetest\\AppData\\Local\\Temp\\wct\\dir\xc4\x99\xc5\x9b\xc4\x81E" === debian11b (64 bit WoW report) === user32: win.c:4070: Test failed: Expected active window 00000000024D016E, got 0000000000000000. win.c:4071: Test failed: Expected focus window 00000000024D016E, got 0000000000000000.
participants (3)
-
Marvin -
Piotr Caban -
Piotr Caban (＠piotr)