[PATCH 0/2] MR4817: shell32: Implement some functions of IFileOperation.
IFileOperation_{SetOperationFlags, NewItem, CopyItem, MoveItem, DeleteItem, GetAnyOperationsAborted, PerformOperations, RenameItem, SetOwnerWindow} -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4817
From: Haoyang Chen <chenhaoyang(a)kylinos.cn> IFileOperation_{SetOperationFlags, NewItem, CopyItem, MoveItem, DeleteItem, GetAnyOperationsAborted, PerformOperations, RenameItem, SetOwnerWindow} --- dlls/shell32/shlfileop.c | 340 +++++++++++++++++++++++++++++++++++---- 1 file changed, 312 insertions(+), 28 deletions(-) diff --git a/dlls/shell32/shlfileop.c b/dlls/shell32/shlfileop.c index b4a941187ab..ff45abc7378 100644 --- a/dlls/shell32/shlfileop.c +++ b/dlls/shell32/shlfileop.c @@ -55,6 +55,8 @@ WINE_DEFAULT_DEBUG_CHANNEL(shell); #define DE_SAMEFILE 0x71 #define DE_DESTSAMETREE 0x7D +#define FO_NEW 0x5 + static DWORD SHNotifyCreateDirectoryA(LPCSTR path, LPSECURITY_ATTRIBUTES sec); static DWORD SHNotifyCreateDirectoryW(LPCWSTR path, LPSECURITY_ATTRIBUTES sec); static DWORD SHNotifyRemoveDirectoryA(LPCSTR path); @@ -1800,18 +1802,148 @@ HRESULT WINAPI SHMultiFileProperties(IDataObject *pdtobj, DWORD flags) struct file_operation { - IFileOperation IFileOperation_iface; - LONG ref; + struct list entry; + UINT wFunc; + LPCWSTR pFrom; + LPCWSTR pTo; + LPCWSTR pNewName; + LPCWSTR pTemplateName; + DWORD attributes; }; -static inline struct file_operation *impl_from_IFileOperation(IFileOperation *iface) +struct file_operations { - return CONTAINING_RECORD(iface, struct file_operation, IFileOperation_iface); + IFileOperation IFileOperation_iface; + LONG ref; + HWND hwnd; + DWORD flags; + BOOL fAnyOperationsAborted; + LPVOID hNameMappings; + LPCWSTR lpszProgressTitle; + struct list ops; +}; + +static HRESULT add_operation( IShellItem *item, IShellItem *folder, + LPCWSTR name, UINT func, struct file_operation **out) +{ + LPWSTR tmp, from, to = NULL; + HRESULT ret; + + if (!out) return ERROR_INVALID_PARAMETER; + + ret = IShellItem_GetDisplayName(item, SIGDN_FILESYSPATH, &tmp); + if (S_OK != ret) + { + return ERROR_INVALID_PARAMETER; + } + + from = calloc(lstrlenW(tmp) + 2, sizeof(WCHAR)); + if (!from) + { + ret = E_OUTOFMEMORY; + CoTaskMemFree(tmp); + goto end; + } + + lstrcpyW(from, tmp); + CoTaskMemFree(tmp); + + if (func != FO_DELETE) + { + + ret = IShellItem_GetDisplayName(folder, SIGDN_FILESYSPATH, &tmp); + if (S_OK != ret) + { + ret = ERROR_INVALID_PARAMETER; + goto end; + } + + if (name) + { + to = calloc(lstrlenW(tmp) + MAX_PATH + 2, sizeof(WCHAR)); + PathCombineW(to, tmp, name); + } + else + { + to = calloc(lstrlenW(tmp) + 2, sizeof(WCHAR)); + if (!to) + { + ret = E_OUTOFMEMORY; + CoTaskMemFree(tmp); + goto end; + } + + lstrcpyW(to, tmp); + } + CoTaskMemFree(tmp); + } + + *out = calloc(1, sizeof(struct file_operation)); + if (!*out) + { + ret = E_OUTOFMEMORY; + goto end; + } + + (*out)->wFunc = func; + (*out)->pFrom = from; + (*out)->pTo = to; + return ret; + +end: + if (from) free(from); + if (to) free(to); + + return ret; +} + +static HRESULT new_item(const WCHAR *folder, const WCHAR *name, DWORD attributes, struct file_operations *ops) +{ + HRESULT ret; + WCHAR path[MAX_PATH]; + HANDLE file; + + if (!(ops->flags & FOF_NOCONFIRMATION) && !PathFileExistsW(folder)) + { + if (!SHELL_ConfirmDialogW(ops->hwnd, ASK_CREATE_FOLDER, PathFindFileNameW(folder), NULL)) + { + ops->fAnyOperationsAborted = TRUE; + return ERROR_CANCELLED; + } + ret = SHNotifyCreateDirectoryW(folder, NULL); + if (S_OK != ret) return ret; + } + + PathCombineW(path, folder, name); + + if (attributes & FILE_ATTRIBUTE_DIRECTORY) + { + ret = SHNotifyCreateDirectoryW(path, NULL); + if (S_OK == ret) + { + if (!SetFileAttributesW(path, attributes)) ret = GetLastError(); + }; + } + else + { + file = CreateFileW(path, GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, attributes, NULL); + + if (file != INVALID_HANDLE_VALUE) + CloseHandle(file); + else + ret = GetLastError(); + } + + return ret; +} +static inline struct file_operations *impl_from_IFileOperation(IFileOperation *iface) +{ + return CONTAINING_RECORD(iface, struct file_operations, IFileOperation_iface); } static HRESULT WINAPI file_operation_QueryInterface(IFileOperation *iface, REFIID riid, void **out) { - struct file_operation *operation = impl_from_IFileOperation(iface); + struct file_operations *operation = impl_from_IFileOperation(iface); TRACE("(%p, %s, %p).\n", iface, debugstr_guid(riid), out); @@ -1831,7 +1963,7 @@ static HRESULT WINAPI file_operation_QueryInterface(IFileOperation *iface, REFII static ULONG WINAPI file_operation_AddRef(IFileOperation *iface) { - struct file_operation *operation = impl_from_IFileOperation(iface); + struct file_operations *operation = impl_from_IFileOperation(iface); ULONG ref = InterlockedIncrement(&operation->ref); TRACE("(%p): ref=%lu.\n", iface, ref); @@ -1841,14 +1973,23 @@ static ULONG WINAPI file_operation_AddRef(IFileOperation *iface) static ULONG WINAPI file_operation_Release(IFileOperation *iface) { - struct file_operation *operation = impl_from_IFileOperation(iface); - ULONG ref = InterlockedDecrement(&operation->ref); + struct file_operations *operations = impl_from_IFileOperation(iface); + struct file_operation *ptr, *next; + ULONG ref = InterlockedDecrement(&operations->ref); TRACE("(%p): ref=%lu.\n", iface, ref); if (!ref) { - free(operation); + LIST_FOR_EACH_ENTRY_SAFE( ptr, next, &operations->ops, struct file_operation, entry ) + { + if (ptr->pFrom) free((void*)ptr->pFrom); + if (ptr->pTo) free((void*)ptr->pTo); + if (ptr->pNewName) free((void*)ptr->pNewName); + if (ptr->pTemplateName) free((void*)ptr->pTemplateName); + free(ptr); + } + free(operations); } return ref; @@ -1870,9 +2011,13 @@ static HRESULT WINAPI file_operation_Unadvise(IFileOperation *iface, DWORD cooki static HRESULT WINAPI file_operation_SetOperationFlags(IFileOperation *iface, DWORD flags) { - FIXME("(%p, %lx): stub.\n", iface, flags); + struct file_operations *operations = impl_from_IFileOperation(iface); - return E_NOTIMPL; + TRACE("(%p): flags: %lx.\n", iface, flags); + + operations->flags = flags; + + return S_OK; } static HRESULT WINAPI file_operation_SetProgressMessage(IFileOperation *iface, LPCWSTR message) @@ -1898,9 +2043,13 @@ static HRESULT WINAPI file_operation_SetProperties(IFileOperation *iface, IPrope static HRESULT WINAPI file_operation_SetOwnerWindow(IFileOperation *iface, HWND owner) { - FIXME("(%p, %p): stub.\n", iface, owner); + struct file_operations *operations = impl_from_IFileOperation(iface); - return E_NOTIMPL; + TRACE("(%p): owner: %p.\n", iface, owner); + + operations->hwnd = owner; + + return S_OK; } static HRESULT WINAPI file_operation_ApplyPropertiesToItem(IFileOperation *iface, IShellItem *item) @@ -1920,9 +2069,16 @@ static HRESULT WINAPI file_operation_ApplyPropertiesToItems(IFileOperation *ifac static HRESULT WINAPI file_operation_RenameItem(IFileOperation *iface, IShellItem *item, LPCWSTR name, IFileOperationProgressSink *sink) { - FIXME("(%p, %p, %s, %p): stub.\n", iface, item, debugstr_w(name), sink); + struct file_operations *operations = impl_from_IFileOperation(iface); + struct file_operation *op; + HRESULT ret; - return E_NOTIMPL; + TRACE("(%p, %p, %s, %p).\n", iface, item, debugstr_w(name), sink); + + ret = add_operation(item, item, name, FO_RENAME, &op); + + if (ret == S_OK) list_add_tail( &operations->ops, &op->entry ); + return ret; } static HRESULT WINAPI file_operation_RenameItems(IFileOperation *iface, IUnknown *items, LPCWSTR name) @@ -1935,9 +2091,15 @@ static HRESULT WINAPI file_operation_RenameItems(IFileOperation *iface, IUnknown static HRESULT WINAPI file_operation_MoveItem(IFileOperation *iface, IShellItem *item, IShellItem *folder, LPCWSTR name, IFileOperationProgressSink *sink) { - FIXME("(%p, %p, %p, %s, %p): stub.\n", iface, item, folder, debugstr_w(name), sink); + struct file_operations *operations = impl_from_IFileOperation(iface); + struct file_operation *op; + HRESULT ret; - return E_NOTIMPL; + TRACE("(%p, %p, %p, %s, %p).\n", iface, item, folder, debugstr_w(name), sink); + ret = add_operation(item, folder, name, FO_MOVE, &op); + + if (ret == S_OK) list_add_tail( &operations->ops, &op->entry ); + return ret; } static HRESULT WINAPI file_operation_MoveItems(IFileOperation *iface, IUnknown *items, IShellItem *folder) @@ -1950,9 +2112,16 @@ static HRESULT WINAPI file_operation_MoveItems(IFileOperation *iface, IUnknown * static HRESULT WINAPI file_operation_CopyItem(IFileOperation *iface, IShellItem *item, IShellItem *folder, LPCWSTR name, IFileOperationProgressSink *sink) { - FIXME("(%p, %p, %p, %s, %p): stub.\n", iface, item, folder, debugstr_w(name), sink); + struct file_operations *operations = impl_from_IFileOperation(iface); + struct file_operation *op; + HRESULT ret; - return E_NOTIMPL; + TRACE("(%p, %p, %p, %s, %p).\n", iface, item, folder, debugstr_w(name), sink); + + ret = add_operation(item, folder, name, FO_COPY, &op); + + if (ret == S_OK) list_add_tail( &operations->ops, &op->entry ); + return ret; } static HRESULT WINAPI file_operation_CopyItems(IFileOperation *iface, IUnknown *items, IShellItem *folder) @@ -1965,9 +2134,16 @@ static HRESULT WINAPI file_operation_CopyItems(IFileOperation *iface, IUnknown * static HRESULT WINAPI file_operation_DeleteItem(IFileOperation *iface, IShellItem *item, IFileOperationProgressSink *sink) { - FIXME("(%p, %p, %p): stub.\n", iface, item, sink); + struct file_operations *operations = impl_from_IFileOperation(iface); + struct file_operation *op; + HRESULT ret; - return E_NOTIMPL; + TRACE("(%p, %p, %p).\n", iface, item, sink); + + ret = add_operation(item, NULL, NULL , FO_DELETE, &op); + + if (ret == S_OK) list_add_tail( &operations->ops, &op->entry ); + return ret; } static HRESULT WINAPI file_operation_DeleteItems(IFileOperation *iface, IUnknown *items) @@ -1980,24 +2156,131 @@ static HRESULT WINAPI file_operation_DeleteItems(IFileOperation *iface, IUnknown static HRESULT WINAPI file_operation_NewItem(IFileOperation *iface, IShellItem *folder, DWORD attributes, LPCWSTR name, LPCWSTR template, IFileOperationProgressSink *sink) { - FIXME("(%p, %p, %lx, %s, %s, %p): stub.\n", iface, folder, attributes, + + struct file_operations *operations = impl_from_IFileOperation(iface); + struct file_operation *op; + HRESULT ret; + LPWSTR tmp, new = NULL, to, temp = NULL; + + TRACE("(%p, %p, %lx, %s, %s, %p).\n", iface, folder, attributes, debugstr_w(name), debugstr_w(template), sink); - return E_NOTIMPL; + ret = IShellItem_GetDisplayName(folder, SIGDN_FILESYSPATH, &tmp); + if (S_OK != ret) + { + return ERROR_INVALID_PARAMETER; + } + + to = calloc(lstrlenW(tmp) + 1, sizeof(WCHAR)); + if (!to) + { + CoTaskMemFree(tmp); + ret = E_OUTOFMEMORY; + goto end; + } + + lstrcpyW(to, tmp); + CoTaskMemFree(tmp); + + if (name) + { + new = calloc(lstrlenW(name) + 1, sizeof(WCHAR)); + if (!new) + { + ret = E_OUTOFMEMORY; + goto end; + } + lstrcpyW(new, name); + } + + if (template) + { + temp = calloc(lstrlenW(template) + 1, sizeof(WCHAR)); + if (!temp) + { + ret = E_OUTOFMEMORY; + goto end; + } + lstrcpyW(temp, template); + } + + op = calloc(1, sizeof(struct file_operation)); + if (!op) + { + ret = E_OUTOFMEMORY; + goto end; + } + op->wFunc = FO_NEW; + op->pTo = to; + op->pNewName = new; + op->pTemplateName = temp; + op->attributes = attributes; + list_add_tail( &operations->ops, &op->entry ); + return ret; + +end: + if (new) free(new); + if (temp) free(temp); + if (to) free(to); + + return ret; } static HRESULT WINAPI file_operation_PerformOperations(IFileOperation *iface) { - FIXME("(%p): stub.\n", iface); + struct file_operations *operations = impl_from_IFileOperation(iface); + struct file_operation *ptr, *next; + SHFILEOPSTRUCTW shfoW; + HRESULT ret = E_UNEXPECTED; - return E_NOTIMPL; + TRACE("\n"); + + shfoW.hwnd = operations->hwnd; + shfoW.fFlags = operations->flags; + shfoW.hNameMappings = operations->hNameMappings; + shfoW.lpszProgressTitle = operations->lpszProgressTitle; + + LIST_FOR_EACH_ENTRY_SAFE( ptr, next, &operations->ops, struct file_operation, entry ) + { + TRACE("func: %d\n", ptr->wFunc); + if (ptr->wFunc == FO_NEW) + { + if (ptr->pTemplateName) + FIXME("stub template\n"); + + ret = new_item(ptr->pTo, ptr->pNewName, ptr->attributes, operations); + + if (ptr->pNewName) free((void*)ptr->pNewName); + if (ptr->pTo) free((void*)ptr->pTo); + if (ptr->pTemplateName) free((void*)ptr->pTemplateName); + list_remove(&ptr->entry); + continue; + } + + shfoW.wFunc = ptr->wFunc; + shfoW.pFrom = ptr->pFrom; + shfoW.pTo = ptr->pTo; + + ret = SHFileOperationW(&shfoW); + operations->fAnyOperationsAborted = shfoW.fAnyOperationsAborted; + + list_remove(&ptr->entry); + if (ptr->pFrom) free((void*)ptr->pFrom); + if (ptr->pTo) free((void*)ptr->pTo); + free(ptr); + } + + return ret; } static HRESULT WINAPI file_operation_GetAnyOperationsAborted(IFileOperation *iface, BOOL *aborted) { - FIXME("(%p, %p): stub.\n", iface, aborted); + struct file_operations *operations = impl_from_IFileOperation(iface); + TRACE("(%p, %p) aborted:%d.\n", iface, aborted, operations->fAnyOperationsAborted); - return E_NOTIMPL; + if (aborted) *aborted = operations->fAnyOperationsAborted; + + return S_OK; } static const IFileOperationVtbl file_operation_vtbl = @@ -2029,7 +2312,7 @@ static const IFileOperationVtbl file_operation_vtbl = HRESULT WINAPI IFileOperation_Constructor(IUnknown *outer, REFIID riid, void **out) { - struct file_operation *object; + struct file_operations *object; HRESULT hr; object = calloc(1, sizeof(*object)); @@ -2038,6 +2321,7 @@ HRESULT WINAPI IFileOperation_Constructor(IUnknown *outer, REFIID riid, void **o object->IFileOperation_iface.lpVtbl = &file_operation_vtbl; object->ref = 1; + list_init( &object->ops); hr = IFileOperation_QueryInterface(&object->IFileOperation_iface, riid, out); IFileOperation_Release(&object->IFileOperation_iface); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4817
From: Haoyang Chen <chenhaoyang(a)kylinos.cn> --- dlls/shell32/tests/shlfileop.c | 229 +++++++++++++++++++++++++++++++++ 1 file changed, 229 insertions(+) diff --git a/dlls/shell32/tests/shlfileop.c b/dlls/shell32/tests/shlfileop.c index 273b09d5005..2d8fdff81fd 100644 --- a/dlls/shell32/tests/shlfileop.c +++ b/dlls/shell32/tests/shlfileop.c @@ -24,6 +24,7 @@ #define COBJMACROS #include <windows.h> #include "shellapi.h" +#include "shlwapi.h" #include "shlobj.h" #include "commoncontrols.h" @@ -44,6 +45,8 @@ broken(retval == ret_prewin32),\ "Expected %d, got %ld\n", ret, retval) +static HRESULT (WINAPI *pSHCreateItemFromParsingName)(PCWSTR,IBindCtx*,REFIID,void**); + static BOOL old_shell32 = FALSE; static CHAR CURR_DIR[MAX_PATH]; @@ -94,6 +97,17 @@ static BOOL file_existsW(LPCWSTR name) return GetFileAttributesW(name) != INVALID_FILE_ATTRIBUTES; } +static BOOL dir_existsW(const WCHAR *name) +{ + DWORD attr; + BOOL dir; + + attr = GetFileAttributesW(name); + dir = ((attr & FILE_ATTRIBUTE_DIRECTORY) == FILE_ATTRIBUTE_DIRECTORY); + + return ((attr != INVALID_FILE_ATTRIBUTES) && dir); +} + static BOOL file_has_content(const CHAR *name, const CHAR *content) { CHAR buf[MAX_PATH]; @@ -2739,11 +2753,42 @@ static BOOL is_old_shell32(void) return FALSE; } +static void init_function_pointers(void) +{ + HMODULE hmod; + hmod = GetModuleHandleA("shell32.dll"); + +#define MAKEFUNC(f) (p##f = (void*)GetProcAddress(hmod, #f)) + MAKEFUNC(SHCreateItemFromParsingName); + +#undef MAKEFUNC +} + static void test_file_operation(void) { IFileOperation *operation; IUnknown *unk; HRESULT hr; + IShellItem *shellitem, *shellitem2; + BOOL aborted; + WCHAR CURR_DIRW[MAX_PATH]; + WCHAR from[MAX_PATH]; + WCHAR to[MAX_PATH]; + WCHAR name1[] = {'t', 'e', 's', 't', '1', '.', 't', 'x', 't', '\0'}; + WCHAR name2[] = {'t', 'e', 's', 't', '2', '.', 't', 'x', 't', '\0'}; + WCHAR name3[] = {'t', 'e', 's', 't', '4', '.', 't', 'x', 't', '\0'}; + WCHAR path[MAX_PATH]; + WCHAR folder[MAX_PATH]; + int len; + DWORD attributes; + + GetCurrentDirectoryW(MAX_PATH, CURR_DIRW); + len = lstrlenW(CURR_DIRW); + + if(len && (CURR_DIRW[len-1] == '\\')) + CURR_DIRW[len-1] = 0; + lstrcpyW(to, CURR_DIRW); + lstrcpyW(from, CURR_DIRW); hr = CoCreateInstance(&CLSID_FileOperation, NULL, CLSCTX_INPROC_SERVER, &IID_IFileOperation, (void **)&operation); @@ -2759,6 +2804,189 @@ static void test_file_operation(void) ok(hr == S_OK, "Got hr %#lx.\n", hr); IUnknown_Release(unk); + hr = IFileOperation_GetAnyOperationsAborted(operation, &aborted); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + ok(aborted == FALSE, "Got unexpected abored.\n"); + + hr = IFileOperation_PerformOperations(operation); + ok(hr == E_UNEXPECTED, "Got hr %#lx.\n", hr); + + hr = IFileOperation_SetOperationFlags(operation, FOF_NO_UI); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + hr = IFileOperation_SetOwnerWindow(operation, GetDesktopWindow()); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + if (pSHCreateItemFromParsingName) + { + /* new directory */ + hr = pSHCreateItemFromParsingName(from, NULL, &IID_IShellItem, (void**)&shellitem); + ok(hr == S_OK, "SHCreateItemFromParsingName returned %lx\n", hr); + hr = IFileOperation_NewItem(operation, shellitem, FILE_ATTRIBUTE_DIRECTORY | FILE_ATTRIBUTE_READONLY, name3, NULL, NULL); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IFileOperation_PerformOperations(operation); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + PathCombineW(path, from, name3); + ok(dir_existsW(path), "Directory %s should exists\n", wine_dbgstr_w(path)); + attributes = GetFileAttributesW(path); + todo_wine + ok(attributes == (FILE_ATTRIBUTE_DIRECTORY | FILE_ATTRIBUTE_READONLY), "Attribute of directory %s expected %x, but got %lx\n", + wine_dbgstr_w(path), FILE_ATTRIBUTE_DIRECTORY | FILE_ATTRIBUTE_READONLY, attributes); + IShellItem_Release(shellitem); + + /* delete directory */ + hr = pSHCreateItemFromParsingName(path, NULL, &IID_IShellItem, (void**)&shellitem); + ok(hr == S_OK, "SHCreateItemFromParsingName returned %lx\n", hr); + hr = IFileOperation_DeleteItem(operation, shellitem, NULL); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IFileOperation_PerformOperations(operation); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + ok(!dir_existsW(path), "Directory %s should not exists\n", wine_dbgstr_w(path)); + IShellItem_Release(shellitem); + + /* new file */ + memset(path, 0, sizeof(path)); + hr = pSHCreateItemFromParsingName(from, NULL, &IID_IShellItem, (void**)&shellitem); + ok(hr == S_OK, "SHCreateItemFromParsingName returned %lx\n", hr); + hr = IFileOperation_NewItem(operation, shellitem, FILE_ATTRIBUTE_ARCHIVE | FILE_ATTRIBUTE_READONLY, name1, NULL, NULL); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IFileOperation_PerformOperations(operation); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + PathCombineW(path, from, name1); + ok(file_existsW(path), "File %s should exists\n", wine_dbgstr_w(path)); + attributes = GetFileAttributesW(path); + ok(attributes == (FILE_ATTRIBUTE_ARCHIVE | FILE_ATTRIBUTE_READONLY), "Attribute of directory %s expected %x, but got %lx\n", + wine_dbgstr_w(path), FILE_ATTRIBUTE_ARCHIVE | FILE_ATTRIBUTE_READONLY, attributes); + IShellItem_Release(shellitem); + + /* delete file */ + SetFileAttributesW(path, attributes & ~FILE_ATTRIBUTE_READONLY); + hr = pSHCreateItemFromParsingName(path, NULL, &IID_IShellItem, (void**)&shellitem); + ok(hr == S_OK, "SHCreateItemFromParsingName returned %lx\n", hr); + hr = IFileOperation_DeleteItem(operation, shellitem, NULL); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IFileOperation_PerformOperations(operation); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + ok(!file_existsW(path), "Directory %s should not exists\n", wine_dbgstr_w(path)); + IShellItem_Release(shellitem); + clean_after_shfo_tests(); + + /* copy file */ + memset(path, 0, sizeof(path)); + PathCombineW(folder, from, name3); + CreateDirectoryW(folder, NULL); + PathCombineW(path, folder, name1); + createTestFileW(path); + hr = pSHCreateItemFromParsingName(path, NULL, &IID_IShellItem, (void**)&shellitem); + ok(hr == S_OK, "SHCreateItemFromParsingName returned %lx\n", hr); + hr = pSHCreateItemFromParsingName(to, NULL, &IID_IShellItem, (void**)&shellitem2); + ok(hr == S_OK, "SHCreateItemFromParsingName returned %lx\n", hr); + hr = IFileOperation_CopyItem(operation, shellitem, shellitem2, NULL, NULL); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IFileOperation_PerformOperations(operation); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + ok(file_existsW(path), "File %s should exists\n", wine_dbgstr_w(path)); + memset(path, 0, sizeof(path)); + PathCombineW(path, to, name1); + ok(file_existsW(path), "File %s should exists\n", wine_dbgstr_w(path)); + IShellItem_Release(shellitem); + IShellItem_Release(shellitem2); + + memset(path, 0, sizeof(path)); + PathCombineW(path, from, name1); + hr = pSHCreateItemFromParsingName(path, NULL, &IID_IShellItem, (void**)&shellitem); + ok(hr == S_OK, "SHCreateItemFromParsingName returned %lx\n", hr); + hr = pSHCreateItemFromParsingName(to, NULL, &IID_IShellItem, (void**)&shellitem2); + ok(hr == S_OK, "SHCreateItemFromParsingName returned %lx\n", hr); + hr = IFileOperation_CopyItem(operation, shellitem, shellitem2, name2, NULL); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IFileOperation_PerformOperations(operation); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + memset(path, 0, sizeof(path)); + PathCombineW(path, to, name2); + ok(file_existsW(path), "File %s should exists\n", wine_dbgstr_w(path)); + IShellItem_Release(shellitem); + IShellItem_Release(shellitem2); + clean_after_shfo_tests(); + + /* move file */ + memset(path, 0, sizeof(path)); + PathCombineW(folder, from, name3); + CreateDirectoryW(folder, NULL); + PathCombineW(path, folder, name1); + createTestFileW(path); + hr = pSHCreateItemFromParsingName(path, NULL, &IID_IShellItem, (void**)&shellitem); + ok(hr == S_OK, "SHCreateItemFromParsingName returned %lx\n", hr); + hr = pSHCreateItemFromParsingName(to, NULL, &IID_IShellItem, (void**)&shellitem2); + ok(hr == S_OK, "SHCreateItemFromParsingName returned %lx\n", hr); + hr = IFileOperation_MoveItem(operation, shellitem, shellitem2, NULL, NULL); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IFileOperation_PerformOperations(operation); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + ok(!file_existsW(path), "File %s should not exists\n", wine_dbgstr_w(path)); + memset(path, 0, sizeof(path)); + PathCombineW(path, to, name1); + ok(file_existsW(path), "File %s should exists\n", wine_dbgstr_w(path)); + IShellItem_Release(shellitem); + IShellItem_Release(shellitem2); + + memset(path, 0, sizeof(path)); + PathCombineW(path, from, name1); + hr = pSHCreateItemFromParsingName(path, NULL, &IID_IShellItem, (void**)&shellitem); + ok(hr == S_OK, "SHCreateItemFromParsingName returned %lx\n", hr); + hr = pSHCreateItemFromParsingName(to, NULL, &IID_IShellItem, (void**)&shellitem2); + ok(hr == S_OK, "SHCreateItemFromParsingName returned %lx\n", hr); + hr = IFileOperation_MoveItem(operation, shellitem, shellitem2, name2, NULL); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IFileOperation_PerformOperations(operation); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + memset(path, 0, sizeof(path)); + PathCombineW(path, to, name2); + ok(file_existsW(path), "File %s should exists\n", wine_dbgstr_w(path)); + memset(path, 0, sizeof(path)); + PathCombineW(path, from, name1); + ok(!file_existsW(path), "File %s should not exists\n", wine_dbgstr_w(path)); + IShellItem_Release(shellitem); + IShellItem_Release(shellitem2); + clean_after_shfo_tests(); + + /* multiply operation */ + PathCombineW(folder, from, name3); + CreateDirectoryW(folder, NULL); + PathCombineW(path, folder, name1); + createTestFileW(path); + hr = pSHCreateItemFromParsingName(path, NULL, &IID_IShellItem, (void**)&shellitem); + ok(hr == S_OK, "SHCreateItemFromParsingName returned %lx\n", hr); + hr = pSHCreateItemFromParsingName(to, NULL, &IID_IShellItem, (void**)&shellitem2); + ok(hr == S_OK, "SHCreateItemFromParsingName returned %lx\n", hr); + hr = IFileOperation_CopyItem(operation, shellitem, shellitem2, name2, NULL); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IFileOperation_DeleteItem(operation, shellitem, NULL); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IFileOperation_PerformOperations(operation); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + memset(path, 0, sizeof(path)); + PathCombineW(path, to, name2); + ok(file_existsW(path), "File %s should exists\n", wine_dbgstr_w(path)); + memset(path, 0, sizeof(path)); + PathCombineW(path, from, name1); + ok(!file_existsW(path), "File %s should not exists\n", wine_dbgstr_w(path)); + IShellItem_Release(shellitem); + IShellItem_Release(shellitem2); + hr = IFileOperation_PerformOperations(operation); + ok(hr == E_UNEXPECTED, "Got hr %#lx.\n", hr); + clean_after_shfo_tests(); + + hr = IFileOperation_GetAnyOperationsAborted(operation, &aborted); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + ok(aborted == FALSE, "Got unexpected abored.\n"); + } + else + { + if (!pSHCreateItemFromParsingName) + win_skip("pSHCreateItemFromParsingName not available\n"); + } + IFileOperation_Release(operation); } @@ -2810,6 +3038,7 @@ START_TEST(shlfileop) CoInitialize(NULL); + init_function_pointers(); test_file_operation(); CoUninitialize(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4817
Esme Povirk (@madewokherd) commented about dlls/shell32/shlfileop.c:
- ULONG ref = InterlockedDecrement(&operation->ref); + struct file_operations *operations = impl_from_IFileOperation(iface); + struct file_operation *ptr, *next; + ULONG ref = InterlockedDecrement(&operations->ref);
TRACE("(%p): ref=%lu.\n", iface, ref);
if (!ref) { - free(operation); + LIST_FOR_EACH_ENTRY_SAFE( ptr, next, &operations->ops, struct file_operation, entry ) + { + if (ptr->pFrom) free((void*)ptr->pFrom); + if (ptr->pTo) free((void*)ptr->pTo); + if (ptr->pNewName) free((void*)ptr->pNewName); + if (ptr->pTemplateName) free((void*)ptr->pTemplateName); If these are really constant strings then we shouldn't be freeing them. I'm guessing the type should just be LPWSTR.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4817#note_58005
Esme Povirk (@madewokherd) commented about dlls/shell32/shlfileop.c:
+ CloseHandle(file); + else + ret = GetLastError(); + } + + return ret; +} +static inline struct file_operations *impl_from_IFileOperation(IFileOperation *iface) +{ + return CONTAINING_RECORD(iface, struct file_operations, IFileOperation_iface); }
static HRESULT WINAPI file_operation_QueryInterface(IFileOperation *iface, REFIID riid, void **out) { - struct file_operation *operation = impl_from_IFileOperation(iface); + struct file_operations *operation = impl_from_IFileOperation(iface); If this struct must be renamed, then it should be in a separate patch.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4817#note_58006
Esme Povirk (@madewokherd) commented about dlls/shell32/shlfileop.c:
+ + ret = new_item(ptr->pTo, ptr->pNewName, ptr->attributes, operations); + + if (ptr->pNewName) free((void*)ptr->pNewName); + if (ptr->pTo) free((void*)ptr->pTo); + if (ptr->pTemplateName) free((void*)ptr->pTemplateName); + list_remove(&ptr->entry); + continue; + } + + shfoW.wFunc = ptr->wFunc; + shfoW.pFrom = ptr->pFrom; + shfoW.pTo = ptr->pTo; + + ret = SHFileOperationW(&shfoW); + operations->fAnyOperationsAborted = shfoW.fAnyOperationsAborted; If fAnyOperationsAborted is already TRUE, this could change it to FALSE, which I don't think we want.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4817#note_58007
Esme Povirk (@madewokherd) commented about dlls/shell32/shlfileop.c:
+ UINT wFunc; + LPCWSTR pFrom; + LPCWSTR pTo; + LPCWSTR pNewName; + LPCWSTR pTemplateName; + DWORD attributes; +}; + +struct file_operations { IFileOperation IFileOperation_iface; LONG ref; + HWND hwnd; + DWORD flags; + BOOL fAnyOperationsAborted; + LPVOID hNameMappings; I don't think this field is ever assigned.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4817#note_58008
On Thu Jan 18 19:38:46 2024 +0000, Esme Povirk wrote:
I don't think this field is ever assigned. I don't see lpszProgressTitle assigned either.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4817#note_58009
Esme Povirk (@madewokherd) commented about dlls/shell32/shlfileop.c:
+}; + +struct file_operations { IFileOperation IFileOperation_iface; LONG ref; + HWND hwnd; + DWORD flags; + BOOL fAnyOperationsAborted; + LPVOID hNameMappings; + LPCWSTR lpszProgressTitle; + struct list ops; };
-static inline struct file_operation *impl_from_IFileOperation(IFileOperation *iface) +static HRESULT add_operation( IShellItem *item, IShellItem *folder, This should probably be named something else since it doesn't actually add the operation to the list.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4817#note_58010
Esme Povirk (@madewokherd) commented about dlls/shell32/shlfileop.c:
-static inline struct file_operation *impl_from_IFileOperation(IFileOperation *iface) +static HRESULT add_operation( IShellItem *item, IShellItem *folder, + LPCWSTR name, UINT func, struct file_operation **out) +{ + LPWSTR tmp, from, to = NULL; + HRESULT ret; + + if (!out) return ERROR_INVALID_PARAMETER; + + ret = IShellItem_GetDisplayName(item, SIGDN_FILESYSPATH, &tmp); + if (S_OK != ret) + { + return ERROR_INVALID_PARAMETER; + } + + from = calloc(lstrlenW(tmp) + 2, sizeof(WCHAR)); If you're just allocating enough to hold the name, I think it should be `+ 1` for the NULL terminator.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4817#note_58011
Esme Povirk (@madewokherd) commented about dlls/shell32/shlfileop.c:
+ + *out = calloc(1, sizeof(struct file_operation)); + if (!*out) + { + ret = E_OUTOFMEMORY; + goto end; + } + + (*out)->wFunc = func; + (*out)->pFrom = from; + (*out)->pTo = to; + return ret; + +end: + if (from) free(from); + if (to) free(to); It's not necessary to check for NULL before calling `free`.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4817#note_58012
Esme Povirk (@madewokherd) commented about dlls/shell32/shlfileop.c:
+ if (attributes & FILE_ATTRIBUTE_DIRECTORY) + { + ret = SHNotifyCreateDirectoryW(path, NULL); + if (S_OK == ret) + { + if (!SetFileAttributesW(path, attributes)) ret = GetLastError(); + }; + } + else + { + file = CreateFileW(path, GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, attributes, NULL); + + if (file != INVALID_HANDLE_VALUE) + CloseHandle(file); + else + ret = GetLastError(); This isn't an HRESULT.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4817#note_58014
On Thu Jan 18 19:55:40 2024 +0000, Esme Povirk wrote:
It's not necessary to check for NULL before calling `free`. Thanks, I will fix it.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4817#note_58116
On Thu Jan 18 19:54:32 2024 +0000, Esme Povirk wrote:
If you're just allocating enough to hold the name, I think it should be `+ 1` for the NULL terminator. Because I use SHFileOperationW, the string must be double-null terminated.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4817#note_58117
On Thu Jan 18 19:42:54 2024 +0000, Esme Povirk wrote:
This should probably be named something else since it doesn't actually add the operation to the list. Thanks, I will rename it.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4817#note_58119
On Thu Jan 18 19:39:43 2024 +0000, Esme Povirk wrote:
I don't see lpszProgressTitle assigned either. Thanks, I will remove it.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4817#note_58121
On Thu Jan 18 19:30:01 2024 +0000, Esme Povirk wrote:
If these are really constant strings then we shouldn't be freeing them. I'm guessing the type should just be LPWSTR. Thanks, I will fix it.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4817#note_58122
On Thu Jan 18 19:33:25 2024 +0000, Esme Povirk wrote:
If this struct must be renamed, then it should be in a separate patch. Thanks, I will fix it.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4817#note_58123
On Thu Jan 18 19:37:51 2024 +0000, Esme Povirk wrote:
If fAnyOperationsAborted is already TRUE, this could change it to FALSE, which I don't think we want. Thanks, I will fix it.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4817#note_58124
On Thu Jan 18 20:12:55 2024 +0000, Esme Povirk wrote:
This isn't an HRESULT. thanks, I will transform with HRESULT_FROM_WIN32.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4817#note_58125
participants (3)
-
Esme Povirk (＠madewokherd) -
Haoyang Chen
-
Haoyang Chen (＠chenhaoyang)