[PATCH v2 0/6] MR7775: shell32: Test and rework rename operation.

[Ziqing Hui (＠zhui)]

--
  v2: shell32: Rework rename_files.
      shell32: Don't parse wildcard for rename operation.
      shell32: Rework add_file_entry, add more parameters.
      shell32/tests: Avoid showing UI when testing.
      shell32/tests: Test NULL and empty file name for SHFileOperation.
      shell32/tests: Add more tests to test_rename.

https://gitlab.winehq.org/wine/wine/-/merge_requests/7775

[PATCH v2 1/6] shell32/tests: Add more tests to test_rename.

[Ziqing Hui]

From: Ziqing Hui

---
 dlls/shell32/tests/shlfileop.c | 99 +++++++++++++++++++++++++++++++---
 1 file changed, 92 insertions(+), 7 deletions(-)

diff --git a/dlls/shell32/tests/shlfileop.c b/dlls/shell32/tests/shlfileop.c
index 0debe098c6d..0710285b869 100644
--- a/dlls/shell32/tests/shlfileop.c
+++ b/dlls/shell32/tests/shlfileop.c
@@ -730,13 +730,21 @@ static void test_rename(void)
 {
     char from[5 * MAX_PATH], to[5 * MAX_PATH];

-    /* Rename a file to a dir. */
+    /* Rename a file to an existing dir. */
     set_curr_dir_path(from, "test1.txt\0");
     set_curr_dir_path(to, "test4.txt\0");
     check_file_operation(FO_RENAME, FOF_NO_UI, from, to,
             DE_FILEDESTISFLD, FALSE, TRUE, FALSE);
     ok(file_exists("test1.txt"), "The file is renamed\n");

+    /* Rename a dir to an existing file. */
+    set_curr_dir_path(from, "test4.txt\0");
+    set_curr_dir_path(to, "test1.txt\0");
+    check_file_operation(FO_RENAME, FOF_NO_UI, from, to,
+            DE_FLDDESTISFILE, FALSE, TRUE, FALSE);
+    ok(dir_exists("test4.txt"), "The file is renamed\n");
+
+    /* Rename file to a different directory. */
     set_curr_dir_path(from, "test3.txt\0");
     set_curr_dir_path(to, "test4.txt\test1.txt\0");
     check_file_operation(FO_RENAME, FOF_NO_UI, from, to,
@@ -758,6 +766,13 @@ static void test_rename(void)
             DE_MANYSRC1DEST, FALSE, TRUE, FALSE);
     ok(file_exists("test1.txt"), "The file is not renamed - many files are specified\n");

+    /* Sources outnumber targets, with FOF_MULTIDESTFILES. */
+    set_curr_dir_path(from, "test1.txt\0test2.txt\0test4.txt\0");
+    set_curr_dir_path(to, "test6.txt\0test7.txt\0");
+    check_file_operation(FO_RENAME, FOF_NO_UI | FOF_MULTIDESTFILES, from, to,
+            DE_MANYSRC1DEST, FALSE, TRUE, FALSE);
+    ok(file_exists("test1.txt"), "The file is not renamed - many files are specified\n");
+
     /* Rename a file. */
     set_curr_dir_path(from, "test1.txt\0");
     set_curr_dir_path(to, "test6.txt\0");
@@ -793,6 +808,16 @@ static void test_rename(void)
     ok(file_exists("test2.txt"), "Expected test2.txt to exist\n");
     ok(!file_exists("a.txt"), "Expected a.txt to not exist\n");

+    /* Rename a file to multiple files. */
+    check_file_operation(FO_RENAME, FOF_NO_UI,
+            "test1.txt\0", "a.txt\0b.txt\0",
+            ERROR_SUCCESS, FALSE, TRUE, TRUE);
+    todo_wine
+    ok(!file_exists("test1.txt"), "Expected test1.txt to not exist\n");
+    todo_wine
+    ok(DeleteFileA("a.txt"), "Expected a.txt to exist\n");
+    ok(!DeleteFileA("b.txt"), "Expected b.txt to not exist\n");
+
     /* Rename a nonexistent file. */
     check_file_operation(FO_RENAME, FOF_NO_UI,
             "idontexist\0", "newfile\0",
@@ -800,26 +825,86 @@ static void test_rename(void)
     ok(!file_exists("newfile"), "Expected newfile to not exist\n");

/* Target already exists. */
+    createTestFile("test1.txt");
     check_file_operation(FO_RENAME, FOF_NO_UI,
             "test1.txt\0", "test2.txt\0",
             ERROR_SUCCESS, FALSE, TRUE, FALSE);
+    check_file_operation(FO_RENAME, FOF_NO_UI,
+            "testdir2\0", "testdir4\0",
+            ERROR_SUCCESS, FALSE, TRUE, FALSE);

-    /* Empty target. */
-    createTestFile("test1.txt");
+    /* Empty source or target. */
+    clean_after_shfo_tests();
+    init_shfo_tests();
+    check_file_operation(FO_RENAME, FOF_NO_UI,
+            "\0", "test1.txt",
+            DE_MANYSRC1DEST, FALSE, TRUE, TRUE);
+    check_file_operation(FO_RENAME, FOF_NO_UI,
+            "\0", "testdir2",
+            DE_MANYSRC1DEST, FALSE, TRUE, TRUE);
+    check_file_operation(FO_RENAME, FOF_NO_UI,
+            "\0", "nonexistence",
+            DE_MANYSRC1DEST, FALSE, TRUE, TRUE);
+    ok(!file_exists("nonexistence"), "Expected nonexistence to not exist\n");
+    check_file_operation(FO_RENAME, FOF_NO_UI,
+            "testdir2\0", "\0",
+            DE_DIFFDIR, FALSE, TRUE, TRUE);
+    ok(file_exists("testdir2"), "Expected testdir2 to exist\n");
     check_file_operation(FO_RENAME, FOF_NO_UI,
             "test1.txt\0", "\0",
             DE_DIFFDIR, FALSE, TRUE, TRUE);
     ok(file_exists("test1.txt"), "Expected test1.txt to exist\n");
-
-    /* Empty source. */
+    check_file_operation(FO_RENAME, FOF_NO_UI,
+            "nonexistence\0", "\0",
+            ERROR_FILE_NOT_FOUND, FALSE, TRUE, TRUE);
     check_file_operation(FO_RENAME, FOF_NO_UI,
             "\0", "\0",
             DE_MANYSRC1DEST, FALSE, TRUE, TRUE);

-    /* pFrom is NULL. */
+    /* NULL source or target. */
+    clean_after_shfo_tests();
+    init_shfo_tests();
+    check_file_operation(FO_RENAME, FOF_NO_UI,
+            NULL, "test1.txt\0",
+            ERROR_INVALID_PARAMETER, 0xdeadbeef, FALSE, FALSE);
     check_file_operation(FO_RENAME, FOF_NO_UI,
-            NULL, "\0",
+            NULL, "testdir2\0",
             ERROR_INVALID_PARAMETER, 0xdeadbeef, FALSE, FALSE);
+    check_file_operation(FO_RENAME, FOF_NO_UI,
+            NULL, "nonexistent\0",
+            ERROR_INVALID_PARAMETER, 0xdeadbeef, FALSE, FALSE);
+    check_file_operation(FO_RENAME, FOF_NO_UI,
+            "test1.txt\0", NULL,
+            ERROR_ACCESS_DENIED, FALSE, TRUE, TRUE);
+    check_file_operation(FO_RENAME, FOF_NO_UI,
+            "testdir2\0", NULL,
+            ERROR_ACCESS_DENIED, FALSE, TRUE, TRUE);
+    check_file_operation(FO_RENAME, FOF_NO_UI,
+            "nonexistent\0", NULL,
+            ERROR_ACCESS_DENIED, FALSE, TRUE, TRUE);
+    check_file_operation(FO_RENAME, FOF_NO_UI,
+            NULL, NULL,
+            ERROR_INVALID_PARAMETER, 0xdeadbeef, FALSE, FALSE);
+
+    /* Wildcard source or target. */
+    clean_after_shfo_tests();
+    init_shfo_tests();
+    check_file_operation(FO_RENAME, FOF_NO_UI,
+            "test?.txt\0", "test_target.txt\0",
+            DE_MANYSRC1DEST, FALSE, TRUE, FALSE);
+    ok(file_exists("test1.txt"), "Expected test1.txt to exist.\n");
+    ok(!DeleteFileA("test_target.txt"), "Expected test_target.txt to not exist.\n");
+
+    check_file_operation(FO_RENAME, FOF_NO_UI,
+            "t?st1.txt\0", "test_target.txt\0",
+            DE_MANYSRC1DEST, FALSE, TRUE, FALSE);
+    ok(file_exists("test1.txt"), "Expected test1.txt to exist.\n");
+    ok(!DeleteFileA("test_target.txt"), "Expected test_target.txt to not exist.\n");
+
+    check_file_operation(FO_RENAME, FOF_NO_UI,
+            "test1.txt\0", "test?.txt\0",
+            ERROR_INVALID_NAME, FALSE, TRUE, TRUE);
+    ok(file_exists("test1.txt"), "Expected test1.txt to exist.\n");
 }

/* tests the FO_COPY action */

[PATCH v2 2/6] shell32/tests: Test NULL and empty file name for SHFileOperation.

[Ziqing Hui]

From: Ziqing Hui

---
 dlls/shell32/tests/shlfileop.c | 130 +++++++++++++++++++++++++++------
 1 file changed, 106 insertions(+), 24 deletions(-)

diff --git a/dlls/shell32/tests/shlfileop.c b/dlls/shell32/tests/shlfileop.c
index 0710285b869..5d3a8c923b8 100644
--- a/dlls/shell32/tests/shlfileop.c
+++ b/dlls/shell32/tests/shlfileop.c
@@ -678,6 +678,11 @@ static void test_delete(void)
             ERROR_SUCCESS, FALSE, TRUE, TRUE);
     ok(file_exists("test1.txt"), "Expected test1.txt to exist\n");

