[PATCH 0/1] MR10680: scrrun: Implement IFile and IFolder Delete and Move methods.

[Francis De Brabandere (＠francisdb)]

file_Delete wraps DeleteFileW, honouring the Force flag by clearing
readonly attributes. folder_Delete recursively removes contents via
the existing delete_file/delete_folder helpers, then RemoveDirectoryW.
Both Move methods wrap MoveFileW.

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

[PATCH 1/1] scrrun: Implement IFile and IFolder Delete and Move methods.

[Francis De Brabandere]

From: Francis De Brabandere <[e-mail]>

file_Delete wraps DeleteFileW, honouring the Force flag by clearing
readonly attributes. folder_Delete recursively removes contents via
the existing delete_file/delete_folder helpers, then RemoveDirectoryW.
Both Move methods wrap MoveFileW.
---
 dlls/scrrun/filesystem.c       |  66 ++++++++--
 dlls/scrrun/tests/filesystem.c | 215 +++++++++++++++++++++++++++++++++
 2 files changed, 273 insertions(+), 8 deletions(-)

diff --git a/dlls/scrrun/filesystem.c b/dlls/scrrun/filesystem.c
index af76f0cf4d8..8ff78a198c2 100644
--- a/dlls/scrrun/filesystem.c
+++ b/dlls/scrrun/filesystem.c
@@ -197,6 +197,8 @@ static HRESULT create_folder(const WCHAR*, IFolder**);
 static HRESULT create_file(BSTR, IFile**);
 static HRESULT create_foldercoll_enum(struct foldercollection*, IUnknown**);
 static HRESULT create_filecoll_enum(struct filecollection*, IUnknown**);
+static inline HRESULT delete_file(const WCHAR*, DWORD, VARIANT_BOOL);
+static HRESULT delete_folder(const WCHAR*, DWORD, VARIANT_BOOL);
 static HRESULT create_drivecoll_enum(struct drivecollection*, IUnknown**);
 static inline DWORD get_parent_folder_name(const WCHAR *path, DWORD len);
 static HRESULT get_date_from_filetime(const FILETIME *ft, DATE *date);
@@ -2667,8 +2669,37 @@ static HRESULT WINAPI folder_get_Type(IFolder *iface, BSTR *type)
 static HRESULT WINAPI folder_Delete(IFolder *iface, VARIANT_BOOL force)
 {
     struct folder *This = impl_from_IFolder(iface);
-    FIXME("(%p)->(%x): stub\n", This, force);
-    return E_NOTIMPL;
+    WCHAR path[MAX_PATH];
+    DWORD len;
+    HRESULT hr;
+
+    TRACE("(%p)->(%x)\n", This, force);
+
+    len = SysStringLen(This->path);
+    if (len + 3 >= MAX_PATH)
+        return E_FAIL;
+
+    memcpy(path, This->path, len * sizeof(WCHAR));
+    path[len] = '\\';
+    path[len + 1] = '*';
+    path[len + 2] = 0;
+
+    hr = delete_file(path, len + 2, force);
+    if (FAILED(hr))
+        return hr;
+
+    hr = delete_folder(path, len + 2, force);
+    if (FAILED(hr))
+        return hr;
+
+    if (!RemoveDirectoryW(This->path))
+    {
+        if (!force || !SetFileAttributesW(This->path, FILE_ATTRIBUTE_NORMAL)
+                || !RemoveDirectoryW(This->path))
+            return create_error(GetLastError());
+    }
+
+    return S_OK;
 }
 
 static HRESULT WINAPI folder_Copy(IFolder *iface, BSTR dest, VARIANT_BOOL overwrite)
