[PATCH v3 0/8] MR4817: shell32: Implement some functions of IFileOperation.
IFileOperation_{SetOperationFlags, NewItem, CopyItem, MoveItem, DeleteItem, GetAnyOperationsAborted, PerformOperations, RenameItem, SetOwnerWindow} -- v3: shell32/tests: Add test for some functions of IFileOperation. shell32: Implement IFileOperation_PerformOperations. shell32: Implement IFileOperation_NewItem. shell32: Implement IFileOperation_{CopyItem,MoveItem,DeleteItem,RenameItem}. shell32: Implement IFileOperation_SetOwnerWindow. shell32: Implement IFileOperation_GetAnyOperationsAborted. shell32: Implement IFileOperation_SetOperationFlags. shell32: Expand to multiple operation fields. https://gitlab.winehq.org/wine/wine/-/merge_requests/4817
From: Haoyang Chen <chenhaoyang(a)kylinos.cn> --- dlls/shell32/shlfileop.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/dlls/shell32/shlfileop.c b/dlls/shell32/shlfileop.c index b4a941187ab..29135abef72 100644 --- a/dlls/shell32/shlfileop.c +++ b/dlls/shell32/shlfileop.c @@ -1798,26 +1798,27 @@ HRESULT WINAPI SHMultiFileProperties(IDataObject *pdtobj, DWORD flags) return E_NOTIMPL; } -struct file_operation +struct file_operations { - IFileOperation IFileOperation_iface; - LONG ref; + IFileOperation IFileOperation_iface; + LONG ref; + struct list ops; }; -static inline struct file_operation *impl_from_IFileOperation(IFileOperation *iface) +static inline struct file_operations *impl_from_IFileOperation(IFileOperation *iface) { - return CONTAINING_RECORD(iface, struct file_operation, 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 *operations = impl_from_IFileOperation(iface); TRACE("(%p, %s, %p).\n", iface, debugstr_guid(riid), out); if (IsEqualIID(&IID_IFileOperation, riid) || IsEqualIID(&IID_IUnknown, riid)) - *out = &operation->IFileOperation_iface; + *out = &operations->IFileOperation_iface; else { FIXME("not implemented for %s.\n", debugstr_guid(riid)); @@ -1831,8 +1832,8 @@ 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); - ULONG ref = InterlockedIncrement(&operation->ref); + struct file_operations *operations = impl_from_IFileOperation(iface); + ULONG ref = InterlockedIncrement(&operations->ref); TRACE("(%p): ref=%lu.\n", iface, ref); @@ -1841,14 +1842,14 @@ 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); + ULONG ref = InterlockedDecrement(&operations->ref); TRACE("(%p): ref=%lu.\n", iface, ref); if (!ref) { - free(operation); + free(operations); } return ref; @@ -2029,7 +2030,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 +2039,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/shlfileop.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/dlls/shell32/shlfileop.c b/dlls/shell32/shlfileop.c index 29135abef72..dce40cbef2d 100644 --- a/dlls/shell32/shlfileop.c +++ b/dlls/shell32/shlfileop.c @@ -1802,6 +1802,7 @@ struct file_operations { IFileOperation IFileOperation_iface; LONG ref; + DWORD flags; struct list ops; }; @@ -1871,9 +1872,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) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4817
From: Haoyang Chen <chenhaoyang(a)kylinos.cn> --- dlls/shell32/shlfileop.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/dlls/shell32/shlfileop.c b/dlls/shell32/shlfileop.c index dce40cbef2d..dcd5d0c2638 100644 --- a/dlls/shell32/shlfileop.c +++ b/dlls/shell32/shlfileop.c @@ -1803,6 +1803,7 @@ struct file_operations IFileOperation IFileOperation_iface; LONG ref; DWORD flags; + BOOL fAnyOperationsAborted; struct list ops; }; @@ -2001,9 +2002,12 @@ static HRESULT WINAPI file_operation_PerformOperations(IFileOperation *iface) 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 = -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4817
From: Haoyang Chen <chenhaoyang(a)kylinos.cn> --- dlls/shell32/shlfileop.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/dlls/shell32/shlfileop.c b/dlls/shell32/shlfileop.c index dcd5d0c2638..ebc784acb93 100644 --- a/dlls/shell32/shlfileop.c +++ b/dlls/shell32/shlfileop.c @@ -1802,6 +1802,7 @@ struct file_operations { IFileOperation IFileOperation_iface; LONG ref; + HWND hwnd; DWORD flags; BOOL fAnyOperationsAborted; struct list ops; @@ -1905,9 +1906,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) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4817
From: Haoyang Chen <chenhaoyang(a)kylinos.cn> --- dlls/shell32/shlfileop.c | 137 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 129 insertions(+), 8 deletions(-) diff --git a/dlls/shell32/shlfileop.c b/dlls/shell32/shlfileop.c index ebc784acb93..6dfc53b29ce 100644 --- a/dlls/shell32/shlfileop.c +++ b/dlls/shell32/shlfileop.c @@ -1798,6 +1798,17 @@ HRESULT WINAPI SHMultiFileProperties(IDataObject *pdtobj, DWORD flags) return E_NOTIMPL; } +struct file_operation +{ + struct list entry; + UINT wFunc; + LPWSTR pFrom; + LPWSTR pTo; + LPWSTR pNewName; + LPWSTR pTemplateName; + DWORD attributes; +}; + struct file_operations { IFileOperation IFileOperation_iface; @@ -1808,6 +1819,80 @@ struct file_operations struct list ops; }; +static HRESULT create_operation( IShellItem *item, IShellItem *folder, + LPCWSTR name, UINT func, struct file_operation **out) +{ + LPWSTR tmp, from = NULL, 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: + free(from); + free(to); + + return ret; +} + static inline struct file_operations *impl_from_IFileOperation(IFileOperation *iface) { return CONTAINING_RECORD(iface, struct file_operations, IFileOperation_iface); @@ -1846,12 +1931,21 @@ static ULONG WINAPI file_operation_AddRef(IFileOperation *iface) static ULONG WINAPI file_operation_Release(IFileOperation *iface) { 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) { + 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); } @@ -1932,9 +2026,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 = create_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) @@ -1947,9 +2048,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 = create_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) @@ -1962,9 +2069,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 = create_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) @@ -1977,9 +2091,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 = create_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) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4817
From: Haoyang Chen <chenhaoyang(a)kylinos.cn> --- dlls/shell32/shlfileop.c | 70 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 68 insertions(+), 2 deletions(-) diff --git a/dlls/shell32/shlfileop.c b/dlls/shell32/shlfileop.c index 6dfc53b29ce..6ecb0da6e8c 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); @@ -2113,10 +2115,74 @@ 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 = NULL, 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: + free(new); + free(temp); + free(to); + + return ret; } static HRESULT WINAPI file_operation_PerformOperations(IFileOperation *iface) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4817
From: Haoyang Chen <chenhaoyang(a)kylinos.cn> --- dlls/shell32/shlfileop.c | 82 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 80 insertions(+), 2 deletions(-) diff --git a/dlls/shell32/shlfileop.c b/dlls/shell32/shlfileop.c index 6ecb0da6e8c..d4a4570a0d4 100644 --- a/dlls/shell32/shlfileop.c +++ b/dlls/shell32/shlfileop.c @@ -1895,6 +1895,45 @@ end: 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 = HRESULT_FROM_WIN32(GetLastError()); + }; + } + else + { + file = CreateFileW(path, GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, attributes, NULL); + + if (file != INVALID_HANDLE_VALUE) + CloseHandle(file); + else + ret = HRESULT_FROM_WIN32(GetLastError()); + } + + return ret; +} static inline struct file_operations *impl_from_IFileOperation(IFileOperation *iface) { return CONTAINING_RECORD(iface, struct file_operations, IFileOperation_iface); @@ -2187,9 +2226,48 @@ end: 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; + + 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); + if (!operations->fAnyOperationsAborted) + 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) -- 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
On Fri Jan 19 08:15:51 2024 +0000, Jactry Zeng wrote:
Hi, Is it possible to split it into a series of smaller patches, for example, implement one API in one patch? This should make it easier to review. It will be nice to split the test patch either. Ok, I will split it.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4817#note_58150
Esme Povirk (@madewokherd) commented about dlls/shell32/shlfileop.c:
return E_NOTIMPL; }
-struct file_operation +struct file_operations { IFileOperation IFileOperation_iface; LONG ref; + struct list ops;
Please introduce this field in the first commit that uses it. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4817#note_58411
Esme Povirk (@madewokherd) commented about dlls/shell32/shlfileop.c:
static ULONG WINAPI file_operation_Release(IFileOperation *iface) { 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) { + 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); These if statements and `(void*)` casts should not be needed.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4817#note_58412
Esme Povirk (@madewokherd) commented about dlls/shell32/shlfileop.c:
+ shfoW.fFlags = operations->flags; + + 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); There should probably be a common "free_item" function. I'm noticing some inconsistency, in that this case doesn't `free(ptr)` and not every case frees all string fields.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4817#note_58414
Esme Povirk (@madewokherd) commented about dlls/shell32/shlfileop.c:
+ 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); `SHNotifyCreateDirectory` returns a win32 error code, not an HRESULT.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4817#note_58415
Esme Povirk (@madewokherd) commented about dlls/shell32/shlfileop.c:
- return E_NOTIMPL; + TRACE("\n"); + + shfoW.hwnd = operations->hwnd; + shfoW.fFlags = operations->flags; + + 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);
I'm noticing that `ret` is ignored for all operations except the last one. If an operation in the middle fails, does it really make sense to keep going and not report an error? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4817#note_58417
Esme Povirk (@madewokherd) commented about dlls/shell32/tests/shlfileop.c:
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);
I'm not sure whether we can rely on write access to the current directory. It may be necessary to create a temporary directory. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4817#note_58418
I'm not sure how much it matters, but I would have split the patches like this: First, implement PerformOperations along with one other method, so that PerformOperations can do something. Then implement the other methods in separate commits. That way, all of the commits make a functional change, and the related code for each function can be added in each commit. For example, the commit that implements NewItem can also introduce the new_item function. Since the way new_item works depends on the item that NewItem creates, it'd be easier to review them together. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4817#note_58422
On Mon Jan 22 16:36:17 2024 +0000, Esme Povirk wrote:
I'm noticing that `ret` is ignored for all operations except the last one. If an operation in the middle fails, does it really make sense to keep going and not report an error? I tested on Windows ， they will keep going and report errors.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4817#note_58567
On Mon Jan 22 16:45:10 2024 +0000, Esme Povirk wrote:
I'm not sure whether we can rely on write access to the current directory. It may be necessary to create a temporary directory. In other tests, a is also used. If I use GetTempPath, test will fail in local. I do not think it should be modified.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4817#note_58568
participants (3)
-
Esme Povirk (＠madewokherd) -
Haoyang Chen
-
Haoyang Chen (＠chenhaoyang)