+    /* NULL filename. */
+    init_shfo_tests();
+    check_file_operation(FO_DELETE, FOF_NO_UI, NULL, NULL,
+            ERROR_INVALID_PARAMETER, 0xdeadbeef, FALSE, FALSE);
+
     /* Invalid function. */
     init_shfo_tests();
     check_file_operation(0, FOF_NO_UI, "test1.txt\0", NULL,
@@ -871,7 +876,7 @@ static void test_rename(void)
             NULL, "testdir2\0",
             ERROR_INVALID_PARAMETER, 0xdeadbeef, FALSE, FALSE);
     check_file_operation(FO_RENAME, FOF_NO_UI,
-            NULL, "nonexistent\0",
+            NULL, "nonexistence\0",
             ERROR_INVALID_PARAMETER, 0xdeadbeef, FALSE, FALSE);
     check_file_operation(FO_RENAME, FOF_NO_UI,
             "test1.txt\0", NULL,
@@ -880,7 +885,7 @@ static void test_rename(void)
             "testdir2\0", NULL,
             ERROR_ACCESS_DENIED, FALSE, TRUE, TRUE);
     check_file_operation(FO_RENAME, FOF_NO_UI,
-            "nonexistent\0", NULL,
+            "nonexistence\0", NULL,
             ERROR_ACCESS_DENIED, FALSE, TRUE, TRUE);
     check_file_operation(FO_RENAME, FOF_NO_UI,
             NULL, NULL,
@@ -1614,17 +1619,58 @@ static void test_copy(void)
     ok(RemoveDirectoryA("one"), "Expected dir to exist\n");
     ok(RemoveDirectoryA("two"), "Expected dir to exist\n");

-    /* pTo is an empty string  */
-    CreateDirectoryA("dir", NULL);
-    createTestFile("dir\abcdefgh.abc");
-    shfo.pFrom = "dir\abcdefgh.abc\0";
-    shfo.pTo = "\0";
-    shfo.fFlags = FOF_NOCONFIRMATION | FOF_SILENT | FOF_NOERRORUI;
-    retval = SHFileOperationA(&shfo);
-    ok(retval == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %ld\n", retval);
-    ok(DeleteFileA("abcdefgh.abc"), "Expected file to exist\n");
-    ok(DeleteFileA("dir\abcdefgh.abc"), "Expected file to exist\n");
-    ok(RemoveDirectoryA("dir"), "Expected dir to exist\n");
+    /* Empty source or target. */
+    clean_after_shfo_tests();
+    init_shfo_tests();
+    check_file_operation(FO_COPY, FOF_NO_UI,
+            "testdir2\one.txt\0", "\0",
+            ERROR_SUCCESS, FALSE, FALSE, FALSE);
+    ok(DeleteFileA("one.txt"), "Expected file to exist\n");
+    check_file_operation(FO_COPY, FOF_NO_UI,
+            "testdir2\nested\0", "\0",
+            ERROR_SUCCESS, FALSE, FALSE, FALSE);
+    ok(DeleteFileA("nested\two.txt"), "Expected file to exist\n");
+    ok(RemoveDirectoryA("nested"), "Expected dir to exist\n");
+    check_file_operation(FO_COPY, FOF_NO_UI,
+            "nonexistence\0", "\0",
+            ERROR_FILE_NOT_FOUND, FALSE, TRUE, FALSE);
+    check_file_operation(FO_COPY, FOF_NO_UI,
+            "\0", "testdir2\0",
+            ERROR_SUCCESS, FALSE, TRUE, TRUE);
+    check_file_operation(FO_COPY, FOF_NO_UI,
+            "\0", "testfile.txt\0",
+            ERROR_SUCCESS, FALSE, TRUE, TRUE);
+    check_file_operation(FO_COPY, FOF_NO_UI,
+            "\0", "nonexistence",
+            ERROR_SUCCESS, FALSE, TRUE, TRUE);
+    check_file_operation(FO_COPY, FOF_NO_UI,
+            "\0", "\0",
+            ERROR_SUCCESS, FALSE, TRUE, TRUE);
+
+    /* NULL source or target. */
+    clean_after_shfo_tests();
+    init_shfo_tests();
+    check_file_operation(FO_COPY, FOF_NO_UI,
+            NULL, "test1.txt\0",
+            ERROR_INVALID_PARAMETER, 0xdeadbeef, FALSE, FALSE);
+    check_file_operation(FO_COPY, FOF_NO_UI,
+            NULL, "testdir2\0",
+            ERROR_INVALID_PARAMETER, 0xdeadbeef, FALSE, FALSE);
+    check_file_operation(FO_COPY, FOF_NO_UI,
+            NULL, "nonexistence\0",
+            ERROR_INVALID_PARAMETER, 0xdeadbeef, FALSE, FALSE);
+    check_file_operation(FO_COPY, FOF_NO_UI,
+            "test1.txt\0", NULL,
+            ERROR_ACCESS_DENIED, FALSE, TRUE, FALSE);
+    check_file_operation(FO_COPY, FOF_NO_UI,
+            "testdir2\0", NULL,
+            ERROR_ACCESS_DENIED, FALSE, TRUE, FALSE);
+    check_file_operation(FO_COPY, FOF_NO_UI,
+            "nonexistence\0", NULL,
+            ERROR_ACCESS_DENIED, FALSE, TRUE, FALSE);
+    check_file_operation(FO_COPY, FOF_NO_UI,
+            NULL, NULL,
+            ERROR_INVALID_PARAMETER, 0xdeadbeef, FALSE, FALSE);

/* Check last error after a successful file operation. */
     clean_after_shfo_tests();
@@ -1940,22 +1986,58 @@ static void test_move(void)
     RemoveDirectoryA("test4.txt");
     RemoveDirectoryA("test5.txt");

-    /* Empty source. */
+    /* Empty source or target. */
+    clean_after_shfo_tests();
     init_shfo_tests();
-    set_curr_dir_path(from, "\0\0");
-    set_curr_dir_path(to, "test6.txt\0\0");
-    check_file_operation(FO_MOVE, FOF_NO_UI | FOF_MULTIDESTFILES, from, to,
+    check_file_operation(FO_MOVE, FOF_NO_UI,
+            "\0", "test1.txt\0",
+            ERROR_SUCCESS, FALSE, TRUE, TRUE);
+    check_file_operation(FO_MOVE, FOF_NO_UI,
+            "\0", "testdir2\0",
+            ERROR_SUCCESS, FALSE, TRUE, TRUE);
+    check_file_operation(FO_MOVE, FOF_NO_UI,
+            "\0", "nonexistence\0",
+            ERROR_SUCCESS, FALSE, TRUE, TRUE);
+    ok(!file_exists("nonexistence"), "Expected nonexistence to not exist.\n");
+    check_file_operation(FO_MOVE, FOF_NO_UI,
+            "test1.txt", "\0",
+            ERROR_FILE_NOT_FOUND, FALSE, FALSE, FALSE);
+    check_file_operation(FO_MOVE, FOF_NO_UI,
+            "testdir2", "\0",
+            DE_DESTSAMETREE, FALSE, TRUE, FALSE);
+    check_file_operation(FO_MOVE, FOF_NO_UI,
+            "nonexistence", "\0",
+            ERROR_FILE_NOT_FOUND, FALSE, FALSE, FALSE);
+    check_file_operation(FO_MOVE, FOF_NO_UI,
+            "\0", "\0",
             ERROR_SUCCESS, FALSE, TRUE, TRUE);
-    ok(!file_exists("test6.txt"), "The file should not exist\n");

-    /* Empty target. */
+    /* NULL source or target. */
+    clean_after_shfo_tests();
     init_shfo_tests();
-    set_curr_dir_path(from, "test1\0\0");
-    set_curr_dir_path(to, "\0\0");
-    check_file_operation(FO_MOVE, FOF_NO_UI | FOF_MULTIDESTFILES, from, to,
-            ERROR_FILE_NOT_FOUND, FALSE, FALSE, FALSE);
-    ok(!file_exists("test6.txt"), "The file should not exist\n");
+    check_file_operation(FO_MOVE, FOF_NO_UI,
+            NULL, "test1.txt\0",
+            ERROR_INVALID_PARAMETER, 0xdeadbeef, FALSE, FALSE);
+    check_file_operation(FO_MOVE, FOF_NO_UI,
+            NULL, "testdir2\0",
+            ERROR_INVALID_PARAMETER, 0xdeadbeef, FALSE, FALSE);
+    check_file_operation(FO_MOVE, FOF_NO_UI,
+            NULL, "nonexistence\0",
+            ERROR_INVALID_PARAMETER, 0xdeadbeef, FALSE, FALSE);
+    check_file_operation(FO_MOVE, FOF_NO_UI,
+            "test1.txt\0", NULL,
+            ERROR_ACCESS_DENIED, FALSE, TRUE, FALSE);
+    check_file_operation(FO_MOVE, FOF_NO_UI,
+            "testdir2\0", NULL,
+            ERROR_ACCESS_DENIED, FALSE, TRUE, FALSE);
+    check_file_operation(FO_MOVE, FOF_NO_UI,
+            "nonexistent\0", NULL,
+            ERROR_ACCESS_DENIED, FALSE, TRUE, FALSE);
+    check_file_operation(FO_MOVE, FOF_NO_UI,
+            NULL, NULL,
+            ERROR_INVALID_PARAMETER, 0xdeadbeef, FALSE, FALSE);

+    clean_after_shfo_tests();
     init_shfo_tests();

set_curr_dir_path(from, "test3.txt\0");

[PATCH v2 3/6] shell32/tests: Avoid showing UI when testing.

[Ziqing Hui]

From: Ziqing Hui

---
 dlls/shell32/tests/shlfileop.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/dlls/shell32/tests/shlfileop.c b/dlls/shell32/tests/shlfileop.c
index 5d3a8c923b8..30fecb5d52a 100644
--- a/dlls/shell32/tests/shlfileop.c
+++ b/dlls/shell32/tests/shlfileop.c
@@ -1195,20 +1195,16 @@ static void test_copy(void)
     ok(DeleteFileA("testdir2\test4.txt\a.txt"), "Expected a.txt to exist\n");
     ok(RemoveDirectoryA("testdir2\test4.txt"), "Expected testdir2\test4.txt to exist\n");

-    /* copy one directory and a file in that dir to another dir */
-    shfo.pFrom = "test4.txt\0test4.txt\a.txt\0";
-    shfo.pTo = "testdir2\0";
-    retval = SHFileOperationA(&shfo);
-    ok(retval == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %ld\n", retval);
+    /* Copy one dir and a file in that dir to another dir. */
+    check_file_operation(FO_COPY, FOF_NO_UI,
+            "test4.txt\a.txt\0test4.txt\0", "testdir2\0",
+            ERROR_SUCCESS, FALSE, FALSE, FALSE);
     ok(DeleteFileA("testdir2\test4.txt\a.txt"), "Expected a.txt to exist\n");
     ok(DeleteFileA("testdir2\a.txt"), "Expected testdir2\a.txt to exist\n");

-    /* copy a file in a directory first, and then the directory to a nonexistent dir */
-    shfo.pFrom = "test4.txt\a.txt\0test4.txt\0";
-    shfo.pTo = "nonexistent\0";
-    retval = SHFileOperationA(&shfo);
-    todo_wine
-    ok(retval == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %ld\n", retval);
+    check_file_operation(FO_COPY, FOF_NO_UI,
+            "test4.txt\a.txt\0test4.txt\0", "nonexistent\0",
+            ERROR_SUCCESS, FALSE, TRUE, TRUE);
     todo_wine
     ok(DeleteFileA("nonexistent\test4.txt\a.txt"), "Expected nonexistent\test4.txt\a.txt to exist\n");
     RemoveDirectoryA("nonexistent\test4.txt");

[PATCH v2 4/6] shell32: Rework add_file_entry, add more parameters.

[Ziqing Hui]

From: Ziqing Hui

---
 dlls/shell32/shlfileop.c | 75 ++++++++++++++++++++--------------------
 1 file changed, 38 insertions(+), 37 deletions(-)

diff --git a/dlls/shell32/shlfileop.c b/dlls/shell32/shlfileop.c
index 4fb0642fc5d..24396e7c585 100644
--- a/dlls/shell32/shlfileop.c
+++ b/dlls/shell32/shlfileop.c
@@ -932,28 +932,43 @@ static inline void grow_list(FILE_LIST *list)
     list->num_alloc *= 2;
 }

-/* adds a file to the FILE_ENTRY struct
- */
-static void add_file_to_entry(FILE_ENTRY *feFile, LPCWSTR szFile)
+static void add_file_entry(FILE_LIST *file_list, DWORD index,
+        const WCHAR *file_name, DWORD attributes, BOOL from_relative, BOOL from_wildcard)
 {
-    DWORD dwLen = lstrlenW(szFile) + 1;
-    LPCWSTR ptr;
+    size_t file_name_len = wcslen(file_name) + 1;
+    FILE_ENTRY *file_entry;
+    const WCHAR *ptr;
+
+    if (index >= file_list->num_alloc)
+        grow_list(file_list);
+    file_entry = &file_list->feFiles[index];

-    feFile->szFullPath = malloc(dwLen * sizeof(WCHAR));
-    lstrcpyW(feFile->szFullPath, szFile);
+    file_entry->szFullPath = malloc(file_name_len * sizeof(WCHAR));
+    wcscpy(file_entry->szFullPath, file_name);

-    ptr = StrRChrW(szFile, NULL, '\');
+    ptr = wcsrchr(file_name, '\');
     if (ptr)
     {
-        dwLen = ptr - szFile + 1;
-        feFile->szDirectory = malloc(dwLen * sizeof(WCHAR));
-        lstrcpynW(feFile->szDirectory, szFile, dwLen);
+        file_name_len = ptr - file_name + 1;
+        file_entry->szDirectory = malloc(file_name_len * sizeof(WCHAR));
+        lstrcpynW(file_entry->szDirectory, file_name, file_name_len);

-        dwLen = lstrlenW(feFile->szFullPath) - dwLen + 1;
-        feFile->szFilename = malloc(dwLen * sizeof(WCHAR));
-        lstrcpyW(feFile->szFilename, ptr + 1); /* skip over backslash */
+        file_name_len = wcslen(file_entry->szFullPath) - file_name_len + 1;
+        file_entry->szFilename = malloc(file_name_len * sizeof(WCHAR));
+        lstrcpyW(file_entry->szFilename, ptr + 1); /* Skip over backslash. */
     }
-    feFile->bFromWildcard = FALSE;
+
+    file_entry->attributes = attributes;
+    file_entry->bFromRelative = from_relative;
+    file_entry->bFromWildcard = from_wildcard;
+    file_entry->bExists = (attributes != INVALID_FILE_ATTRIBUTES);
+
+    if (IsAttribDir(attributes))
+        file_list->bAnyDirectories = TRUE;
+    if (from_wildcard)
+        file_list->bAnyFromWildcard = TRUE;
+    if (!file_entry->bExists)
+        file_list->bAnyDontExist = TRUE;
 }

static LPWSTR wildcard_to_file(LPCWSTR szWildCard, LPCWSTR szFileName)
@@ -974,11 +989,10 @@ static LPWSTR wildcard_to_file(LPCWSTR szWildCard, LPCWSTR szFileName)
     return szFullPath;
 }

-static void parse_wildcard_files(FILE_LIST *flList, LPCWSTR szFile, LPDWORD pdwListIndex)
+static void parse_wildcard_files(FILE_LIST *flList, LPCWSTR szFile, LPDWORD pdwListIndex, BOOL from_relative)
 {
     WIN32_FIND_DATAW wfd;
     HANDLE hFile = FindFirstFileW(szFile, &wfd);
-    FILE_ENTRY *file;
     LPWSTR szFullPath;
     BOOL res;

@@ -987,13 +1001,8 @@ static void parse_wildcard_files(FILE_LIST *flList, LPCWSTR szFile, LPDWORD pdwL
     for (res = TRUE; res; res = FindNextFileW(hFile, &wfd))
     {
         if (IsDotDir(wfd.cFileName)) continue;
-        if (*pdwListIndex >= flList->num_alloc) grow_list( flList );
         szFullPath = wildcard_to_file(szFile, wfd.cFileName);
-        file = &flList->feFiles[(*pdwListIndex)++];
-        add_file_to_entry(file, szFullPath);
-        file->bFromWildcard = TRUE;
-        file->attributes = wfd.dwFileAttributes;
-        if (IsAttribDir(file->attributes)) flList->bAnyDirectories = TRUE;
+        add_file_entry(flList, (*pdwListIndex)++, szFullPath, wfd.dwFileAttributes, from_relative, TRUE);
         free(szFullPath);
     }

@@ -1025,38 +1034,30 @@ static HRESULT parse_file_list(FILE_LIST *flList, LPCWSTR szFiles)

while (*ptr)
     {
-        if (i >= flList->num_alloc) grow_list( flList );
+        BOOL from_relative = PathIsRelativeW(ptr), from_wildcard = !!wcspbrk(ptr, L"*?");

/* change relative to absolute path */
-        if (PathIsRelativeW(ptr))
+        if (from_relative)
         {
             GetCurrentDirectoryW(MAX_PATH, szCurFile);
             PathCombineW(szCurFile, szCurFile, ptr);
-            flList->feFiles[i].bFromRelative = TRUE;
         }
         else
         {
             lstrcpyW(szCurFile, ptr);
-            flList->feFiles[i].bFromRelative = FALSE;
         }

for (p = szCurFile; *p; p++) if (*p == '/') *p = '\';

/* parse wildcard files if they are in the filename */
-        if (StrPBrkW(szCurFile, L"*?"))
+        if (from_wildcard)
         {
-            parse_wildcard_files(flList, szCurFile, &i);
-            flList->bAnyFromWildcard = TRUE;
+            parse_wildcard_files(flList, szCurFile, &i, from_relative);
             i--;
         }
         else
         {
-            FILE_ENTRY *file = &flList->feFiles[i];
-            add_file_to_entry(file, szCurFile);
-            file->attributes = GetFileAttributesW( file->szFullPath );
-            file->bExists = (file->attributes != INVALID_FILE_ATTRIBUTES);
-            if (!file->bExists) flList->bAnyDontExist = TRUE;
-            if (IsAttribDir(file->attributes)) flList->bAnyDirectories = TRUE;
+            add_file_entry(flList, i, szCurFile, GetFileAttributesW(szCurFile), from_relative, FALSE);
         }

/* advance to the next string */
@@ -1530,7 +1531,7 @@ static int rename_files(LPSHFILEOPSTRUCTW lpFileOp, const FILE_LIST *flFrom, con
     feTo= &flTo->feFiles[0];

/* fail if destination doesn't exist */
-    if (!feFrom->bExists)
+    if (!feFrom->bExists || feFrom->bFromWildcard)
         return ERROR_SHELL_INTERNAL_FILE_NOT_FOUND;

/* fail if destination already exists */

[PATCH v2 5/6] shell32: Don't parse wildcard for rename operation.

[Ziqing Hui]

From: Ziqing Hui

---
 dlls/shell32/shlfileop.c       | 12 ++++++------
 dlls/shell32/tests/shlfileop.c |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/dlls/shell32/shlfileop.c b/dlls/shell32/shlfileop.c
index 24396e7c585..6e155d2cf65 100644
--- a/dlls/shell32/shlfileop.c
+++ b/dlls/shell32/shlfileop.c
@@ -1010,7 +1010,7 @@ static void parse_wildcard_files(FILE_LIST *flList, LPCWSTR szFile, LPDWORD pdwL
 }

/* takes the null-separated file list and fills out the FILE_LIST */
-static HRESULT parse_file_list(FILE_LIST *flList, LPCWSTR szFiles)
+static HRESULT parse_file_list(FILE_LIST *flList, LPCWSTR szFiles, BOOL parse_wildcard)
 {
     LPCWSTR ptr = szFiles;
     WCHAR szCurFile[MAX_PATH];
@@ -1050,14 +1050,14 @@ static HRESULT parse_file_list(FILE_LIST *flList, LPCWSTR szFiles)
         for (p = szCurFile; *p; p++) if (*p == '/') *p = '\';

/* parse wildcard files if they are in the filename */
-        if (from_wildcard)
+        if (from_wildcard && parse_wildcard)
         {
             parse_wildcard_files(flList, szCurFile, &i, from_relative);
             i--;
         }
         else
         {
-            add_file_entry(flList, i, szCurFile, GetFileAttributesW(szCurFile), from_relative, FALSE);
+            add_file_entry(flList, i, szCurFile, GetFileAttributesW(szCurFile), from_relative, from_wildcard);
         }

/* advance to the next string */
@@ -1196,7 +1196,7 @@ static int copy_files(FILE_OPERATION *op, const FILE_LIST *flFrom, FILE_LIST *fl

destroy_file_list(flTo);
         ZeroMemory(flTo, sizeof(FILE_LIST));
-        parse_file_list(flTo, curdir);
+        parse_file_list(flTo, curdir, FALSE);
         fileDest = &flTo->feFiles[0];
     }

@@ -1571,11 +1571,11 @@ int WINAPI SHFileOperationW(LPSHFILEOPSTRUCTW lpFileOp)
     ZeroMemory(&flFrom, sizeof(FILE_LIST));
     ZeroMemory(&flTo, sizeof(FILE_LIST));

-    if ((ret = parse_file_list(&flFrom, lpFileOp->pFrom)))
+    if ((ret = parse_file_list(&flFrom, lpFileOp->pFrom, lpFileOp->wFunc != FO_RENAME)))
         return ret;

if (lpFileOp->wFunc != FO_DELETE)
-        parse_file_list(&flTo, lpFileOp->pTo);
+        parse_file_list(&flTo, lpFileOp->pTo, FALSE);

ZeroMemory(&op, sizeof(op));
     op.req = lpFileOp;
diff --git a/dlls/shell32/tests/shlfileop.c b/dlls/shell32/tests/shlfileop.c
index 30fecb5d52a..2fcb715dfdd 100644
--- a/dlls/shell32/tests/shlfileop.c
+++ b/dlls/shell32/tests/shlfileop.c
@@ -908,7 +908,7 @@ static void test_rename(void)

check_file_operation(FO_RENAME, FOF_NO_UI,
             "test1.txt\0", "test?.txt\0",
-            ERROR_INVALID_NAME, FALSE, TRUE, TRUE);
+            ERROR_INVALID_NAME, FALSE, FALSE, FALSE);
     ok(file_exists("test1.txt"), "Expected test1.txt to exist.\n");
 }

[PATCH v2 6/6] shell32: Rework rename_files.

[Ziqing Hui]

From: Ziqing Hui

---
 dlls/shell32/shlfileop.c       | 52 +++++++++++++++++++++-------------
 dlls/shell32/tests/shlfileop.c | 39 ++++++++++++-------------
 2 files changed, 51 insertions(+), 40 deletions(-)

diff --git a/dlls/shell32/shlfileop.c b/dlls/shell32/shlfileop.c
index 6e155d2cf65..d71eb6b968d 100644
--- a/dlls/shell32/shlfileop.c
+++ b/dlls/shell32/shlfileop.c
@@ -54,8 +54,19 @@ WINE_DEFAULT_DEBUG_CHANNEL(shell);

#define FO_MASK         0xF

-#define DE_SAMEFILE      0x71
-#define DE_DESTSAMETREE  0x7D
+#define DE_SAMEFILE        0x71
+#define DE_MANYSRC1DEST    0x72
+#define DE_DIFFDIR         0x73
+#define DE_ROOTDIR         0x74
+#define DE_OPCANCELLED     0x75
+#define DE_DESTSUBTREE     0x76
+#define DE_ACCESSDENIEDSRC 0x78
+#define DE_PATHTOODEEP     0x79
+#define DE_MANYDEST        0x7A
+#define DE_INVALIDFILES    0x7C
+#define DE_DESTSAMETREE    0x7D
+#define DE_FLDDESTISFILE   0x7E
+#define DE_FILEDESTISFLD   0x80

static DWORD SHNotifyCreateDirectoryA(LPCSTR path, LPSECURITY_ATTRIBUTES sec);
 static DWORD SHNotifyCreateDirectoryW(LPCWSTR path, LPSECURITY_ATTRIBUTES sec);
@@ -1515,30 +1526,33 @@ static int move_files(LPSHFILEOPSTRUCTW lpFileOp, const FILE_LIST *flFrom, const
     return ERROR_SUCCESS;
 }

-/* the FO_RENAME files */
-static int rename_files(LPSHFILEOPSTRUCTW lpFileOp, const FILE_LIST *flFrom, const FILE_LIST *flTo)
+/* The FO_RENAME operation of SHFileOperation. */
+static DWORD rename_files(SHFILEOPSTRUCTW *op, const FILE_LIST *from, const FILE_LIST *to)
 {
-    const FILE_ENTRY *feFrom;
-    const FILE_ENTRY *feTo;
+    const FILE_ENTRY *entry_from, *entry_to;

-    if (flFrom->dwNumFiles != 1)
-        return ERROR_GEN_FAILURE;
+    if (from->dwNumFiles != 1)
+        return DE_MANYSRC1DEST;

-    if (flTo->dwNumFiles != 1)
-        return ERROR_CANCELLED;
+    entry_from = &from->feFiles[0];
+    if (entry_from->bFromWildcard)
+        return DE_MANYSRC1DEST;
+    if (!entry_from->bExists)
+        return ERROR_FILE_NOT_FOUND;

-    feFrom = &flFrom->feFiles[0];
-    feTo= &flTo->feFiles[0];
+    if (!to->dwNumFiles)
+        return DE_DIFFDIR;
+    entry_to = &to->feFiles[0];
+    if (entry_to->bFromWildcard)
+        return ERROR_INVALID_NAME;

-    /* fail if destination doesn't exist */
-    if (!feFrom->bExists || feFrom->bFromWildcard)
-        return ERROR_SHELL_INTERNAL_FILE_NOT_FOUND;
+    if (wcscmp(entry_from->szDirectory, entry_to->szDirectory) != 0)
+        return DE_DIFFDIR;

-    /* fail if destination already exists */
-    if (feTo->bExists)
-        return ERROR_ALREADY_EXISTS;
+    if (entry_to->bExists && IsAttribDir(entry_from->attributes) != IsAttribDir(entry_to->attributes))
+        return IsAttribDir(entry_to->attributes) ? DE_FILEDESTISFLD : DE_FLDDESTISFILE;

-    return SHNotifyMoveFileW(feFrom->szFullPath, feTo->szFullPath);
+    return SHNotifyMoveFileW(entry_from->szFullPath, entry_to->szFullPath);
 }

/* alert the user if an unsupported flag is used */
diff --git a/dlls/shell32/tests/shlfileop.c b/dlls/shell32/tests/shlfileop.c
index 2fcb715dfdd..d69995d06eb 100644
--- a/dlls/shell32/tests/shlfileop.c
+++ b/dlls/shell32/tests/shlfileop.c
@@ -739,43 +739,42 @@ static void test_rename(void)
     set_curr_dir_path(from, "test1.txt\0");
     set_curr_dir_path(to, "test4.txt\0");
     check_file_operation(FO_RENAME, FOF_NO_UI, from, to,
-            DE_FILEDESTISFLD, FALSE, TRUE, FALSE);
+            DE_FILEDESTISFLD, FALSE, FALSE, FALSE);
     ok(file_exists("test1.txt"), "The file is renamed\n");

/* Rename a dir to an existing file. */
     set_curr_dir_path(from, "test4.txt\0");
     set_curr_dir_path(to, "test1.txt\0");
     check_file_operation(FO_RENAME, FOF_NO_UI, from, to,
-            DE_FLDDESTISFILE, FALSE, TRUE, FALSE);
+            DE_FLDDESTISFILE, FALSE, FALSE, FALSE);
     ok(dir_exists("test4.txt"), "The file is renamed\n");

/* Rename file to a different directory. */
     set_curr_dir_path(from, "test3.txt\0");
     set_curr_dir_path(to, "test4.txt\test1.txt\0");
     check_file_operation(FO_RENAME, FOF_NO_UI, from, to,
-            DE_DIFFDIR, FALSE, TRUE, FALSE);
-    todo_wine
+            DE_DIFFDIR, FALSE, FALSE, FALSE);
     ok(!file_exists("test4.txt\test1.txt"), "The file is renamed\n");

/* Multiple sources and targets, no FOF_MULTIDESTFILES. */
     set_curr_dir_path(from, "test1.txt\0test2.txt\0test4.txt\0");
     set_curr_dir_path(to, "test6.txt\0test7.txt\0test8.txt\0");
     check_file_operation(FO_RENAME, FOF_NO_UI, from, to,
-            DE_MANYSRC1DEST, FALSE, TRUE, FALSE);
+            DE_MANYSRC1DEST, FALSE, FALSE, FALSE);
     ok(file_exists("test1.txt"), "The file is renamed - many files are specified\n");

/* Multiple sources and targets, with FOF_MULTIDESTFILES. */
     set_curr_dir_path(from, "test1.txt\0test2.txt\0test4.txt\0");
     set_curr_dir_path(to, "test6.txt\0test7.txt\0test8.txt\0");
     check_file_operation(FO_RENAME, FOF_NO_UI | FOF_MULTIDESTFILES, from, to,
-            DE_MANYSRC1DEST, FALSE, TRUE, FALSE);
+            DE_MANYSRC1DEST, FALSE, FALSE, FALSE);
     ok(file_exists("test1.txt"), "The file is not renamed - many files are specified\n");

/* Sources outnumber targets, with FOF_MULTIDESTFILES. */
     set_curr_dir_path(from, "test1.txt\0test2.txt\0test4.txt\0");
     set_curr_dir_path(to, "test6.txt\0test7.txt\0");
     check_file_operation(FO_RENAME, FOF_NO_UI | FOF_MULTIDESTFILES, from, to,
-            DE_MANYSRC1DEST, FALSE, TRUE, FALSE);
+            DE_MANYSRC1DEST, FALSE, FALSE, FALSE);
     ok(file_exists("test1.txt"), "The file is not renamed - many files are specified\n");

/* Rename a file. */
@@ -808,7 +807,7 @@ static void test_rename(void)
     /* Rename multiple files to a single file. */
     check_file_operation(FO_RENAME, FOF_NO_UI,
             "test1.txt\0test2.txt\0", "a.txt\0",
-            DE_MANYSRC1DEST, FALSE, TRUE, FALSE);
+            DE_MANYSRC1DEST, FALSE, FALSE, FALSE);
     ok(file_exists("test1.txt"), "Expected test1.txt to exist\n");
     ok(file_exists("test2.txt"), "Expected test2.txt to exist\n");
     ok(!file_exists("a.txt"), "Expected a.txt to not exist\n");
@@ -816,24 +815,22 @@ static void test_rename(void)
     /* Rename a file to multiple files. */
     check_file_operation(FO_RENAME, FOF_NO_UI,
             "test1.txt\0", "a.txt\0b.txt\0",
-            ERROR_SUCCESS, FALSE, TRUE, TRUE);
-    todo_wine
+            ERROR_SUCCESS, FALSE, FALSE, FALSE);
     ok(!file_exists("test1.txt"), "Expected test1.txt to not exist\n");
