[PATCH v9 0/1] MR5020: kernelbase/file: implement CopyFile2
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
From: Kartavya Vashishtha <sendtokartavya(a)gmail.com> --- 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, ¶ms ); +} + /************************************************************************** * 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 -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5020
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. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5020#note_60956
@zfigura I'm really sorry that this patch required so many reviews. I'll definitely pay more attention to formatting next time! -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5020#note_61105
@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. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5020#note_61524
participants (3)
-
Kartavya Vashishtha -
Kartavya Vashishtha (＠kvashis)
-
Zebediah Figura (＠zfigura)