https://git.reactos.org/?p=reactos.git;a=commitdiff;h=1273bbe417825fa4d6d012c7d19c58637a02da46
commit 1273bbe417825fa4d6d012c7d19c58637a02da46 Author: Huw Campbell <[email protected]> AuthorDate: Sat Aug 5 20:12:36 2023 +1000 Commit: GitHub <[email protected]> CommitDate: Sat Aug 5 13:12:36 2023 +0300 [SHELL32] Improve CFSDropTarget::CopyItems logic (#5481) The previous version resolved the path of the parent then did logic assuming simple pidls. This now resolves each source directly. This change addresses a longstanding TODO in copying and moving to shell folders. It's a simple change, but it removes a bit of code and makes things simpler. Corresponding Wine PR: https://gitlab.winehq.org/wine/wine/-/merge_requests/3360 --- dll/win32/shell32/droptargets/CFSDropTarget.cpp | 99 ++++++++++--------------- 1 file changed, 41 insertions(+), 58 deletions(-) diff --git a/dll/win32/shell32/droptargets/CFSDropTarget.cpp b/dll/win32/shell32/droptargets/CFSDropTarget.cpp index 7dff8fb3885..10aee5e3532 100644 --- a/dll/win32/shell32/droptargets/CFSDropTarget.cpp +++ b/dll/win32/shell32/droptargets/CFSDropTarget.cpp @@ -24,82 +24,65 @@ WINE_DEFAULT_DEBUG_CHANNEL (shell); -/**************************************************************************** - * BuildPathsList - * - * Builds a list of paths like the one used in SHFileOperation from a table of - * PIDLs relative to the given base folder - */ -static WCHAR* BuildPathsList(LPCWSTR wszBasePath, int cidl, LPCITEMIDLIST *pidls) -{ - WCHAR *pwszPathsList = (WCHAR *)HeapAlloc(GetProcessHeap(), 0, MAX_PATH * sizeof(WCHAR) * cidl + 1); - WCHAR *pwszListPos = pwszPathsList; - - for (int i = 0; i < cidl; i++) - { - FileStructW* pDataW = _ILGetFileStructW(pidls[i]); - if (!pDataW) - { - ERR("Mistreating a pidl:\n"); - pdump_always(pidls[i]); - continue; - } - - PathCombineW(pwszListPos, wszBasePath, pDataW->wszName); - pwszListPos += wcslen(pwszListPos) + 1; - } - *pwszListPos = 0; - return pwszPathsList; -} /**************************************************************************** * CFSDropTarget::_CopyItems * - * copies items to this folder - * FIXME: We should not ask the parent folder: 'What is your path', and then manually build paths assuming everything is a simple pidl! - * We should be asking the parent folder: Give me a full name for this pidl (for each child!) + * copies or moves items to this folder */ HRESULT CFSDropTarget::_CopyItems(IShellFolder * pSFFrom, UINT cidl, LPCITEMIDLIST * apidl, BOOL bCopy) { - LPWSTR pszSrcList; - HRESULT hr; - WCHAR wszTargetPath[MAX_PATH + 1]; + HRESULT ret; + WCHAR wszDstPath[MAX_PATH + 1] = {0}; + PWCHAR pwszSrcPathsList = (PWCHAR) HeapAlloc(GetProcessHeap(), 0, MAX_PATH * sizeof(WCHAR) * cidl + 1); + if (!pwszSrcPathsList) + return E_OUTOFMEMORY; - wcscpy(wszTargetPath, m_sPathTarget); - //Double NULL terminate. - wszTargetPath[wcslen(wszTargetPath) + 1] = '\0'; + PWCHAR pwszListPos = pwszSrcPathsList; + STRRET strretFrom; + SHFILEOPSTRUCTW fop; - TRACE ("(%p)->(%p,%u,%p)\n", this, pSFFrom, cidl, apidl); + /* Build a double null terminated list of C strings from source paths */ + for (UINT i = 0; i < cidl; i++) + { + ret = pSFFrom->GetDisplayNameOf(apidl[i], SHGDN_FORPARSING, &strretFrom); + if (FAILED(ret)) + goto cleanup; - STRRET strretFrom; - hr = pSFFrom->GetDisplayNameOf(NULL, SHGDN_FORPARSING, &strretFrom); - if (hr != S_OK) - return hr; + ret = StrRetToBufW(&strretFrom, NULL, pwszListPos, MAX_PATH); + if (FAILED(ret)) + goto cleanup; - pszSrcList = BuildPathsList(strretFrom.pOleStr, cidl, apidl); - TRACE("Source file (just the first) = %s, target path = %s, bCopy: %d\n", debugstr_w(pszSrcList), debugstr_w(m_sPathTarget), bCopy); - CoTaskMemFree(strretFrom.pOleStr); - if (!pszSrcList) - return E_OUTOFMEMORY; + pwszListPos += lstrlenW(pwszListPos) + 1; + } + /* Append the final null. */ + *pwszListPos = L'\0'; - SHFILEOPSTRUCTW op = {0}; - op.pFrom = pszSrcList; - op.pTo = wszTargetPath; - op.hwnd = m_hwndSite; - op.wFunc = bCopy ? FO_COPY : FO_MOVE; - op.fFlags = FOF_ALLOWUNDO | FOF_NOCONFIRMMKDIR; + /* Build a double null terminated target (this path) */ + ret = StringCchCopyW(wszDstPath, MAX_PATH, m_sPathTarget); + if (FAILED(ret)) + goto cleanup; - int res = SHFileOperationW(&op); + wszDstPath[lstrlenW(wszDstPath) + 1] = L'\0'; - HeapFree(GetProcessHeap(), 0, pszSrcList); + ZeroMemory(&fop, sizeof(fop)); + fop.hwnd = m_hwndSite; + fop.wFunc = bCopy ? FO_COPY : FO_MOVE; + fop.pFrom = pwszSrcPathsList; + fop.pTo = wszDstPath; + fop.fFlags = FOF_ALLOWUNDO | FOF_NOCONFIRMMKDIR; + ret = S_OK; - if (res) + if (SHFileOperationW(&fop)) { - ERR("SHFileOperationW failed with 0x%x\n", res); - return E_FAIL; + ERR("SHFileOperationW failed\n"); + ret = E_FAIL; } - return S_OK; + +cleanup: + HeapFree(GetProcessHeap(), 0, pwszSrcPathsList); + return ret; } CFSDropTarget::CFSDropTarget():