-    todo_wine
     ok(DeleteFileA("a.txt"), "Expected a.txt to exist\n");
     ok(!DeleteFileA("b.txt"), "Expected b.txt to not exist\n");

/* Rename a nonexistent file. */
     check_file_operation(FO_RENAME, FOF_NO_UI,
             "idontexist\0", "newfile\0",
-            ERROR_FILE_NOT_FOUND, FALSE, TRUE, FALSE);
+            ERROR_FILE_NOT_FOUND, FALSE, FALSE, FALSE);
     ok(!file_exists("newfile"), "Expected newfile to not exist\n");

/* Target already exists. */
     createTestFile("test1.txt");
     check_file_operation(FO_RENAME, FOF_NO_UI,
             "test1.txt\0", "test2.txt\0",
-            ERROR_SUCCESS, FALSE, TRUE, FALSE);
+            ERROR_SUCCESS, FALSE, FALSE, FALSE);
     check_file_operation(FO_RENAME, FOF_NO_UI,
             "testdir2\0", "testdir4\0",
             ERROR_SUCCESS, FALSE, TRUE, FALSE);
@@ -853,15 +850,15 @@ static void test_rename(void)
     ok(!file_exists("nonexistence"), "Expected nonexistence to not exist\n");
     check_file_operation(FO_RENAME, FOF_NO_UI,
             "testdir2\0", "\0",
-            DE_DIFFDIR, FALSE, TRUE, TRUE);
+            DE_DIFFDIR, FALSE, FALSE, FALSE);
     ok(file_exists("testdir2"), "Expected testdir2 to exist\n");
     check_file_operation(FO_RENAME, FOF_NO_UI,
             "test1.txt\0", "\0",
-            DE_DIFFDIR, FALSE, TRUE, TRUE);
+            DE_DIFFDIR, FALSE, FALSE, FALSE);
     ok(file_exists("test1.txt"), "Expected test1.txt to exist\n");
     check_file_operation(FO_RENAME, FOF_NO_UI,
             "nonexistence\0", "\0",
-            ERROR_FILE_NOT_FOUND, FALSE, TRUE, TRUE);
+            ERROR_FILE_NOT_FOUND, FALSE, FALSE, FALSE);
     check_file_operation(FO_RENAME, FOF_NO_UI,
             "\0", "\0",
             DE_MANYSRC1DEST, FALSE, TRUE, TRUE);
