[PATCH 0/1] MR9104: scrrun: Implement MoveFolder().
From: Robert Wilhelm <robert.wilhelm(a)gmx.net> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52128 --- dlls/scrrun/filesystem.c | 85 ++++++++++++++++++++- dlls/scrrun/tests/filesystem.c | 130 +++++++++++++++++++++++++++++++++ 2 files changed, 211 insertions(+), 4 deletions(-) diff --git a/dlls/scrrun/filesystem.c b/dlls/scrrun/filesystem.c index d174125ea91..da213c02d98 100644 --- a/dlls/scrrun/filesystem.c +++ b/dlls/scrrun/filesystem.c @@ -3811,12 +3811,89 @@ 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 src, BSTR dst) { - FIXME("%p %s %s\n", iface, debugstr_w(Source), debugstr_w(Destination)); + BOOL wildcard = FALSE, separator = FALSE, success = FALSE; + int src_len, dst_len, name_len; + WIN32_FIND_DATAW find_data; + HANDLE find; + BOOL ret; - return E_NOTIMPL; + TRACE("%p %s %s\n", iface, debugstr_w(src), debugstr_w(dst)); + + if (!src || !src[0] || !dst || !dst[0]) + return E_INVALIDARG; + + src_len = SysStringLen(src); + if (src[src_len - 1] == '\\' || src[src_len - 1] == '/') + return CTL_E_ILLEGALFUNCTIONCALL; + + if (wcspbrk(src, L"*?><")) + wildcard = TRUE; + + dst_len = SysStringLen(dst); + if (dst[dst_len - 1] == '\\' || dst[dst_len - 1] == '/') + separator = TRUE; + + if (!wildcard && !separator) + { + /* Ensure that we open a directory by appending a slash. */ + WCHAR *src_copy = malloc((src_len + 2) * sizeof(WCHAR)); + + memcpy(src_copy, src, src_len * sizeof(WCHAR)); + wcscpy(src_copy + src_len, L"\\"); + + ret = MoveFileW(src_copy, dst); + free(src_copy); + if (ret) + return S_OK; + if (GetLastError() == ERROR_INVALID_NAME || GetLastError() == ERROR_FILE_NOT_FOUND) + return CTL_E_PATHNOTFOUND; + return create_error(GetLastError()); + } + + /* Back up to the slash if there is one. */ + while (src_len > 0 && src[src_len] != '/' && src[src_len] != '\\') + --src_len; + + if ((find = FindFirstFileW(src, &find_data)) == INVALID_HANDLE_VALUE) + return create_error(GetLastError()); + + do + { + WCHAR *src_folder, *dst_folder; + + if (!(find_data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)) + continue; + + name_len = wcslen(find_data.cFileName); + src_folder = malloc((src_len + name_len + 1) * sizeof(WCHAR)); + dst_folder = malloc((dst_len + 1 + name_len + 1) * sizeof(WCHAR)); + + if (src_len) + memcpy(src_folder, src, src_len * sizeof(WCHAR)); + memcpy(src_folder + src_len, find_data.cFileName, (name_len + 1) * sizeof(WCHAR)); + + memcpy(dst_folder, dst, dst_len * sizeof(WCHAR)); + if (!separator) + dst_folder[dst_len] = '\\'; + memcpy(dst_folder + dst_len + !separator, find_data.cFileName, (name_len + 1) * sizeof(WCHAR)); + + ret = MoveFileW(src_folder, dst_folder); + free(src_folder); + free(dst_folder); + if (!ret) + { + FindClose(find); + if (GetLastError() == ERROR_FILE_NOT_FOUND) + return CTL_E_PATHNOTFOUND; + return create_error(GetLastError()); + } + success = TRUE; + } while (FindNextFileW(find, &find_data)); + + FindClose(find); + return success ? S_OK : CTL_E_PATHNOTFOUND; } 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 ea0d0925a42..5edf11824e0 100644 --- a/dlls/scrrun/tests/filesystem.c +++ b/dlls/scrrun/tests/filesystem.c @@ -2709,6 +2709,135 @@ static void test_MoveFile(void) SysFreeString(str); } +static void test_MoveFolder(void) +{ + WCHAR temp_path[MAX_PATH], cwd[MAX_PATH]; + BSTR src, dst, empty; + HANDLE file; + HRESULT hr; + BOOL ret; + + GetCurrentDirectoryW(MAX_PATH, cwd); + GetTempPathW(MAX_PATH, temp_path); + SetCurrentDirectoryW(temp_path); + src = SysAllocString(L"winetest_src"); + dst = SysAllocString(L"winetest_dst"); + + ret = CreateDirectoryW(L"winetest_src", NULL); + ok(ret, "Got error %lu.\n", GetLastError()); + hr = IFileSystem3_MoveFolder(fs3, src, dst); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IFileSystem3_MoveFolder(fs3, src, dst); + ok(hr == CTL_E_PATHNOTFOUND, "Got hr %#lx.\n", hr); + ret = CreateDirectoryW(L"winetest_src", NULL); + ok(ret, "Got error %lu.\n", GetLastError()); + hr = IFileSystem3_MoveFolder(fs3, src, dst); + ok(hr == CTL_E_FILEALREADYEXISTS, "Got hr %#lx.\n", hr); + ret = RemoveDirectoryW(L"winetest_dst"); + ok(ret, "Got error %lu.\n", GetLastError()); + ret = RemoveDirectoryW(L"winetest_src"); + ok(ret, "Got error %lu.\n", GetLastError()); + + empty = SysAllocString(L""); + hr = IFileSystem3_MoveFolder(fs3, src, NULL); + ok(hr == E_INVALIDARG, "Got hr %#lx.\n", hr); + hr = IFileSystem3_MoveFolder(fs3, NULL, dst); + ok(hr == E_INVALIDARG, "Got hr %#lx.\n", hr); + hr = IFileSystem3_MoveFolder(fs3, src, empty); + ok(hr == E_INVALIDARG, "Got hr %#lx.\n", hr); + hr = IFileSystem3_MoveFolder(fs3, empty, dst); + ok(hr == E_INVALIDARG, "Got hr %#lx.\n", hr); + SysFreeString(empty); + + file = CreateFileW(L"winetest_src", GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); + ok(file != INVALID_HANDLE_VALUE, "Got error %lu.\n", GetLastError()); + CloseHandle(file); + hr = IFileSystem3_MoveFolder(fs3, src, dst); + ok(hr == CTL_E_PATHNOTFOUND, "Got hr %#lx.\n", hr); + ret = DeleteFileW(L"winetest_src"); + ok(ret, "Got error %lu.\n", GetLastError()); + + dst = SysAllocString(L"winetest_dst\\"); + + ret = CreateDirectoryW(L"winetest_src", NULL); + ok(ret, "Got error %lu.\n", GetLastError()); + hr = IFileSystem3_MoveFolder(fs3, src, dst); + ok(hr == CTL_E_PATHNOTFOUND, "Got hr %#lx.\n", hr); + ret = CreateDirectoryW(L"winetest_dst", NULL); + ok(ret, "Got error %lu.\n", GetLastError()); + hr = IFileSystem3_MoveFolder(fs3, src, dst); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + ret = RemoveDirectoryW(L"winetest_dst\\winetest_src"); + ok(ret, "Got error %lu.\n", GetLastError()); + ret = RemoveDirectoryW(L"winetest_dst"); + ok(ret, "Got error %lu.\n", GetLastError()); + + src = SysAllocString(L"winetest_src\\"); + dst = SysAllocString(L"winetest_dst"); + + ret = CreateDirectoryW(L"winetest_src", NULL); + ok(ret, "Got error %lu.\n", GetLastError()); + hr = IFileSystem3_MoveFolder(fs3, src, dst); + ok(hr == CTL_E_ILLEGALFUNCTIONCALL, "Got hr %#lx.\n", hr); + src = SysAllocString(L"winetest_src\\"); + hr = IFileSystem3_MoveFolder(fs3, src, dst); + ok(hr == CTL_E_ILLEGALFUNCTIONCALL, "Got hr %#lx.\n", hr); + + ret = RemoveDirectoryW(L"winetest_src"); + ok(ret, "Got error %lu.\n", GetLastError()); + + ret = CreateDirectoryW(L"winetest_src1", NULL); + ok(ret, "Got error %lu.\n", GetLastError()); + ret = CreateDirectoryW(L"winetest_src2", NULL); + ok(ret, "Got error %lu.\n", GetLastError()); + file = CreateFileW(L"winetest_src3", GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); + ok(file != INVALID_HANDLE_VALUE, "Got error %lu.\n", GetLastError()); + CloseHandle(file); + + src = SysAllocString(L"winetest_src*"); + hr = IFileSystem3_MoveFolder(fs3, src, dst); + ok(hr == CTL_E_PATHNOTFOUND, "Got hr %#lx.\n", hr); + ret = CreateDirectoryW(L"winetest_dst", NULL); + ok(ret, "Got error %lu.\n", GetLastError()); + hr = IFileSystem3_MoveFolder(fs3, src, dst); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IFileSystem3_MoveFolder(fs3, src, dst); + ok(hr == CTL_E_PATHNOTFOUND, "Got hr %#lx.\n", hr); + ret = RemoveDirectoryW(L"winetest_dst/winetest_src1"); + ok(ret, "Got error %lu.\n", GetLastError()); + ret = RemoveDirectoryW(L"winetest_dst/winetest_src2"); + ok(ret, "Got error %lu.\n", GetLastError()); + ret = RemoveDirectoryW(L"winetest_dst"); + ok(ret, "Got error %lu.\n", GetLastError()); + ret = DeleteFileW(L"winetest_src3"); + ok(ret, "Got error %lu.\n", GetLastError()); + + /* Fail to copy one file. */ + ret = CreateDirectoryW(L"winetest_src1", NULL); + ok(ret, "Got error %lu.\n", GetLastError()); + ret = CreateDirectoryW(L"winetest_src2", NULL); + ok(ret, "Got error %lu.\n", GetLastError()); + ret = CreateDirectoryW(L"winetest_src3", NULL); + ok(ret, "Got error %lu.\n", GetLastError()); + ret = CreateDirectoryW(L"winetest_dst", NULL); + ok(ret, "Got error %lu.\n", GetLastError()); + ret = CreateDirectoryW(L"winetest_dst\\winetest_src2", NULL); + ok(ret, "Got error %lu.\n", GetLastError()); + hr = IFileSystem3_MoveFolder(fs3, src, dst); + ok(hr == CTL_E_FILEALREADYEXISTS, "Got hr %#lx.\n", hr); + ret = RemoveDirectoryW(L"winetest_dst/winetest_src1"); + ok(ret, "Got error %lu.\n", GetLastError()); + ret = RemoveDirectoryW(L"winetest_dst/winetest_src2"); + ok(ret, "Got error %lu.\n", GetLastError()); + ret = RemoveDirectoryW(L"winetest_dst"); + ok(ret, "Got error %lu.\n", GetLastError()); + ret = RemoveDirectoryW(L"winetest_src2"); + ok(ret, "Got error %lu.\n", GetLastError()); + ret = RemoveDirectoryW(L"winetest_src3"); + ok(ret, "Got error %lu.\n", GetLastError()); +} + static void test_DoOpenPipeStream(void) { static const char testdata[] = "test"; @@ -2843,6 +2972,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/9104
This merge request was approved by Elizabeth Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9104
Another version was !391. My main concerns would be all of manual path handling, that ideally should be handled with something we have, maybe PathCch* functions? Then error codes fixups. Since it supports wildcard patterns, maybe we can have findnext loop in all cases to get rid of "if (!wildcard && !separator)" special case. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9104#note_117849
Another version was !391. My main concerns would be all of manual path handling, that ideally should be handled with something we have, maybe PathCch\* functions? Then error codes fixups.
Sure, I guess that could save a couple lines.
Since it supports wildcard patterns, maybe we can have findnext loop in all cases to get rid of "if (!wildcard && !separator)" special case.
The reason for the special case is that if there's a wildcard or a separator, copying "src" to "dst" always copies src *into* dst, but if there's neither a wildcard nor a separator, it renames "src" to "dst". -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9104#note_117855
On Tue Oct 7 16:41:52 2025 +0000, Elizabeth Figura wrote:
Another version was !391. My main concerns would be all of manual path handling, that ideally should be handled with something we have, maybe PathCch\* functions? Then error codes fixups. Sure, I guess that could save a couple lines. Since it supports wildcard patterns, maybe we can have findnext loop in all cases to get rid of "if (!wildcard && !separator)" special case. The reason for the special case is that if there's a wildcard or a separator, copying "src" to "dst" always copies src *into* dst, but if there's neither a wildcard nor a separator, it renames "src" to "dst". Alright. So if you think it's possible to simplify this a little, or make it shorter, I think we should do that. If not, let's have it as is.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9104#note_117858
This merge request was approved by Nikolay Sivov. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9104
On Tue Oct 7 16:41:52 2025 +0000, Nikolay Sivov wrote:
Alright. So if you think it's possible to simplify this a little, or make it shorter, I think we should do that. If not, let's have it as is. Oops, sorry, dropped this on the floor.
I pushed a v2 that uses PathAllocCombine() for dst_folder. I'm not sure that anything else really gets easier with kernelbase path functions [in particular src_folder doesn't because we're only copying part of the "src" string], though I'm open to using them anyway as a matter of taste. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9104#note_119079
participants (3)
-
Elizabeth Figura (＠zfigura) -
Nikolay Sivov (＠nsivov)
-
Robert Wilhelm