Signed-off-by: Stefan Dösinger stefan@codeweavers.com
---
This is a resend of the patch from months ago with a tautological check removed.
I tried to test what happens if the temp path is longer than MAX_PATH characters, but I could not get Windows to accept a long temp path. If %TMP% is longer than 100 characters (or somewhere in that area, I did not test the precise limit) it falls back to %TEMP%, %USERPROFILE% and finally "C:\Windows".
I assume msvcp does not clear the extra bytes like GetTempPathW either because it doesn't call GetTempPath and implements the logic itself or because it writes to a temp buffer and copies the data. --- dlls/msvcp140/msvcp140.spec | 2 +- dlls/msvcp140/tests/msvcp140.c | 26 ++++++++++++++++++++++++++ dlls/msvcp90/ios.c | 6 ++++++ 3 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/dlls/msvcp140/msvcp140.spec b/dlls/msvcp140/msvcp140.spec index e705e1eb84..57d02776c3 100644 --- a/dlls/msvcp140/msvcp140.spec +++ b/dlls/msvcp140/msvcp140.spec @@ -3718,7 +3718,7 @@ @ stub _Strxfrm @ cdecl _Symlink(wstr wstr) tr2_sys__Symlink_wchar @ stub _Symlink_get -@ stub _Temp_get +@ cdecl _Temp_get(ptr) @ stub _Thrd_abort @ cdecl _Thrd_create(ptr ptr ptr) _Thrd_create @ cdecl -norelay _Thrd_current() diff --git a/dlls/msvcp140/tests/msvcp140.c b/dlls/msvcp140/tests/msvcp140.c index 06ea11d55b..b1a5bfc9cf 100644 --- a/dlls/msvcp140/tests/msvcp140.c +++ b/dlls/msvcp140/tests/msvcp140.c @@ -178,6 +178,7 @@ static WCHAR* (__cdecl *p_Read_dir)(WCHAR*, void*, enum file_type*); static MSVCP_bool (__cdecl *p_Remove_dir)(WCHAR const*); static enum file_type (__cdecl *p_Stat)(WCHAR const *, int *); static int (__cdecl *p_Symlink)(WCHAR const*, WCHAR const*); +static WCHAR* (__cdecl *p_Temp_get)(WCHAR *); static int (__cdecl *p_To_byte)(const WCHAR *src, char *dst); static int (__cdecl *p_To_wide)(const char *src, WCHAR *dst); static int (__cdecl *p_Unlink)(WCHAR const*); @@ -260,6 +261,7 @@ static BOOL init(void) SET(p_Remove_dir, "_Remove_dir"); SET(p_Stat, "_Stat"); SET(p_Symlink, "_Symlink"); + SET(p_Temp_get, "_Temp_get"); SET(p_To_byte, "_To_byte"); SET(p_To_wide, "_To_wide"); SET(p_Unlink, "_Unlink"); @@ -1060,6 +1062,29 @@ static void test_Unlink(void) ok(SetCurrentDirectoryW(current_path), "SetCurrentDirectoryW failed\n"); }
+static void test_Temp_get(void) +{ + WCHAR path[MAX_PATH + 1], temp_path[MAX_PATH]; + WCHAR *retval; + DWORD len; + + GetTempPathW(sizeof(temp_path)/sizeof(*temp_path), temp_path); + + /* This crashes on Windows, the input pointer is not validated. */ + if (0) + { + retval = p_Temp_get(NULL); + ok(!retval, "_Temp_get(): Got %p\n", retval); + } + + memset(path, 0xaa, sizeof(path)); + retval = p_Temp_get(path); + ok(retval == path, "_Temp_get(): Got %p, expected %p\n", retval, path); + ok(!wcscmp(path, temp_path), "Expected path %s, got %s\n", wine_dbgstr_w(temp_path), wine_dbgstr_w(path)); + len = wcslen(path); + todo_wine ok(path[len + 1] == 0xaaaa, "Too many bytes were zeroed - %x\n", path[len + 1]); +} + START_TEST(msvcp140) { if(!init()) return; @@ -1078,5 +1103,6 @@ START_TEST(msvcp140) test_Stat(); test_dir_operation(); test_Unlink(); + test_Temp_get(); FreeLibrary(msvcp); } diff --git a/dlls/msvcp90/ios.c b/dlls/msvcp90/ios.c index 3f859979cb..ab9d7a44a4 100644 --- a/dlls/msvcp90/ios.c +++ b/dlls/msvcp90/ios.c @@ -15745,6 +15745,12 @@ enum file_type __cdecl _Lstat(WCHAR const* path, int* permissions) return _Stat(path, permissions); }
+WCHAR * __cdecl _Temp_get(WCHAR *dst) +{ + GetTempPathW(MAX_PATH, dst); + return dst; +} + /* ??1_Winit@std@@QAE@XZ */ /* ??1_Winit@std@@QAE@XZ */ DEFINE_THISCALL_WRAPPER(_Winit_dtor, 4)
Signed-off-by: Stefan Dösinger stefan@codeweavers.com --- dlls/msvcp120/tests/msvcp120.c | 58 +++++++++++++++++++++++++++++++++++++----- dlls/msvcp90/ios.c | 3 --- 2 files changed, 52 insertions(+), 9 deletions(-)
diff --git a/dlls/msvcp120/tests/msvcp120.c b/dlls/msvcp120/tests/msvcp120.c index a1308271c7..db9517d305 100644 --- a/dlls/msvcp120/tests/msvcp120.c +++ b/dlls/msvcp120/tests/msvcp120.c @@ -1474,9 +1474,8 @@ static void test_tr2_sys__Rename(void) BY_HANDLE_FILE_INFORMATION info1, info2; char temp_path[MAX_PATH], current_path[MAX_PATH]; LARGE_INTEGER file_size; - WCHAR testW[] = {'t','r','2','_','t','e','s','t','_','d','i','r','/','f','1',0}; - WCHAR testW2[] = {'t','r','2','_','t','e','s','t','_','d','i','r','/','f','w',0}; - struct { + static const WCHAR testW[] = {'t','r','2','_','t','e','s','t','_','d','i','r','/','f','1',0}; + static const struct { char const *old_path; char const *new_path; int val; @@ -1488,6 +1487,29 @@ static void test_tr2_sys__Rename(void) { "tr2_test_dir\f1_rename", "tr2_test_dir\??invalid_name>>", ERROR_INVALID_NAME }, { "tr2_test_dir\not_exist_file", "tr2_test_dir\not_exist_rename", ERROR_FILE_NOT_FOUND } }; + static const WCHAR f1_renameW[] = + {'t','r','2','_','t','e','s','t','_','d','i','r','\','f','1','_','r','e','n','a','m','e',0}; + static const WCHAR f1_rename2W[] = + {'t','r','2','_','t','e','s','t','_','d','i','r','\','f','1','_','r','e','n','a','m','e','2',0}; + static const WCHAR not_existW[] = + {'t','r','2','_','t','e','s','t','_','d','i','r','\','n','o','t','_','e','x','i','s','t',0}; + static const WCHAR not_exist2W[] = + {'t','r','2','_','t','e','s','t','_','d','i','r','\','n','o','t','_','e','x','i','s','t','2',0}; + static const WCHAR invalidW[] = + {'t','r','2','_','t','e','s','t','_','d','i','r','\','?','?','i','n','v','a','l','i','d','>',0}; + static const struct { + const WCHAR *old_path; + const WCHAR *new_path; + int val; + } testsW[] = { + { testW, f1_renameW, ERROR_SUCCESS }, + { testW, NULL, ERROR_FILE_NOT_FOUND }, /* Differs from the A version */ + { testW, f1_renameW, ERROR_FILE_NOT_FOUND }, + { NULL, f1_rename2W, ERROR_PATH_NOT_FOUND }, /* Differs from the A version */ + { f1_renameW, invalidW, ERROR_INVALID_NAME }, + { not_existW, not_exist2W, ERROR_FILE_NOT_FOUND }, + { not_existW, invalidW, ERROR_FILE_NOT_FOUND } + };
memset(current_path, 0, MAX_PATH); GetCurrentDirectoryA(MAX_PATH, current_path); @@ -1538,10 +1560,34 @@ static void test_tr2_sys__Rename(void) ok(ret == ERROR_ALREADY_EXISTS, "test_tr2_sys__Rename(): expect: ERROR_ALREADY_EXISTS, got %d\n", ret); ok(p_tr2_sys__File_size("tr2_test_dir\f1") == 7, "test_tr2_sys__Rename(): expect: 7, got %s\n", wine_dbgstr_longlong(p_tr2_sys__File_size("tr2_test_dir\f1"))); ok(p_tr2_sys__File_size("tr2_test_dir\f1_rename") == 0, "test_tr2_sys__Rename(): expect: 0, got %s\n",wine_dbgstr_longlong(p_tr2_sys__File_size("tr2_test_dir\f1_rename"))); - ret = p_tr2_sys__Rename_wchar(testW, testW2); - ok(ret == ERROR_SUCCESS, "tr2_sys__Rename_wchar(): expect: ERROR_SUCCESS, got %d\n", ret);
- ok(DeleteFileW(testW2), "expect fw to exist\n"); + ok(DeleteFileA("tr2_test_dir\f1_rename"), "expect f1_rename to exist\n"); + + for(i=0; i<ARRAY_SIZE(testsW); i++) { + errno = 0xdeadbeef; + if(testsW[i].val == ERROR_SUCCESS) { + h1 = CreateFileW(testsW[i].old_path, 0, FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE, + NULL, OPEN_EXISTING, 0, 0); + ok(h1 != INVALID_HANDLE_VALUE, "create file failed: INVALID_HANDLE_VALUE\n"); + ok(GetFileInformationByHandle(h1, &info1), "GetFileInformationByHandle failed\n"); + CloseHandle(h1); + } + SetLastError(0xdeadbeef); + ret = p_tr2_sys__Rename_wchar(testsW[i].old_path, testsW[i].new_path); + ok(ret == testsW[i].val, "test_tr2_sys__Rename_wchar(): test %d expect: %d, got %d\n", i+1, testsW[i].val, ret); + ok(errno == 0xdeadbeef, "test_tr2_sys__Rename_wchar(): test %d errno expect 0xdeadbeef, got %d\n", i+1, errno); + if(ret == ERROR_SUCCESS) { + h2 = CreateFileW(testsW[i].new_path, 0, FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE, + NULL, OPEN_EXISTING, 0, 0); + ok(h2 != INVALID_HANDLE_VALUE, "create file failed: INVALID_HANDLE_VALUE\n"); + ok(GetFileInformationByHandle(h2, &info2), "GetFileInformationByHandle failed\n"); + CloseHandle(h2); + ok(info1.nFileIndexHigh == info2.nFileIndexHigh + && info1.nFileIndexLow == info2.nFileIndexLow, + "test_tr2_sys__Rename_wchar(): test %d expect two files equivalent\n", i+1); + } + } + ok(DeleteFileA("tr2_test_dir\f1_rename"), "expect f1_rename to exist\n"); ret = p_tr2_sys__Remove_dir("tr2_test_dir"); ok(ret == 1, "test_tr2_sys__Remove_dir(): expect %d got %d\n", 1, ret); diff --git a/dlls/msvcp90/ios.c b/dlls/msvcp90/ios.c index ab9d7a44a4..cd745c2774 100644 --- a/dlls/msvcp90/ios.c +++ b/dlls/msvcp90/ios.c @@ -15641,9 +15641,6 @@ int __cdecl tr2_sys__Rename_wchar(WCHAR const* old_path, WCHAR const* new_path) { TRACE("(%s %s)\n", debugstr_w(old_path), debugstr_w(new_path));
- if(!old_path || !new_path) - return ERROR_INVALID_PARAMETER; - if(MoveFileExW(old_path, new_path, MOVEFILE_COPY_ALLOWED)) return ERROR_SUCCESS; return GetLastError();
Signed-off-by: Piotr Caban piotr@codeweavers.com
Signed-off-by: Stefan Dösinger stefan@codeweavers.com
---
And port the tests to show that it behaves like the msvcp120 version. --- dlls/msvcp140/msvcp140.spec | 2 +- dlls/msvcp140/tests/msvcp140.c | 97 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 1 deletion(-)
diff --git a/dlls/msvcp140/msvcp140.spec b/dlls/msvcp140/msvcp140.spec index 57d02776c3..f999e9acef 100644 --- a/dlls/msvcp140/msvcp140.spec +++ b/dlls/msvcp140/msvcp140.spec @@ -3694,7 +3694,7 @@ @ cdecl -ret64 _Query_perf_frequency() @ cdecl _Read_dir(ptr ptr ptr) tr2_sys__Read_dir_wchar @ cdecl _Remove_dir(wstr) tr2_sys__Remove_dir_wchar -@ stub _Rename +@ cdecl _Rename(wstr wstr) tr2_sys__Rename_wchar @ stub _Resize @ stub _Set_last_write_time @ stub _Sinh diff --git a/dlls/msvcp140/tests/msvcp140.c b/dlls/msvcp140/tests/msvcp140.c index b1a5bfc9cf..caa7fa7782 100644 --- a/dlls/msvcp140/tests/msvcp140.c +++ b/dlls/msvcp140/tests/msvcp140.c @@ -176,6 +176,7 @@ static int (__cdecl *p_Make_dir)(WCHAR const*); static void* (__cdecl *p_Open_dir)(WCHAR*, WCHAR const*, int *, enum file_type*); static WCHAR* (__cdecl *p_Read_dir)(WCHAR*, void*, enum file_type*); static MSVCP_bool (__cdecl *p_Remove_dir)(WCHAR const*); +static int (__cdecl *p_Rename)(WCHAR const*, WCHAR const*); static enum file_type (__cdecl *p_Stat)(WCHAR const *, int *); static int (__cdecl *p_Symlink)(WCHAR const*, WCHAR const*); static WCHAR* (__cdecl *p_Temp_get)(WCHAR *); @@ -259,6 +260,7 @@ static BOOL init(void) SET(p_Open_dir, "_Open_dir"); SET(p_Read_dir, "_Read_dir"); SET(p_Remove_dir, "_Remove_dir"); + SET(p_Rename, "_Rename"); SET(p_Stat, "_Stat"); SET(p_Symlink, "_Symlink"); SET(p_Temp_get, "_Temp_get"); @@ -1085,6 +1087,100 @@ static void test_Temp_get(void) todo_wine ok(path[len + 1] == 0xaaaa, "Too many bytes were zeroed - %x\n", path[len + 1]); }
+static void test_Rename(void) +{ + int ret, i; + HANDLE file, h1, h2; + BY_HANDLE_FILE_INFORMATION info1, info2; + WCHAR temp_path[MAX_PATH], current_path[MAX_PATH]; + LARGE_INTEGER file_size; + static const WCHAR wine_test_dirW[] = + {'w','i','n','e','_','t','e','s','t','_','d','i','r',0}; + static const WCHAR f1W[] = + {'w','i','n','e','_','t','e','s','t','_','d','i','r','\','f','1',0}; + static const WCHAR f1_renameW[] = + {'w','i','n','e','_','t','e','s','t','_','d','i','r','\','f','1','_','r','e','n','a','m','e',0}; + static const WCHAR f1_rename2W[] = + {'w','i','n','e','_','t','e','s','t','_','d','i','r','\','f','1','_','r','e','n','a','m','e','2',0}; + static const WCHAR not_existW[] = + {'w','i','n','e','_','t','e','s','t','_','d','i','r','\','n','o','t','_','e','x','i','s','t',0}; + static const WCHAR not_exist2W[] = + {'w','i','n','e','_','t','e','s','t','_','d','i','r','\','n','o','t','_','e','x','i','s','t','2',0}; + static const WCHAR invalidW[] = + {'w','i','n','e','_','t','e','s','t','_','d','i','r','\','?','?','i','n','v','a','l','i','d','>',0}; + static const struct { + const WCHAR *old_path; + const WCHAR *new_path; + int val; + } tests[] = { + { f1W, f1_renameW, ERROR_SUCCESS }, + { f1W, NULL, ERROR_FILE_NOT_FOUND }, + { f1W, f1_renameW, ERROR_FILE_NOT_FOUND }, + { NULL, f1_rename2W, ERROR_PATH_NOT_FOUND }, + { f1_renameW, invalidW, ERROR_INVALID_NAME }, + { not_existW, not_exist2W, ERROR_FILE_NOT_FOUND }, + { not_existW, invalidW, ERROR_FILE_NOT_FOUND } + }; + + memset(current_path, 0, MAX_PATH); + GetCurrentDirectoryW(MAX_PATH, current_path); + memset(temp_path, 0, MAX_PATH); + GetTempPathW(MAX_PATH, temp_path); + ok(SetCurrentDirectoryW(temp_path), "SetCurrentDirectoryW to temp_path failed\n"); + ret = p_Make_dir(wine_test_dirW); + + ok(ret == 1, "_Make_dir(): expect 1 got %d\n", ret); + file = CreateFileW(f1W, GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, 0, NULL); + ok(file != INVALID_HANDLE_VALUE, "create file failed: INVALID_HANDLE_VALUE\n"); + CloseHandle(file); + + ret = p_Rename(f1W, f1W); + todo_wine ok(ERROR_SUCCESS == ret, "_Rename(): expect: ERROR_SUCCESS, got %d\n", ret); + for(i=0; i<ARRAY_SIZE(tests); i++) { + errno = 0xdeadbeef; + if(tests[i].val == ERROR_SUCCESS) { + h1 = CreateFileW(tests[i].old_path, 0, FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE, + NULL, OPEN_EXISTING, 0, 0); + ok(h1 != INVALID_HANDLE_VALUE, "create file failed: INVALID_HANDLE_VALUE\n"); + ok(GetFileInformationByHandle(h1, &info1), "GetFileInformationByHandle failed\n"); + CloseHandle(h1); + } + SetLastError(0xdeadbeef); + ret = p_Rename(tests[i].old_path, tests[i].new_path); + ok(ret == tests[i].val, "_Rename(): test %d expect: %d, got %d\n", i+1, tests[i].val, ret); + ok(errno == 0xdeadbeef, "_Rename(): test %d errno expect 0xdeadbeef, got %d\n", i+1, errno); + if(ret == ERROR_SUCCESS) { + h2 = CreateFileW(tests[i].new_path, 0, FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE, + NULL, OPEN_EXISTING, 0, 0); + ok(h2 != INVALID_HANDLE_VALUE, "create file failed: INVALID_HANDLE_VALUE\n"); + ok(GetFileInformationByHandle(h2, &info2), "GetFileInformationByHandle failed\n"); + CloseHandle(h2); + ok(info1.nFileIndexHigh == info2.nFileIndexHigh + && info1.nFileIndexLow == info2.nFileIndexLow, + "test_tr2_sys__Rename(): test %d expect two files equivalent\n", i+1); + } + } + + file = CreateFileW(f1W, GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, 0, NULL); + ok(file != INVALID_HANDLE_VALUE, "create file failed: INVALID_HANDLE_VALUE\n"); + file_size.QuadPart = 7; + ok(SetFilePointerEx(file, file_size, NULL, FILE_BEGIN), "SetFilePointerEx failed\n"); + ok(SetEndOfFile(file), "SetEndOfFile failed\n"); + CloseHandle(file); + ret = p_Rename(f1W, f1_renameW); + ok(ret == ERROR_ALREADY_EXISTS, "_Rename(): expect: ERROR_ALREADY_EXISTS, got %d\n", ret); + ok(p_File_size(f1W) == 7, "_Rename(): expect: 7, got %s\n", + wine_dbgstr_longlong(p_File_size(f1W))); + ok(p_File_size(f1_renameW) == 0, "test_tr2_sys__Rename(): expect: 0, got %s\n", + wine_dbgstr_longlong(p_File_size(f1_renameW))); + + ok(DeleteFileW(f1_renameW), "expect f1_rename to exist\n"); + ok(DeleteFileW(f1W), "expect f1 to exist\n"); + ret = p_Remove_dir(wine_test_dirW); + ok(ret == 1, "_Remove_dir(): expect %d got %d\n", 1, ret); + ok(SetCurrentDirectoryW(current_path), "SetCurrentDirectoryW failed\n"); +} + START_TEST(msvcp140) { if(!init()) return; @@ -1104,5 +1200,6 @@ START_TEST(msvcp140) test_dir_operation(); test_Unlink(); test_Temp_get(); + test_Rename(); FreeLibrary(msvcp); }
Signed-off-by: Piotr Caban piotr@codeweavers.com
On Thu, 19 Jul 2018 at 15:14, Stefan Dösinger stefan@codeweavers.com wrote:
@@ -1060,6 +1062,29 @@ static void test_Unlink(void) ok(SetCurrentDirectoryW(current_path), "SetCurrentDirectoryW failed\n"); }
+static void test_Temp_get(void) +{
- WCHAR path[MAX_PATH + 1], temp_path[MAX_PATH];
- WCHAR *retval;
- DWORD len;
- GetTempPathW(sizeof(temp_path)/sizeof(*temp_path), temp_path);
What about using ARRAY_SIZE() or MAX_PATH here?
-- Mathew Hodson
Am 2018-07-20 um 02:05 schrieb Mathew Hodson:
What about using ARRAY_SIZE() or MAX_PATH here?
Good catch, this patch has been sitting on my disk since before we had ARRAY_SIZE everywhere. I've resent it.
-
Mathew Hodson -
Piotr Caban -
Stefan Dösinger