[PATCH 1/2 v4] kernel32: Support MOVEFILE_WRITE_THROUGH / CopyFileExW progress
Zebediah Figura
z.figura12 at gmail.com
Thu Mar 8 22:13:38 CST 2018
On 04/03/18 23:04, Tom Watson wrote:
> Signed-off-by: Tom Watson <coder at tommywatson.com
> <mailto:coder at tommywatson.com>>
> ---
> v4
> Added "optional" DWORD return value for WriteFile() to stop segfaults
> General tidy up and temporarily removed failing tests that require
> updated kernel32.dll
>
>
>
>
>
Hello Tom, thanks for contributing. I defer to others' reviews, but in
lieu thereof, I have a few comments to add:
First of all, when sending patches, can you please send them inline,
preferably using `git send-email`? This makes review easier.
Secondly, it seems you still have some C++-style comments left:
+ switch (progressExit) {
+ case PROGRESS_CONTINUE:
+ // keep at it
+ break;
+ case PROGRESS_QUIET:
+ // requested not to be called again
+ progress = NULL;
+ break;
+ case PROGRESS_CANCEL:
+ // cancel and clean up
+ CloseHandle( h2 );
+ h2 = INVALID_HANDLE_VALUE;
+ DeleteFileW( dest );
+ SetLastError( ERROR_REQUEST_ABORTED );
+ goto done;
+ case PROGRESS_STOP:
+ // do nothing, allow for a restart (??)
+ SetLastError( ERROR_REQUEST_ABORTED );
+ goto done;
With regard to the content of the patch itself, it would probably be
best to split this up into separate patches: first implement the
progress callback, and then MOVEFILE_WRITE_THROUGH.
It also doesn't seem clear to me that your implementation of
MOVEFILE_WRITE_THROUGH is correct. In particular: firstly, the flag
seems to guarantee that a copy-and-delete is completed *if* it is
performed, but not that setting the flag forces MoveFileWithProgress()
to perform a copy-and-delete.
Secondly, an implementation using the CopyFileEx() callback seems
excessively complicated; why not simply call FlushFileBuffers() directly?
ἔρρωσο,
Zeb
More information about the wine-devel
mailing list