@@ -880,13 +877,13 @@ static void test_rename(void)
             ERROR_INVALID_PARAMETER, 0xdeadbeef, FALSE, FALSE);
     check_file_operation(FO_RENAME, FOF_NO_UI,
             "test1.txt\0", NULL,
-            ERROR_ACCESS_DENIED, FALSE, TRUE, TRUE);
+            ERROR_ACCESS_DENIED, FALSE, TRUE, FALSE);
     check_file_operation(FO_RENAME, FOF_NO_UI,
             "testdir2\0", NULL,
-            ERROR_ACCESS_DENIED, FALSE, TRUE, TRUE);
+            ERROR_ACCESS_DENIED, FALSE, TRUE, FALSE);
     check_file_operation(FO_RENAME, FOF_NO_UI,
             "nonexistence\0", NULL,
-            ERROR_ACCESS_DENIED, FALSE, TRUE, TRUE);
+            ERROR_ACCESS_DENIED, FALSE, TRUE, FALSE);
     check_file_operation(FO_RENAME, FOF_NO_UI,
             NULL, NULL,
             ERROR_INVALID_PARAMETER, 0xdeadbeef, FALSE, FALSE);
@@ -896,13 +893,13 @@ static void test_rename(void)
     init_shfo_tests();
     check_file_operation(FO_RENAME, FOF_NO_UI,
             "test?.txt\0", "test_target.txt\0",
-            DE_MANYSRC1DEST, FALSE, TRUE, FALSE);
+            DE_MANYSRC1DEST, FALSE, FALSE, FALSE);
     ok(file_exists("test1.txt"), "Expected test1.txt to exist.\n");
     ok(!DeleteFileA("test_target.txt"), "Expected test_target.txt to not exist.\n");

check_file_operation(FO_RENAME, FOF_NO_UI,
             "t?st1.txt\0", "test_target.txt\0",
-            DE_MANYSRC1DEST, FALSE, TRUE, FALSE);
+            DE_MANYSRC1DEST, FALSE, FALSE, FALSE);
     ok(file_exists("test1.txt"), "Expected test1.txt to exist.\n");
     ok(!DeleteFileA("test_target.txt"), "Expected test_target.txt to not exist.\n");