[PATCH v9 0/1] MR5020: kernelbase/file: implement CopyFile2

[Kartavya Vashishtha (＠kvashis)]

copy implementation of CopyFileExW into CopyFile2,
then implement CopyFileExW in terms of CopyFile2

should fix https://bugs.winehq.org/show_bug.cgi?id=55897, as Python does not use any new functionality nor the callback parameter from CopyFile2.

I have not written extensive documentation since the original function didn't seem to have any; I can do that if recommended.

I originally intended to implement the callback functionality in this MR as well, but realized that would be better as a follow-up.

--
  v9: implement CopyFile2

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

[PATCH v9 1/1] implement CopyFile2 and reimplement CopyFileEx in terms of CopyFile2

[Kartavya Vashishtha]

From: Kartavya Vashishtha

---
 dlls/kernel32/kernel32.spec     |  1 +
 dlls/kernelbase/file.c          | 50 +++++++++++++++++++++++++++++----
 dlls/kernelbase/kernelbase.spec |  2 +-
 3 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/dlls/kernel32/kernel32.spec b/dlls/kernel32/kernel32.spec
index 9d5dc076acb..28a3f554374 100644
--- a/dlls/kernel32/kernel32.spec
+++ b/dlls/kernel32/kernel32.spec
@@ -257,6 +257,7 @@
 @ stdcall -import ConvertThreadToFiberEx(ptr long)
 @ stdcall ConvertToGlobalHandle(long)
 @ stdcall -import CopyContext(ptr long ptr)
+@ stdcall -import CopyFile2(wstr wstr ptr)
 @ stdcall CopyFileA(str str long)
 @ stdcall CopyFileExA (str str ptr ptr ptr long)
 @ stdcall -import CopyFileExW(wstr wstr ptr ptr ptr long)
diff --git a/dlls/kernelbase/file.c b/dlls/kernelbase/file.c
index e1ba92a6448..602374f0815 100644
--- a/dlls/kernelbase/file.c
+++ b/dlls/kernelbase/file.c
@@ -487,13 +487,15 @@ BOOL WINAPI DECLSPEC_HOTPATCH AreFileApisANSI(void)
     return !oem_file_apis;
 }

-
-/***********************************************************************
- *	CopyFileExW   (kernelbase.@)
+/******************************************************************************
+ *  copy_file
  */
-BOOL WINAPI CopyFileExW( const WCHAR *source, const WCHAR *dest, LPPROGRESS_ROUTINE progress,
-                         void *param, BOOL *cancel_ptr, DWORD flags )
+static BOOL copy_file( const WCHAR *source, const WCHAR *dest, COPYFILE2_EXTENDED_PARAMETERS *params )
 {
+    DWORD flags = params ? params->dwCopyFlags : 0;
+    BOOL *cancel_ptr = params ? params->pfCancel : NULL;
+    PCOPYFILE2_PROGRESS_ROUTINE progress = params ? params->pProgressRoutine : NULL;
+
     static const int buffer_size = 65536;
     HANDLE h1, h2;
     FILE_BASIC_INFORMATION info;
@@ -502,6 +504,11 @@ BOOL WINAPI CopyFileExW( const WCHAR *source, const WCHAR *dest, LPPROGRESS_ROUT
     BOOL ret = FALSE;
     char *buffer;

+    if (cancel_ptr)
+        FIXME("pfCancel is not supported\n");
+    if (progress)
+        FIXME("PCOPYFILE2_PROGRESS_ROUTINE is not supported\n");
+
     if (!source || !dest)
     {
         SetLastError( ERROR_INVALID_PARAMETER );
@@ -577,7 +584,7 @@ BOOL WINAPI CopyFileExW( const WCHAR *source, const WCHAR *dest, LPPROGRESS_ROUT
             count -= res;
         }
     }
-    ret =  TRUE;
+    ret = TRUE;
 done:
     /* Maintain the timestamp of source file to destination file */
     info.FileAttributes = 0;
@@ -589,6 +596,37 @@ done:
     return ret;
 }

+/***********************************************************************
+ *	CopyFile2   (kernelbase.@)
+ */
+HRESULT WINAPI CopyFile2( const WCHAR *source, const WCHAR *dest, COPYFILE2_EXTENDED_PARAMETERS *params )
+{
+    return copy_file(source, dest, params) ? S_OK : HRESULT_FROM_WIN32(GetLastError());
+}
+
+
+/***********************************************************************
+ *	CopyFileExW   (kernelbase.@)
+ */
+BOOL WINAPI CopyFileExW( const WCHAR *source, const WCHAR *dest, LPPROGRESS_ROUTINE progress,
+                         void *param, BOOL *cancel_ptr, DWORD flags )
+{
+    COPYFILE2_EXTENDED_PARAMETERS params;
+
+    if (progress)
+        FIXME("LPPROGRESS_ROUTINE is not supported\n");
+    if (cancel_ptr)
+        FIXME("cancel_ptr is not supported\n");
+
+    params.dwSize = sizeof(params);
+    params.dwCopyFlags = flags;
+    params.pProgressRoutine = NULL;
+    params.pvCallbackContext = NULL;
+    params.pfCancel = NULL;
+
+    return copy_file( source, dest, &params );
+}
+

/**************************************************************************
  *	CopyFileW   (kernelbase.@)
diff --git a/dlls/kernelbase/kernelbase.spec b/dlls/kernelbase/kernelbase.spec
index d3bd59a78e3..a133380a27a 100644
--- a/dlls/kernelbase/kernelbase.spec
+++ b/dlls/kernelbase/kernelbase.spec
@@ -168,7 +168,7 @@
 @ stdcall ConvertThreadToFiberEx(ptr long)
 @ stdcall ConvertToAutoInheritPrivateObjectSecurity(ptr ptr ptr ptr long ptr)
 @ stdcall CopyContext(ptr long ptr)
-# @ stub CopyFile2
+@ stdcall CopyFile2(wstr wstr ptr)
 @ stdcall CopyFileExW(wstr wstr ptr ptr ptr long)
 @ stdcall CopyFileW(wstr wstr long)
 @ stdcall -arch=x86_64 CopyMemoryNonTemporal(ptr ptr long) ntdll.RtlCopyMemoryNonTemporal

[Kartavya Vashishtha (＠kvashis)]

> we still need to not clobber GetLastError.

re-reading the [tests for CopyFile2](dlls/kernel32/tests/file.c#L1108-1110), apparently the tests expect CopyFile2 to return a HRESULT _and_`SetLastError` (example in linked code)

therefore we apparently _are_ supposed to clobber `GetLastError`. MSDN documentation doesn't mention anything either way.

[Kartavya Vashishtha (＠kvashis)]

@zfigura I'm really sorry that this patch required so many reviews. I'll definitely pay more attention to formatting next time!

[Zebediah Figura (＠zfigura)]

> @zfigura I'm really sorry that this patch required so many reviews. I'll definitely pay more attention to formatting next time!

No need to apologize, thank you for your patience working with the review process.

One more comment which I had missed unfortunately, the commit message needs fixed formatting, like:

"kernelbase: Implement CopyFile2()."

I don't think you need the part about reimplementing CopyFileEx().

Otherwise the patch looks good to me.