shell32: Fix and simplify the FO_COPY operation, with tests.

diff --git a/dlls/shell32/shlfileop.c b/dlls/shell32/shlfileop.c
index cb7f9958a1a54e3df9b463ff2d7e7a0694de182e..89a61759a912e039cc32b306844bdcdddca5338b 100644 (file)
--- a/dlls/shell32/shlfileop.c
+++ b/dlls/shell32/shlfileop.c
@@ -1173,59 +1173,64 @@ static void create_dest_dirs(LPCWSTR szDestDir)
 }
 
 /* the FO_COPY operation */
-static HRESULT copy_files(FILE_OPERATION *op, const FILE_LIST *flFrom, const FILE_LIST *flTo)
+static HRESULT copy_files(FILE_OPERATION *op, const FILE_LIST *flFrom, FILE_LIST *flTo)
 {
     DWORD i;
     const FILE_ENTRY *entryToCopy;
     const FILE_ENTRY *fileDest = &flTo->feFiles[0];
-    BOOL bCancelIfAnyDirectories = FALSE;
 
     if (flFrom->bAnyDontExist)
         return ERROR_SHELL_INTERNAL_FILE_NOT_FOUND;
 
-    if (op->req->fFlags & FOF_MULTIDESTFILES && flFrom->bAnyFromWildcard)
-        return ERROR_CANCELLED;
-
-    if (!(op->req->fFlags & FOF_MULTIDESTFILES) &&
-        flFrom->dwNumFiles != 1 && flTo->dwNumFiles != 1 &&
-        !flFrom->bAnyFromWildcard)
+    if (op->req->fFlags & FOF_MULTIDESTFILES)
     {
-        return ERROR_CANCELLED;
-    }
+        if (flFrom->bAnyFromWildcard)
+            return ERROR_CANCELLED;
 
-    if (op->req->fFlags & FOF_MULTIDESTFILES && flFrom->dwNumFiles != 1 &&
-        flFrom->dwNumFiles != flTo->dwNumFiles)
-    {
-        return ERROR_CANCELLED;
-    }
+        if (flFrom->dwNumFiles != flTo->dwNumFiles)
+        {
+            if (flFrom->dwNumFiles != 1 && !IsAttribDir(fileDest->attributes))
+                return ERROR_CANCELLED;
 
-    if (flFrom->dwNumFiles > 1 && flTo->dwNumFiles == 1 &&
-        !PathFileExistsW(flTo->feFiles[0].szFullPath) &&
-        IsAttribFile(fileDest->attributes))
-    {
-        bCancelIfAnyDirectories = TRUE;
+            flTo->dwNumFiles = 1;
+        }
+        else if (IsAttribDir(fileDest->attributes))
+        {
+            for (i = 1; i < flTo->dwNumFiles; i++)
+                if (!IsAttribDir(flTo->feFiles[i].attributes) ||
+                    !IsAttribDir(flFrom->feFiles[i].attributes))
+                {
+                    return ERROR_CANCELLED;
+                }
+        }
     }
-
-    if (flFrom->dwNumFiles > 1 && flTo->dwNumFiles == 1 && fileDest->bFromRelative &&
-        !PathFileExistsW(fileDest->szFullPath))
+    else if (flFrom->dwNumFiles != 1)
     {
-        op->req->fAnyOperationsAborted = TRUE;
-        return ERROR_CANCELLED;
-    }
+        if (flTo->dwNumFiles != 1 && !IsAttribDir(fileDest->attributes))
+            return ERROR_CANCELLED;
 
-    if (!(op->req->fFlags & FOF_MULTIDESTFILES) && flFrom->dwNumFiles != 1 &&
-        PathFileExistsW(fileDest->szFullPath) &&
-        IsAttribFile(fileDest->attributes))
-    {
-        return ERROR_CANCELLED;
+        if (PathFileExistsW(fileDest->szFullPath) &&
+            IsAttribFile(fileDest->attributes))
+        {
+            return ERROR_CANCELLED;
+        }
+
+        if (flTo->dwNumFiles == 1 && fileDest->bFromRelative &&
+            !PathFileExistsW(fileDest->szFullPath))
+        {
+            return ERROR_CANCELLED;
+        }
     }
 
     for (i = 0; i < flFrom->dwNumFiles; i++)
     {
         entryToCopy = &flFrom->feFiles[i];
 
-        if (op->req->fFlags & FOF_MULTIDESTFILES)
+        if ((op->req->fFlags & FOF_MULTIDESTFILES) &&
+            flTo->dwNumFiles > 1)
+        {
             fileDest = &flTo->feFiles[i];
+        }
 
         if (IsAttribDir(entryToCopy->attributes) &&
             !lstrcmpiW(entryToCopy->szFullPath, fileDest->szDirectory))
@@ -1233,9 +1238,6 @@ static HRESULT copy_files(FILE_OPERATION *op, const FILE_LIST *flFrom, const FIL
             return ERROR_SUCCESS;
         }
 
-        if (IsAttribDir(entryToCopy->attributes) && bCancelIfAnyDirectories)
-            return ERROR_CANCELLED;
-
         create_dest_dirs(fileDest->szDirectory);
 
         if (!lstrcmpiW(entryToCopy->szFullPath, fileDest->szFullPath))
@@ -1247,8 +1249,7 @@ static HRESULT copy_files(FILE_OPERATION *op, const FILE_LIST *flFrom, const FIL
         }
 
         if ((flFrom->dwNumFiles > 1 && flTo->dwNumFiles == 1) ||
-            (IsAttribDir(fileDest->attributes) &&
-             (flFrom->dwNumFiles == 1 || flFrom->bAnyFromWildcard)))
+            IsAttribDir(fileDest->attributes))
         {
             copy_to_dir(op, entryToCopy, fileDest);
         }

diff --git a/dlls/shell32/tests/shlfileop.c b/dlls/shell32/tests/shlfileop.c
index bc82fc12e06e0cd81c2cd4649a3c95013167ba8b..7650b6484ec636db5bb045118ed410efd3c156ae 100644 (file)
--- a/dlls/shell32/tests/shlfileop.c
+++ b/dlls/shell32/tests/shlfileop.c
@@ -481,6 +481,7 @@ static void test_copy(void)
     CHAR to[5*MAX_PATH];
     FILEOP_FLAGS tmp_flags;
     DWORD retval;
+    LPSTR ptr;
 
     shfo.hwnd = NULL;
     shfo.wFunc = FO_COPY;
@@ -915,24 +916,142 @@ static void test_copy(void)
     createTestFile("one.txt");
     createTestFile("two.txt");
 
+    /* no double-NULL terminator for pTo,
+     * multiple source files,
+     * dest directory does not exist
+     */
+    memset(to, 'a', 2 * MAX_PATH);
+    lstrcpyA(to, "threedir");
+    shfo.pFrom = "one.txt\0two.txt\0";
+    shfo.pTo = to;
+    shfo.fFlags = FOF_NOCONFIRMATION | FOF_SILENT | FOF_NOERRORUI;
+    retval = SHFileOperation(&shfo);
+    ok(retval == ERROR_CANCELLED, "Expected ERROR_CANCELLED, got %d\n", retval);
+    ok(!DeleteFileA("threedir\\one.txt"), "Expected file to not exist\n");
+    ok(!DeleteFileA("threedir\\two.txt"), "Expected file to not exist\n");
+    ok(DeleteFileA("one.txt"), "Expected file to exist\n");
+    ok(DeleteFileA("two.txt"), "Expected file to exist\n");
+    ok(!DeleteFileA("threedir"), "Expected file to not exist\n");
+    ok(!RemoveDirectoryA("threedir"), "Expected dir to not exist\n");
+
+    createTestFile("one.txt");
+    createTestFile("two.txt");
+    CreateDirectoryA("threedir", NULL);
+
+    /* no double-NULL terminator for pTo,
+     * multiple source files,
+     * dest directory does exist
+     */
+    memset(to, 'a', 2 * MAX_PATH);
+    lstrcpyA(to, "threedir");
+    shfo.pFrom = "one.txt\0two.txt\0";
+    shfo.pTo = to;
+    shfo.fFlags = FOF_NOCONFIRMATION | FOF_SILENT | FOF_NOERRORUI;
+    retval = SHFileOperation(&shfo);
+    ok(retval == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", retval);
+    ok(DeleteFileA("threedir\\one.txt"), "Expected file to exist\n");
+    ok(DeleteFileA("threedir\\two.txt"), "Expected file to exist\n");
+    ok(DeleteFileA("one.txt"), "Expected file to exist\n");
+    ok(DeleteFileA("two.txt"), "Expected file to exist\n");
+    ok(RemoveDirectoryA("threedir"), "Expected dir to exist\n");
+
+    createTestFile("one.txt");
+    createTestFile("two.txt");
+
     /* no double-NULL terminator for pTo,
      * multiple source files, FOF_MULTIDESTFILES
+     * dest dir does not exist
      */
     memset(to, 'a', 2 * MAX_PATH);
-    lstrcpyA(to, "three.txt");
+    lstrcpyA(to, "threedir");
     shfo.pFrom = "one.txt\0two.txt\0";
     shfo.pTo = to;
     shfo.fFlags = FOF_MULTIDESTFILES | FOF_NOCONFIRMATION |
                   FOF_SILENT | FOF_NOERRORUI;
     retval = SHFileOperation(&shfo);
     ok(retval == ERROR_CANCELLED, "Expected ERROR_CANCELLED, got %d\n", retval);
+    ok(!DeleteFileA("threedir\\one.txt"), "Expected file to not exist\n");
+    ok(!DeleteFileA("threedir\\two.txt"), "Expected file to not exist\n");
     ok(DeleteFileA("one.txt"), "Expected file to exist\n");
     ok(DeleteFileA("two.txt"), "Expected file to exist\n");
+    ok(!RemoveDirectoryA("threedir"), "Expected dir to not exist\n");
     todo_wine
     {
-        ok(!DeleteFileA("three.txt"), "Expected file to not exist\n");
+        ok(!DeleteFileA("threedir"), "Expected file to not exist\n");
     }
 
+    createTestFile("one.txt");
+    createTestFile("two.txt");
+    CreateDirectoryA("threedir", NULL);
+
+    /* no double-NULL terminator for pTo,
+     * multiple source files, FOF_MULTIDESTFILES
+     * dest dir does exist
+     */
+    memset(to, 'a', 2 * MAX_PATH);
+    lstrcpyA(to, "threedir");
+    ptr = to + lstrlenA(to) + 1;
+    lstrcpyA(ptr, "fourdir");
+    shfo.pFrom = "one.txt\0two.txt\0";
+    shfo.pTo = to;
+    shfo.fFlags = FOF_MULTIDESTFILES | FOF_NOCONFIRMATION |
+                  FOF_SILENT | FOF_NOERRORUI;
+    retval = SHFileOperation(&shfo);
+    ok(retval == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", retval);
+    ok(DeleteFileA("threedir\\one.txt"), "Expected file to exist\n");
+    ok(DeleteFileA("threedir\\two.txt"), "Expected file to exist\n");
+    ok(DeleteFileA("one.txt"), "Expected file to exist\n");
+    ok(DeleteFileA("two.txt"), "Expected file to exist\n");
+    ok(RemoveDirectoryA("threedir"), "Expected dir to exist\n");
+    ok(!DeleteFileA("fourdir"), "Expected file to not exist\n");
+    ok(!RemoveDirectoryA("fourdir"), "Expected dir to not exist\n");
+
+    createTestFile("one.txt");
+    createTestFile("two.txt");
+    CreateDirectoryA("threedir", NULL);
+
+    /* multiple source files, FOF_MULTIDESTFILES
+     * multiple dest files, but first dest dir exists
+     * num files in lists is equal
+     */
+    shfo.pFrom = "one.txt\0two.txt\0";
+    shfo.pTo = "threedir\0fourdir\0";
+    shfo.fFlags = FOF_MULTIDESTFILES | FOF_NOCONFIRMATION |
+                  FOF_SILENT | FOF_NOERRORUI;
+    retval = SHFileOperation(&shfo);
+    ok(retval == ERROR_CANCELLED, "Expected ERROR_CANCELLED, got %d\n", retval);
+    ok(!DeleteFileA("threedir\\one.txt"), "Expected file to not exist\n");
+    ok(!DeleteFileA("threedir\\two.txt"), "Expected file to not exist\n");
+    ok(DeleteFileA("one.txt"), "Expected file to exist\n");
+    ok(DeleteFileA("two.txt"), "Expected file to exist\n");
+    ok(RemoveDirectoryA("threedir"), "Expected dir to exist\n");
+    ok(!DeleteFileA("fourdir"), "Expected file to not exist\n");
+    ok(!RemoveDirectoryA("fourdir"), "Expected dit to not exist\n");
+
+    createTestFile("one.txt");
+    createTestFile("two.txt");
+    CreateDirectoryA("threedir", NULL);
+
+    /* multiple source files, FOF_MULTIDESTFILES
+     * multiple dest files, but first dest dir exists
+     * num files in lists is not equal
+     */
+    shfo.pFrom = "one.txt\0two.txt\0";
+    shfo.pTo = "threedir\0fourdir\0five\0";
+    shfo.fFlags = FOF_MULTIDESTFILES | FOF_NOCONFIRMATION |
+                  FOF_SILENT | FOF_NOERRORUI;
+    retval = SHFileOperation(&shfo);
+    ok(retval == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", retval);
+    ok(DeleteFileA("threedir\\one.txt"), "Expected file to exist\n");
+    ok(DeleteFileA("threedir\\two.txt"), "Expected file to exist\n");
+    ok(DeleteFileA("one.txt"), "Expected file to exist\n");
+    ok(DeleteFileA("two.txt"), "Expected file to exist\n");
+    ok(RemoveDirectoryA("threedir"), "Expected dir to exist\n");
+    ok(!DeleteFileA("fourdir"), "Expected file to not exist\n");
+    ok(!RemoveDirectoryA("fourdir"), "Expected dit to not exist\n");
+    ok(!DeleteFileA("five"), "Expected file to not exist\n");
+    ok(!RemoveDirectoryA("five"), "Expected dit to not exist\n");
+
     createTestFile("aa.txt");
     createTestFile("ab.txt");
     CreateDirectoryA("one", NULL);
@@ -946,6 +1065,8 @@ static void test_copy(void)
     ok(retval == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", retval);
     ok(DeleteFileA("one\\aa.txt"), "Expected file to exist\n");
     ok(DeleteFileA("one\\ab.txt"), "Expected file to exist\n");
+    ok(!DeleteFileA("two\\aa.txt"), "Expected file to not exist\n");
+    ok(!DeleteFileA("two\\ab.txt"), "Expected file to not exist\n");
     ok(DeleteFileA("aa.txt"), "Expected file to exist\n");
     ok(DeleteFileA("ab.txt"), "Expected file to exist\n");
     ok(RemoveDirectoryA("one"), "Expected dir to exist\n");