[PATCH v8 0/8] MR391: scrrun: Implement MoveFolder().
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52128 Signed-off-by: Robert Wilhelm <robert.wilhelm(a)gmx.net> -- v8: scrrun: Move directories only in MoveFolder(). scrrun: Support wildcards in MoveFolder(). scrrun: Move source dir into destination dir if destination ends with separator in MoveFolder(). scrrun: Check that source is directory in MoveFolder(). scrrun: Check for non-existant source in MoveFolder(). https://gitlab.winehq.org/wine/wine/-/merge_requests/391
From: Robert Wilhelm <robert.wilhelm(a)gmx.net> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52128 Signed-off-by: Robert Wilhelm <robert.wilhelm(a)gmx.net> --- dlls/scrrun/filesystem.c | 7 +++---- dlls/scrrun/tests/filesystem.c | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/dlls/scrrun/filesystem.c b/dlls/scrrun/filesystem.c index 35b43e2f124..dac3b9fdcf7 100644 --- a/dlls/scrrun/filesystem.c +++ b/dlls/scrrun/filesystem.c @@ -3764,12 +3764,11 @@ static HRESULT WINAPI filesys_MoveFile(IFileSystem3 *iface, BSTR source, BSTR de return MoveFileW(source, destination) ? S_OK : create_error(GetLastError()); } -static HRESULT WINAPI filesys_MoveFolder(IFileSystem3 *iface,BSTR Source, - BSTR Destination) +static HRESULT WINAPI filesys_MoveFolder(IFileSystem3 *iface, BSTR source, BSTR destination) { - FIXME("%p %s %s\n", iface, debugstr_w(Source), debugstr_w(Destination)); + TRACE("%p %s %s\n", iface, debugstr_w(source), debugstr_w(destination)); - return E_NOTIMPL; + return MoveFileW(source, destination) ? S_OK : create_error(GetLastError()); } static inline HRESULT copy_file(const WCHAR *source, DWORD source_len, diff --git a/dlls/scrrun/tests/filesystem.c b/dlls/scrrun/tests/filesystem.c index 1ba0e81a2d1..7654b07b003 100644 --- a/dlls/scrrun/tests/filesystem.c +++ b/dlls/scrrun/tests/filesystem.c @@ -2600,6 +2600,25 @@ static void test_MoveFile(void) SysFreeString(str); } +static void test_MoveFolder(void) +{ + BSTR src, dst; + WCHAR buffW1[MAX_PATH],buffW2[MAX_PATH]; + HRESULT hr; + + get_temp_path(L"foo", buffW1); + get_temp_path(L"bar", buffW2); + + ok(CreateDirectoryW(buffW1, NULL), "CreateDirectory(%s) failed\n", wine_dbgstr_w(buffW1)); + src = SysAllocString(buffW1); + dst = SysAllocString(buffW2); + hr = IFileSystem3_MoveFolder(fs3, src, dst); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + SysFreeString(src); + SysFreeString(dst); + ok(RemoveDirectoryW(buffW2), "can't remove %s directory\n", wine_dbgstr_w(buffW2)); +} + static void test_DoOpenPipeStream(void) { static const char testdata[] = "test"; @@ -2733,6 +2752,7 @@ START_TEST(filesystem) test_GetExtensionName(); test_GetSpecialFolder(); test_MoveFile(); + test_MoveFolder(); test_DoOpenPipeStream(); IFileSystem3_Release(fs3); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/391
From: Robert Wilhelm <robert.wilhelm(a)gmx.net> Signed-off-by: Robert Wilhelm <robert.wilhelm(a)gmx.net> --- dlls/scrrun/filesystem.c | 3 +++ dlls/scrrun/tests/filesystem.c | 15 ++++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/dlls/scrrun/filesystem.c b/dlls/scrrun/filesystem.c index dac3b9fdcf7..52e10239e05 100644 --- a/dlls/scrrun/filesystem.c +++ b/dlls/scrrun/filesystem.c @@ -3768,6 +3768,9 @@ static HRESULT WINAPI filesys_MoveFolder(IFileSystem3 *iface, BSTR source, BSTR { TRACE("%p %s %s\n", iface, debugstr_w(source), debugstr_w(destination)); + if(!source || !source[0] || !destination || !destination[0]) + return E_INVALIDARG; + return MoveFileW(source, destination) ? S_OK : create_error(GetLastError()); } diff --git a/dlls/scrrun/tests/filesystem.c b/dlls/scrrun/tests/filesystem.c index 7654b07b003..48560a90a63 100644 --- a/dlls/scrrun/tests/filesystem.c +++ b/dlls/scrrun/tests/filesystem.c @@ -2602,7 +2602,7 @@ static void test_MoveFile(void) static void test_MoveFolder(void) { - BSTR src, dst; + BSTR src, dst, str, empty; WCHAR buffW1[MAX_PATH],buffW2[MAX_PATH]; HRESULT hr; @@ -2617,6 +2617,19 @@ static void test_MoveFolder(void) SysFreeString(src); SysFreeString(dst); ok(RemoveDirectoryW(buffW2), "can't remove %s directory\n", wine_dbgstr_w(buffW2)); + + str = SysAllocString(L"null.dir"); + empty = SysAllocString(L""); + hr = IFileSystem3_MoveFolder(fs3, str, NULL); + ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr); + hr = IFileSystem3_MoveFolder(fs3, NULL, str); + ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr); + hr = IFileSystem3_MoveFolder(fs3, str, empty); + ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr); + hr = IFileSystem3_MoveFolder(fs3, empty, str); + ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr); + SysFreeString(str); + SysFreeString(empty); } static void test_DoOpenPipeStream(void) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/391
From: Robert Wilhelm <robert.wilhelm(a)gmx.net> Signed-off-by: Robert Wilhelm <robert.wilhelm(a)gmx.net> --- dlls/scrrun/tests/filesystem.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/dlls/scrrun/tests/filesystem.c b/dlls/scrrun/tests/filesystem.c index 48560a90a63..5b183b9436e 100644 --- a/dlls/scrrun/tests/filesystem.c +++ b/dlls/scrrun/tests/filesystem.c @@ -2630,6 +2630,17 @@ static void test_MoveFolder(void) ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr); SysFreeString(str); SysFreeString(empty); + + ok(CreateDirectoryW(buffW1, NULL), "CreateDirectory(%s) failed\n", wine_dbgstr_w(buffW1)); + ok(CreateDirectoryW(buffW2, NULL), "CreateDirectory(%s) failed\n", wine_dbgstr_w(buffW2)); + src = SysAllocString(buffW1); + dst = SysAllocString(buffW2); + hr = IFileSystem3_MoveFolder(fs3, src, dst); /* dst already exists */ + ok(hr == CTL_E_FILEALREADYEXISTS, "Unexpected hr %#lx.\n", hr); + SysFreeString(src); + SysFreeString(dst); + ok(RemoveDirectoryW(buffW1), "can't remove %s directory\n", wine_dbgstr_w(buffW1)); + ok(RemoveDirectoryW(buffW2), "can't remove %s directory\n", wine_dbgstr_w(buffW2)); } static void test_DoOpenPipeStream(void) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/391
From: Robert Wilhelm <robert.wilhelm(a)gmx.net> Signed-off-by: Robert Wilhelm <robert.wilhelm(a)gmx.net> --- dlls/scrrun/filesystem.c | 16 +++++++++++++++- dlls/scrrun/tests/filesystem.c | 7 +++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/dlls/scrrun/filesystem.c b/dlls/scrrun/filesystem.c index 52e10239e05..08db360bbe6 100644 --- a/dlls/scrrun/filesystem.c +++ b/dlls/scrrun/filesystem.c @@ -3764,6 +3764,20 @@ static HRESULT WINAPI filesys_MoveFile(IFileSystem3 *iface, BSTR source, BSTR de return MoveFileW(source, destination) ? S_OK : create_error(GetLastError()); } +static inline HRESULT create_movefolder_error(DWORD err) +{ + switch(err) { + case ERROR_FILE_NOT_FOUND: + case ERROR_PATH_NOT_FOUND: return CTL_E_PATHNOTFOUND; + case ERROR_ACCESS_DENIED: return CTL_E_PERMISSIONDENIED; + case ERROR_FILE_EXISTS: return CTL_E_FILEALREADYEXISTS; + case ERROR_ALREADY_EXISTS: return CTL_E_FILEALREADYEXISTS; + default: + FIXME("Unsupported error code: %ld\n", err); + return E_FAIL; + } +} + static HRESULT WINAPI filesys_MoveFolder(IFileSystem3 *iface, BSTR source, BSTR destination) { TRACE("%p %s %s\n", iface, debugstr_w(source), debugstr_w(destination)); @@ -3771,7 +3785,7 @@ static HRESULT WINAPI filesys_MoveFolder(IFileSystem3 *iface, BSTR source, BSTR if(!source || !source[0] || !destination || !destination[0]) return E_INVALIDARG; - return MoveFileW(source, destination) ? S_OK : create_error(GetLastError()); + return MoveFileW(source, destination) ? S_OK : create_movefolder_error(GetLastError()); } static inline HRESULT copy_file(const WCHAR *source, DWORD source_len, diff --git a/dlls/scrrun/tests/filesystem.c b/dlls/scrrun/tests/filesystem.c index 5b183b9436e..8b84e743809 100644 --- a/dlls/scrrun/tests/filesystem.c +++ b/dlls/scrrun/tests/filesystem.c @@ -2641,6 +2641,13 @@ static void test_MoveFolder(void) SysFreeString(dst); ok(RemoveDirectoryW(buffW1), "can't remove %s directory\n", wine_dbgstr_w(buffW1)); ok(RemoveDirectoryW(buffW2), "can't remove %s directory\n", wine_dbgstr_w(buffW2)); + + src = SysAllocString(buffW1); + dst = SysAllocString(buffW2); + hr = IFileSystem3_MoveFolder(fs3, src, dst); /* src nonexistant */ + ok(hr == CTL_E_PATHNOTFOUND, "Unexpected hr %#lx.\n", hr); + SysFreeString(src); + SysFreeString(dst); } static void test_DoOpenPipeStream(void) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/391
From: Robert Wilhelm <robert.wilhelm(a)gmx.net> Signed-off-by: Robert Wilhelm <robert.wilhelm(a)gmx.net> --- dlls/scrrun/filesystem.c | 10 +++++++++- dlls/scrrun/tests/filesystem.c | 14 ++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/dlls/scrrun/filesystem.c b/dlls/scrrun/filesystem.c index 08db360bbe6..a7488454e95 100644 --- a/dlls/scrrun/filesystem.c +++ b/dlls/scrrun/filesystem.c @@ -3767,6 +3767,7 @@ static HRESULT WINAPI filesys_MoveFile(IFileSystem3 *iface, BSTR source, BSTR de static inline HRESULT create_movefolder_error(DWORD err) { switch(err) { + case ERROR_INVALID_NAME: case ERROR_FILE_NOT_FOUND: case ERROR_PATH_NOT_FOUND: return CTL_E_PATHNOTFOUND; case ERROR_ACCESS_DENIED: return CTL_E_PERMISSIONDENIED; @@ -3780,12 +3781,19 @@ static inline HRESULT create_movefolder_error(DWORD err) static HRESULT WINAPI filesys_MoveFolder(IFileSystem3 *iface, BSTR source, BSTR destination) { + int len; + WCHAR src_path[MAX_PATH]; + TRACE("%p %s %s\n", iface, debugstr_w(source), debugstr_w(destination)); if(!source || !source[0] || !destination || !destination[0]) return E_INVALIDARG; - return MoveFileW(source, destination) ? S_OK : create_movefolder_error(GetLastError()); + len = SysStringLen(source); + lstrcpyW(src_path, source); + if (source[len-1] != '\\' && source[len-1] != '/') wcscat(src_path, L"\\"); + + return MoveFileW(src_path, destination) ? S_OK : create_movefolder_error(GetLastError()); } static inline HRESULT copy_file(const WCHAR *source, DWORD source_len, diff --git a/dlls/scrrun/tests/filesystem.c b/dlls/scrrun/tests/filesystem.c index 8b84e743809..0cd8cf2c85d 100644 --- a/dlls/scrrun/tests/filesystem.c +++ b/dlls/scrrun/tests/filesystem.c @@ -2605,6 +2605,7 @@ static void test_MoveFolder(void) BSTR src, dst, str, empty; WCHAR buffW1[MAX_PATH],buffW2[MAX_PATH]; HRESULT hr; + HANDLE *file; get_temp_path(L"foo", buffW1); get_temp_path(L"bar", buffW2); @@ -2648,6 +2649,19 @@ static void test_MoveFolder(void) ok(hr == CTL_E_PATHNOTFOUND, "Unexpected hr %#lx.\n", hr); SysFreeString(src); SysFreeString(dst); + + file = CreateFileW(buffW1, GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, + FILE_ATTRIBUTE_NORMAL, NULL); + ok(file != INVALID_HANDLE_VALUE, "CreateFile failed\n"); + CloseHandle(file); + + src = SysAllocString(buffW1); + dst = SysAllocString(buffW2); + hr = IFileSystem3_MoveFolder(fs3, src, dst); /* src is regular file */ + ok(hr == CTL_E_PATHNOTFOUND, "Unexpected hr %#lx.\n", hr); + SysFreeString(src); + SysFreeString(dst); + DeleteFileW(buffW1); } static void test_DoOpenPipeStream(void) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/391
From: Robert Wilhelm <robert.wilhelm(a)gmx.net> Signed-off-by: Robert Wilhelm <robert.wilhelm(a)gmx.net> --- dlls/scrrun/filesystem.c | 16 ++++++++++++-- dlls/scrrun/tests/filesystem.c | 38 +++++++++++++++++++++++++++++++++- 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/dlls/scrrun/filesystem.c b/dlls/scrrun/filesystem.c index a7488454e95..f9f136a4445 100644 --- a/dlls/scrrun/filesystem.c +++ b/dlls/scrrun/filesystem.c @@ -3781,18 +3781,30 @@ static inline HRESULT create_movefolder_error(DWORD err) static HRESULT WINAPI filesys_MoveFolder(IFileSystem3 *iface, BSTR source, BSTR destination) { - int len; - WCHAR src_path[MAX_PATH]; + int len, dst_len; + WCHAR src_path[MAX_PATH], dst_path[MAX_PATH]; + WCHAR *filename; TRACE("%p %s %s\n", iface, debugstr_w(source), debugstr_w(destination)); if(!source || !source[0] || !destination || !destination[0]) return E_INVALIDARG; + if (!GetFullPathNameW(source, MAX_PATH, src_path, &filename)) + return E_FAIL; + len = SysStringLen(source); lstrcpyW(src_path, source); if (source[len-1] != '\\' && source[len-1] != '/') wcscat(src_path, L"\\"); + dst_len = lstrlenW(destination); + if (destination[dst_len-1] == '\\' || destination[dst_len-1] == '/') { + lstrcpyW(dst_path, destination); + lstrcatW(dst_path, filename); + TRACE("move %s to %s\n", debugstr_w(src_path), debugstr_w(dst_path)); + return MoveFileW(src_path, dst_path) ? S_OK : create_error(GetLastError()); + } + TRACE("move %s to %s\n", debugstr_w(src_path), debugstr_w(destination)); return MoveFileW(src_path, destination) ? S_OK : create_movefolder_error(GetLastError()); } diff --git a/dlls/scrrun/tests/filesystem.c b/dlls/scrrun/tests/filesystem.c index 0cd8cf2c85d..6d1e69ce09e 100644 --- a/dlls/scrrun/tests/filesystem.c +++ b/dlls/scrrun/tests/filesystem.c @@ -2603,7 +2603,7 @@ static void test_MoveFile(void) static void test_MoveFolder(void) { BSTR src, dst, str, empty; - WCHAR buffW1[MAX_PATH],buffW2[MAX_PATH]; + WCHAR buffW1[MAX_PATH],buffW2[MAX_PATH],pathW[MAX_PATH]; HRESULT hr; HANDLE *file; @@ -2662,6 +2662,42 @@ static void test_MoveFolder(void) SysFreeString(src); SysFreeString(dst); DeleteFileW(buffW1); + + GetTempPathW(MAX_PATH, buffW1); + lstrcatW(buffW1,L"foo"); + GetTempPathW(MAX_PATH, buffW2); + lstrcatW(buffW2,L"bar"); + ok(CreateDirectoryW(buffW1, NULL), "CreateDirectory(%s) failed\n", wine_dbgstr_w(buffW1)); + ok(CreateDirectoryW(buffW2, NULL), "CreateDirectory(%s) failed\n", wine_dbgstr_w(buffW2)); + lstrcpyW(pathW,buffW2); + lstrcatW(pathW,L"\\"); + src = SysAllocString(buffW1); + dst = SysAllocString(pathW); + hr = IFileSystem3_MoveFolder(fs3, src, dst); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + SysFreeString(src); + SysFreeString(dst); + lstrcatW(pathW,L"foo"); + ok(RemoveDirectoryW(pathW), "can't remove %s directory\n", wine_dbgstr_w(pathW)); + ok(RemoveDirectoryW(buffW2), "can't remove %s directory\n", wine_dbgstr_w(buffW2)); + + GetTempPathW(MAX_PATH, buffW1); + lstrcatW(buffW1,L"foo"); + GetTempPathW(MAX_PATH, buffW2); + lstrcatW(buffW2,L"bar"); + ok(CreateDirectoryW(buffW1, NULL), "CreateDirectory(%s) failed\n", wine_dbgstr_w(buffW1)); + ok(CreateDirectoryW(buffW2, NULL), "CreateDirectory(%s) failed\n", wine_dbgstr_w(buffW2)); + lstrcpyW(pathW,buffW2); + lstrcatW(pathW,L"/"); + src = SysAllocString(buffW1); + dst = SysAllocString(pathW); + hr = IFileSystem3_MoveFolder(fs3, src, dst); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + SysFreeString(src); + SysFreeString(dst); + lstrcatW(pathW,L"foo"); + ok(RemoveDirectoryW(pathW), "can't remove %s directory\n", wine_dbgstr_w(pathW)); + ok(RemoveDirectoryW(buffW2), "can't remove %s directory\n", wine_dbgstr_w(buffW2)); } static void test_DoOpenPipeStream(void) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/391
From: Robert Wilhelm <robert.wilhelm(a)gmx.net> Signed-off-by: Robert Wilhelm <robert.wilhelm(a)gmx.net> --- dlls/scrrun/filesystem.c | 46 ++++++++++++++++++++++++++++------ dlls/scrrun/tests/filesystem.c | 27 +++++++++++++++++++- 2 files changed, 64 insertions(+), 9 deletions(-) diff --git a/dlls/scrrun/filesystem.c b/dlls/scrrun/filesystem.c index f9f136a4445..4ec2511a811 100644 --- a/dlls/scrrun/filesystem.c +++ b/dlls/scrrun/filesystem.c @@ -3781,9 +3781,12 @@ static inline HRESULT create_movefolder_error(DWORD err) static HRESULT WINAPI filesys_MoveFolder(IFileSystem3 *iface, BSTR source, BSTR destination) { - int len, dst_len; + int len, src_len, dst_len, name_len; WCHAR src_path[MAX_PATH], dst_path[MAX_PATH]; WCHAR *filename; + WIN32_FIND_DATAW ffd; + HANDLE f; + BOOL wildcard = FALSE, separator = FALSE; TRACE("%p %s %s\n", iface, debugstr_w(source), debugstr_w(destination)); @@ -3792,20 +3795,47 @@ static HRESULT WINAPI filesys_MoveFolder(IFileSystem3 *iface, BSTR source, BSTR if (!GetFullPathNameW(source, MAX_PATH, src_path, &filename)) return E_FAIL; + src_len = (filename - src_path) / sizeof(WCHAR); + + if (wcspbrk(filename,L"*?")) + wildcard = TRUE; len = SysStringLen(source); lstrcpyW(src_path, source); if (source[len-1] != '\\' && source[len-1] != '/') wcscat(src_path, L"\\"); dst_len = lstrlenW(destination); - if (destination[dst_len-1] == '\\' || destination[dst_len-1] == '/') { - lstrcpyW(dst_path, destination); - lstrcatW(dst_path, filename); - TRACE("move %s to %s\n", debugstr_w(src_path), debugstr_w(dst_path)); - return MoveFileW(src_path, dst_path) ? S_OK : create_error(GetLastError()); + + if (destination[dst_len-1] == '\\' || destination[dst_len-1] == '/') + separator = TRUE; + + if (!wildcard && !separator) { + TRACE("move %s to %s\n", debugstr_w(src_path), debugstr_w(destination)); + return MoveFileW(src_path, destination) ? S_OK : create_movefolder_error(GetLastError()); } - TRACE("move %s to %s\n", debugstr_w(src_path), debugstr_w(destination)); - return MoveFileW(src_path, destination) ? S_OK : create_movefolder_error(GetLastError()); + + memcpy(dst_path, destination, dst_len*sizeof(WCHAR)); + if (!separator) + dst_path[dst_len++] = '\\'; + + f = FindFirstFileW(source, &ffd); + if(f == INVALID_HANDLE_VALUE) + return create_error(GetLastError()); + + do { + name_len = lstrlenW(ffd.cFileName); + if(src_len+name_len+1>=MAX_PATH || dst_len+name_len+1>=MAX_PATH) { + FindClose(f); + return E_FAIL; + } + memcpy(filename, ffd.cFileName, (name_len+1)*sizeof(WCHAR)); + memcpy(dst_path + dst_len, ffd.cFileName, (name_len+1)*sizeof(WCHAR)); + TRACE("move %s to %s\n", debugstr_w(src_path), debugstr_w(dst_path)); + if (!MoveFileW(src_path, dst_path)) return create_error(GetLastError()); + } while(FindNextFileW(f, &ffd)); + FindClose(f); + + return S_OK; } static inline HRESULT copy_file(const WCHAR *source, DWORD source_len, diff --git a/dlls/scrrun/tests/filesystem.c b/dlls/scrrun/tests/filesystem.c index 6d1e69ce09e..31bd5e96e59 100644 --- a/dlls/scrrun/tests/filesystem.c +++ b/dlls/scrrun/tests/filesystem.c @@ -2603,7 +2603,7 @@ static void test_MoveFile(void) static void test_MoveFolder(void) { BSTR src, dst, str, empty; - WCHAR buffW1[MAX_PATH],buffW2[MAX_PATH],pathW[MAX_PATH]; + WCHAR buffW1[MAX_PATH], buffW2[MAX_PATH], pathW[MAX_PATH], srcW[MAX_PATH]; HRESULT hr; HANDLE *file; @@ -2698,6 +2698,31 @@ static void test_MoveFolder(void) lstrcatW(pathW,L"foo"); ok(RemoveDirectoryW(pathW), "can't remove %s directory\n", wine_dbgstr_w(pathW)); ok(RemoveDirectoryW(buffW2), "can't remove %s directory\n", wine_dbgstr_w(buffW2)); + + GetTempPathW(MAX_PATH, buffW1); + lstrcatW(buffW1,L"foo1"); + GetTempPathW(MAX_PATH, buffW2); + lstrcatW(buffW2,L"foo2"); + GetTempPathW(MAX_PATH, srcW); + lstrcatW(srcW,L"foo?"); + GetTempPathW(MAX_PATH, pathW); + lstrcatW(pathW,L"bar"); + ok(CreateDirectoryW(buffW1, NULL), "CreateDirectory(%s) failed\n", wine_dbgstr_w(buffW1)); + ok(CreateDirectoryW(buffW2, NULL), "CreateDirectory(%s) failed\n", wine_dbgstr_w(buffW2)); + ok(CreateDirectoryW(pathW, NULL), "CreateDirectory(%s) failed\n", wine_dbgstr_w(pathW)); + src = SysAllocString(srcW); + dst = SysAllocString(pathW); + hr = IFileSystem3_MoveFolder(fs3, src, dst); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + SysFreeString(src); + SysFreeString(dst); + lstrcpyW(buffW1,pathW); + lstrcatW(buffW1,L"\\foo1"); + lstrcpyW(buffW2,pathW); + lstrcatW(buffW2,L"\\foo2"); + ok(RemoveDirectoryW(buffW1), "can't remove %s directory\n", wine_dbgstr_w(buffW1)); + ok(RemoveDirectoryW(buffW2), "can't remove %s directory\n", wine_dbgstr_w(buffW2)); + ok(RemoveDirectoryW(pathW), "can't remove %s directory\n", wine_dbgstr_w(pathW)); } static void test_DoOpenPipeStream(void) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/391
From: Robert Wilhelm <robert.wilhelm(a)gmx.net> Signed-off-by: Robert Wilhelm <robert.wilhelm(a)gmx.net> --- dlls/scrrun/filesystem.c | 8 ++++++-- dlls/scrrun/tests/filesystem.c | 10 +++++++++- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/dlls/scrrun/filesystem.c b/dlls/scrrun/filesystem.c index 4ec2511a811..236f304c854 100644 --- a/dlls/scrrun/filesystem.c +++ b/dlls/scrrun/filesystem.c @@ -3824,14 +3824,18 @@ static HRESULT WINAPI filesys_MoveFolder(IFileSystem3 *iface, BSTR source, BSTR do { name_len = lstrlenW(ffd.cFileName); - if(src_len+name_len+1>=MAX_PATH || dst_len+name_len+1>=MAX_PATH) { + if(src_len+name_len+2>=MAX_PATH || dst_len+name_len+1>=MAX_PATH) { FindClose(f); return E_FAIL; } memcpy(filename, ffd.cFileName, (name_len+1)*sizeof(WCHAR)); + wcscat(filename, L"\\"); memcpy(dst_path + dst_len, ffd.cFileName, (name_len+1)*sizeof(WCHAR)); TRACE("move %s to %s\n", debugstr_w(src_path), debugstr_w(dst_path)); - if (!MoveFileW(src_path, dst_path)) return create_error(GetLastError()); + if (!MoveFileW(src_path, dst_path)) { + if (GetLastError() == ERROR_INVALID_NAME) continue; + return create_movefolder_error(GetLastError()); + } } while(FindNextFileW(f, &ffd)); FindClose(f); diff --git a/dlls/scrrun/tests/filesystem.c b/dlls/scrrun/tests/filesystem.c index 31bd5e96e59..87347627884 100644 --- a/dlls/scrrun/tests/filesystem.c +++ b/dlls/scrrun/tests/filesystem.c @@ -2603,7 +2603,7 @@ static void test_MoveFile(void) static void test_MoveFolder(void) { BSTR src, dst, str, empty; - WCHAR buffW1[MAX_PATH], buffW2[MAX_PATH], pathW[MAX_PATH], srcW[MAX_PATH]; + WCHAR buffW1[MAX_PATH], buffW2[MAX_PATH], buffW3[MAX_PATH], pathW[MAX_PATH], srcW[MAX_PATH]; HRESULT hr; HANDLE *file; @@ -2703,6 +2703,8 @@ static void test_MoveFolder(void) lstrcatW(buffW1,L"foo1"); GetTempPathW(MAX_PATH, buffW2); lstrcatW(buffW2,L"foo2"); + GetTempPathW(MAX_PATH, buffW3); + lstrcatW(buffW3,L"foo3"); GetTempPathW(MAX_PATH, srcW); lstrcatW(srcW,L"foo?"); GetTempPathW(MAX_PATH, pathW); @@ -2710,6 +2712,11 @@ static void test_MoveFolder(void) ok(CreateDirectoryW(buffW1, NULL), "CreateDirectory(%s) failed\n", wine_dbgstr_w(buffW1)); ok(CreateDirectoryW(buffW2, NULL), "CreateDirectory(%s) failed\n", wine_dbgstr_w(buffW2)); ok(CreateDirectoryW(pathW, NULL), "CreateDirectory(%s) failed\n", wine_dbgstr_w(pathW)); + /* create a file, should not be moved by MoveFolder() */ + file = CreateFileW(buffW3, GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, + FILE_ATTRIBUTE_NORMAL, NULL); + ok(file != INVALID_HANDLE_VALUE, "CreateFile failed\n"); + CloseHandle(file); src = SysAllocString(srcW); dst = SysAllocString(pathW); hr = IFileSystem3_MoveFolder(fs3, src, dst); @@ -2723,6 +2730,7 @@ static void test_MoveFolder(void) ok(RemoveDirectoryW(buffW1), "can't remove %s directory\n", wine_dbgstr_w(buffW1)); ok(RemoveDirectoryW(buffW2), "can't remove %s directory\n", wine_dbgstr_w(buffW2)); ok(RemoveDirectoryW(pathW), "can't remove %s directory\n", wine_dbgstr_w(pathW)); + ok(DeleteFileW(buffW3), "can't remove %s\n", wine_dbgstr_w(buffW3)); } static void test_DoOpenPipeStream(void) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/391
On Tue Jul 26 15:35:09 2022 +0000, Esme Povirk wrote:
I had an important realization about the directory check. If the source filename ends with a backslash, the MoveFileW call will only succeed if the source file is a directory. That should remove the need to check beforehand and eliminate the time-of-check to time-of-use problem. It does still require remapping some error codes. I get `ERROR_FILE_NOT_FOUND` if it doesn't exist and `ERROR_INVALID_NAME` if it's a regular file. But I think it'd be an improvement overall. Hi Esme, I have pushed new MR where I add a backslash as suggested by you. Please have a look. Thanks, Robert
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/391#note_4972
Esme Povirk (@madewokherd) commented about dlls/scrrun/tests/filesystem.c:
BSTR src, dst, str, empty; WCHAR buffW1[MAX_PATH],buffW2[MAX_PATH]; HRESULT hr; + HANDLE *file;
This should just be a HANDLE. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/391#note_5356
Esme Povirk (@madewokherd) commented about dlls/scrrun/filesystem.c:
static HRESULT WINAPI filesys_MoveFolder(IFileSystem3 *iface, BSTR source, BSTR destination) { + int len; + WCHAR src_path[MAX_PATH]; + TRACE("%p %s %s\n", iface, debugstr_w(source), debugstr_w(destination));
if(!source || !source[0] || !destination || !destination[0]) return E_INVALIDARG;
- return MoveFileW(source, destination) ? S_OK : create_movefolder_error(GetLastError()); + len = SysStringLen(source); + lstrcpyW(src_path, source); + if (source[len-1] != '\\' && source[len-1] != '/') wcscat(src_path, L"\\");
This looks very similar to `PathAddBackslashW`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/391#note_5357
Esme Povirk (@madewokherd) commented about dlls/scrrun/filesystem.c:
static HRESULT WINAPI filesys_MoveFolder(IFileSystem3 *iface, BSTR source, BSTR destination) { - int len; - WCHAR src_path[MAX_PATH]; + int len, dst_len; + WCHAR src_path[MAX_PATH], dst_path[MAX_PATH]; + WCHAR *filename;
TRACE("%p %s %s\n", iface, debugstr_w(source), debugstr_w(destination));
if(!source || !source[0] || !destination || !destination[0]) return E_INVALIDARG;
+ if (!GetFullPathNameW(source, MAX_PATH, src_path, &filename))
Looking at Wine's code, I think this will set filename to NULL if source ends in a path separator. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/391#note_5358
Esme Povirk (@madewokherd) commented about dlls/scrrun/filesystem.c:
if(!source || !source[0] || !destination || !destination[0]) return E_INVALIDARG;
+ if (!GetFullPathNameW(source, MAX_PATH, src_path, &filename)) + return E_FAIL; + len = SysStringLen(source); lstrcpyW(src_path, source); if (source[len-1] != '\\' && source[len-1] != '/') wcscat(src_path, L"\\");
+ dst_len = lstrlenW(destination); + if (destination[dst_len-1] == '\\' || destination[dst_len-1] == '/') { + lstrcpyW(dst_path, destination); + lstrcatW(dst_path, filename); + TRACE("move %s to %s\n", debugstr_w(src_path), debugstr_w(dst_path)); + return MoveFileW(src_path, dst_path) ? S_OK : create_error(GetLastError());
This should probably be using create_movefolder_error? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/391#note_5359
Esme Povirk (@madewokherd) commented about dlls/scrrun/filesystem.c:
- if(src_len+name_len+1>=MAX_PATH || dst_len+name_len+1>=MAX_PATH) { + if(src_len+name_len+2>=MAX_PATH || dst_len+name_len+1>=MAX_PATH) { FindClose(f); return E_FAIL; } memcpy(filename, ffd.cFileName, (name_len+1)*sizeof(WCHAR)); + wcscat(filename, L"\\"); memcpy(dst_path + dst_len, ffd.cFileName, (name_len+1)*sizeof(WCHAR)); TRACE("move %s to %s\n", debugstr_w(src_path), debugstr_w(dst_path)); - if (!MoveFileW(src_path, dst_path)) return create_error(GetLastError()); + if (!MoveFileW(src_path, dst_path)) { + if (GetLastError() == ERROR_INVALID_NAME) continue; + return create_movefolder_error(GetLastError()); + } } while(FindNextFileW(f, &ffd)); FindClose(f); There should probably be a check after this loop for the case where a wildcard found only regular files, and nothing was moved. I'd expect Windows to treat that as an error.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/391#note_5360
On Sat Jul 30 03:26:17 2022 +0000, Esme Povirk wrote:
This should just be a HANDLE. I will fix this in next MR.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/391#note_5517
On Sat Jul 30 03:26:18 2022 +0000, Esme Povirk wrote:
This looks very similar to `PathAddBackslashW`. PathAddBackslashW does only check for backslash as last char, but not for slash. Therefore we cannot use it here.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/391#note_5518
On Sat Jul 30 03:26:18 2022 +0000, Esme Povirk wrote:
Looking at Wine's code, I think this will set filename to NULL if source ends in a path separator. If source ends with path separator, native Movefolder returns E_INVALIDARG. I will add a test and fix the code.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/391#note_5519
On Sat Jul 30 03:26:19 2022 +0000, Esme Povirk wrote:
This should probably be using create_movefolder_error? I will change it to create_movefolder_error. With current tests it makes no difference, but it is more consistent.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/391#note_5520
On Sat Jul 30 03:26:20 2022 +0000, Esme Povirk wrote:
There should probably be a check after this loop for the case where a wildcard found only regular files, and nothing was moved. I'd expect Windows to treat that as an error. You are right. Native seems to return path not found error in this case. I will add test and check for this case in code.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/391#note_5521
participants (3)
-
Esme Povirk (＠madewokherd) -
Robert Wilhelm
-
Robert Wilhelm (＠sloper42)