[PATCH 1/2 v4] kernel32: Support MOVEFILE_WRITE_THROUGH / CopyFileExW progress
Signed-off-by: Tom Watson <coder(a)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
On 04/03/18 23:04, Tom Watson wrote:
Signed-off-by: Tom Watson <coder(a)tommywatson.com <mailto:coder(a)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
participants (2)
-
Tom Watson -
Zebediah Figura