@@ -2681,8 +2712,13 @@ static HRESULT WINAPI folder_Copy(IFolder *iface, BSTR dest, VARIANT_BOOL overwr
 static HRESULT WINAPI folder_Move(IFolder *iface, BSTR dest)
 {
     struct folder *This = impl_from_IFolder(iface);
-    FIXME("(%p)->(%s): stub\n", This, debugstr_w(dest));
-    return E_NOTIMPL;
+
+    TRACE("(%p)->(%s)\n", This, debugstr_w(dest));
+
+    if (!MoveFileW(This->path, dest))
+        return create_error(GetLastError());
+
+    return S_OK;
 }
 
 static HRESULT WINAPI folder_get_IsRootFolder(IFolder *iface, VARIANT_BOOL *isroot)
@@ -3143,8 +3179,17 @@ static HRESULT WINAPI file_get_Type(IFile *iface, BSTR *pbstrType)
 static HRESULT WINAPI file_Delete(IFile *iface, VARIANT_BOOL Force)
 {
     struct file *This = impl_from_IFile(iface);
-    FIXME("(%p)->(%x)\n", This, Force);
-    return E_NOTIMPL;
+
+    TRACE("(%p)->(%x)\n", This, Force);
+
+    if (!DeleteFileW(This->path))
+    {
+        if (!Force || !SetFileAttributesW(This->path, FILE_ATTRIBUTE_NORMAL)
+                || !DeleteFileW(This->path))
+            return create_error(GetLastError());
+    }
+
+    return S_OK;
 }
 
 static HRESULT WINAPI file_Copy(IFile *iface, BSTR Destination, VARIANT_BOOL OverWriteFiles)
@@ -3157,8 +3202,13 @@ static HRESULT WINAPI file_Copy(IFile *iface, BSTR Destination, VARIANT_BOOL Ove
 static HRESULT WINAPI file_Move(IFile *iface, BSTR Destination)
 {
     struct file *This = impl_from_IFile(iface);
-    FIXME("(%p)->(%s)\n", This, debugstr_w(Destination));
-    return E_NOTIMPL;
+
+    TRACE("(%p)->(%s)\n", This, debugstr_w(Destination));
+
+    if (!MoveFileW(This->path, Destination))
+        return create_error(GetLastError());
+
+    return S_OK;
 }
 
 static HRESULT WINAPI file_OpenAsTextStream(IFile *iface, IOMode mode, Tristate format, ITextStream **stream)
diff --git a/dlls/scrrun/tests/filesystem.c b/dlls/scrrun/tests/filesystem.c
index 9b5abdf5c97..68c34d07584 100644
--- a/dlls/scrrun/tests/filesystem.c
+++ b/dlls/scrrun/tests/filesystem.c
@@ -3135,6 +3135,217 @@ static void test_DoOpenPipeStream(void)
         ITextStream_Release(stream_write);
 }
 
+static void test_File_Delete(void)
+{
+    WCHAR pathW[MAX_PATH];
+    IFile *file;
+    BSTR path;
+    HRESULT hr;
+    HANDLE hf;
+    DWORD attrs;
+
+    /* Create a normal file and delete it. */
+    get_temp_path(NULL, pathW);
+    hf = CreateFileW(pathW, GENERIC_WRITE, 0, NULL, CREATE_NEW, FILE_ATTRIBUTE_NORMAL, NULL);
+    if (hf == INVALID_HANDLE_VALUE)
+    {
+        skip("Can't create temporary file\n");
+        return;
+    }
+    CloseHandle(hf);
+
+    path = SysAllocString(pathW);
+    hr = IFileSystem3_GetFile(fs3, path, &file);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    hr = IFile_Delete(file, VARIANT_FALSE);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    attrs = GetFileAttributesW(pathW);
+    ok(attrs == INVALID_FILE_ATTRIBUTES, "expected file to be deleted\n");
+
+    IFile_Release(file);
+    SysFreeString(path);
+
+    /* Create a readonly file and try deleting without force. */
+    hf = CreateFileW(pathW, GENERIC_WRITE, 0, NULL, CREATE_NEW, FILE_ATTRIBUTE_READONLY, NULL);
+    if (hf == INVALID_HANDLE_VALUE)
+    {
+        skip("Can't create temporary readonly file\n");
+        return;
+    }
+    CloseHandle(hf);
+
+    path = SysAllocString(pathW);
+    hr = IFileSystem3_GetFile(fs3, path, &file);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    hr = IFile_Delete(file, VARIANT_FALSE);
+    ok(hr == CTL_E_PERMISSIONDENIED, "Unexpected hr %#lx.\n", hr);
+
+    attrs = GetFileAttributesW(pathW);
+    ok(attrs != INVALID_FILE_ATTRIBUTES, "file should still exist\n");
+
+    /* Now delete with force. */
+    hr = IFile_Delete(file, VARIANT_TRUE);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    attrs = GetFileAttributesW(pathW);
+    ok(attrs == INVALID_FILE_ATTRIBUTES, "expected file to be deleted\n");
+
+    IFile_Release(file);
+    SysFreeString(path);
+}
+
+static void test_File_Move(void)
+{
+    WCHAR src_path[MAX_PATH], dst_path[MAX_PATH];
+    IFile *file;
+    BSTR path, dst;
+    HRESULT hr;
+    HANDLE hf;
+    DWORD attrs;
+
+    get_temp_path(NULL, src_path);
+    hf = CreateFileW(src_path, GENERIC_WRITE, 0, NULL, CREATE_NEW, FILE_ATTRIBUTE_NORMAL, NULL);
+    if (hf == INVALID_HANDLE_VALUE)
+    {
+        skip("Can't create temporary file\n");
+        return;
+    }
+    CloseHandle(hf);
+
+    get_temp_path(NULL, dst_path);
+    /* get_temp_path creates and deletes, so dst_path is free. */
+
+    path = SysAllocString(src_path);
+    hr = IFileSystem3_GetFile(fs3, path, &file);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    dst = SysAllocString(dst_path);
+    hr = IFile_Move(file, dst);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    attrs = GetFileAttributesW(src_path);
+    ok(attrs == INVALID_FILE_ATTRIBUTES, "source file should be gone\n");
+
+    attrs = GetFileAttributesW(dst_path);
+    ok(attrs != INVALID_FILE_ATTRIBUTES, "destination file should exist\n");
+
+    IFile_Release(file);
+    SysFreeString(path);
+    SysFreeString(dst);
+    DeleteFileW(dst_path);
+}
+
+static void test_Folder_Delete(void)
+{
+    WCHAR temp_path[MAX_PATH], dir_path[MAX_PATH], file_path[MAX_PATH];
+    IFolder *folder;
+    BSTR path;
+    HRESULT hr;
+    HANDLE hf;
+    DWORD attrs;
+
+    GetTempPathW(MAX_PATH, temp_path);
+    lstrcpyW(dir_path, temp_path);
+    lstrcatW(dir_path, L"scrrun_del_test");
+
+    if (!CreateDirectoryW(dir_path, NULL))
+    {
+        skip("Can't create temporary directory\n");
+        return;
+    }
+
+    /* Create a file inside the folder. */
+    lstrcpyW(file_path, dir_path);
+    lstrcatW(file_path, L"\\testfile.txt");
+    hf = CreateFileW(file_path, GENERIC_WRITE, 0, NULL, CREATE_NEW, FILE_ATTRIBUTE_NORMAL, NULL);
+    ok(hf != INVALID_HANDLE_VALUE, "CreateFile failed, error %ld\n", GetLastError());
+    CloseHandle(hf);
+
+    path = SysAllocString(dir_path);
+    hr = IFileSystem3_GetFolder(fs3, path, &folder);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    hr = IFolder_Delete(folder, VARIANT_FALSE);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    attrs = GetFileAttributesW(dir_path);
+    ok(attrs == INVALID_FILE_ATTRIBUTES, "expected folder to be deleted\n");
+
+    IFolder_Release(folder);
+    SysFreeString(path);
+
+    /* Test deleting folder containing a readonly file with force. */
+    ok(CreateDirectoryW(dir_path, NULL), "CreateDirectory failed, error %ld\n", GetLastError());
+    lstrcpyW(file_path, dir_path);
+    lstrcatW(file_path, L"\\readonly.txt");
+    hf = CreateFileW(file_path, GENERIC_WRITE, 0, NULL, CREATE_NEW, FILE_ATTRIBUTE_READONLY, NULL);
+    ok(hf != INVALID_HANDLE_VALUE, "CreateFile failed, error %ld\n", GetLastError());
+    CloseHandle(hf);
+
+    path = SysAllocString(dir_path);
+    hr = IFileSystem3_GetFolder(fs3, path, &folder);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    hr = IFolder_Delete(folder, VARIANT_FALSE);
+    ok(hr == CTL_E_PERMISSIONDENIED, "Unexpected hr %#lx.\n", hr);
+
+    attrs = GetFileAttributesW(dir_path);
+    ok(attrs != INVALID_FILE_ATTRIBUTES, "folder should still exist\n");
+
+    hr = IFolder_Delete(folder, VARIANT_TRUE);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    attrs = GetFileAttributesW(dir_path);
+    ok(attrs == INVALID_FILE_ATTRIBUTES, "expected folder to be deleted\n");
+
+    IFolder_Release(folder);
+    SysFreeString(path);
+}
+
+static void test_Folder_Move(void)
+{
+    WCHAR temp_path[MAX_PATH], src_path[MAX_PATH], dst_path[MAX_PATH];
+    IFolder *folder;
+    BSTR path, dst;
+    HRESULT hr;
+    DWORD attrs;
+
+    GetTempPathW(MAX_PATH, temp_path);
+    lstrcpyW(src_path, temp_path);
+    lstrcatW(src_path, L"scrrun_move_src");
+    lstrcpyW(dst_path, temp_path);
+    lstrcatW(dst_path, L"scrrun_move_dst");
+
+    if (!CreateDirectoryW(src_path, NULL))
+    {
+        skip("Can't create temporary directory\n");
+        return;
+    }
+
+    path = SysAllocString(src_path);
+    hr = IFileSystem3_GetFolder(fs3, path, &folder);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    dst = SysAllocString(dst_path);
+    hr = IFolder_Move(folder, dst);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    attrs = GetFileAttributesW(src_path);
+    ok(attrs == INVALID_FILE_ATTRIBUTES, "source folder should be gone\n");
+
+    attrs = GetFileAttributesW(dst_path);
+    ok(attrs != INVALID_FILE_ATTRIBUTES, "destination folder should exist\n");
+    ok(attrs & FILE_ATTRIBUTE_DIRECTORY, "expected directory attribute\n");
+
+    IFolder_Release(folder);
+    SysFreeString(path);
+    SysFreeString(dst);
+    RemoveDirectoryW(dst_path);
+}
+
 START_TEST(filesystem)
 {
     HRESULT hr;
@@ -3185,6 +3396,10 @@ START_TEST(filesystem)
     test_MoveFile();
     test_MoveFolder();
     test_DoOpenPipeStream();
+    test_File_Delete();
+    test_File_Move();
+    test_Folder_Delete();
+    test_Folder_Move();
 
     IFileSystem3_Release(fs3);
 
-- 
GitLab

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

[Francis De Brabandere (＠francisdb)]

Tests that do a recursive delete are always a risk. Any safeguards needed?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10680#note_136448

[Nikolay Sivov (＠nsivov)]

Please split this per-method.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10680#note_136562