[PATCH 2/2 v4] kernel32: Support MOVEFILE_WRITE_THROUGH / CopyFileExW progress

Zebediah Figura z.figura12 at gmail.com
Thu Mar 8 22:35:47 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
> 
> 
> 
> 

A few comments here:

+    /*trace("Chunk %d
%d\n",test_MFWPChunks,TotalBytesTransferred.QuadPart);*/
...
+    trace("Running MoveFileWithProgressW() tests...\n");
...
+        trace("Move file %s -> %s\n",source,dest);

These seem superfluous, especially when commented.

+    static const char prefix[] = {'p','f','x',0};

There's no reason not to write this inline; it's only necessary to
"spell out" WCHAR strings.

+    ok(ret != 0, "GetTempPathA error %d\n", GetLastError());
+    ok(ret < MAX_PATH, "temp path should fit into MAX_PATH\n");

I don't think you need to test these here.

+    ret = GetTempFileNameA(temp_path, prefix, 0, tmpl);
+    ok(ret != 0, "GetTempFileNameW error %d\n", GetLastError());
+    MultiByteToWideChar(CP_ACP,0,tmpl,-1,wdst,MAX_PATH);
+    ok(TRUE == DeleteFileW(wdst), "Failed to remove file %s\n",dest);

Why not just use DeleteFileA()?

+        ok(TRUE ==
MoveFileWithProgressW(wsrc,wdst,test_MoveFileWithProgressWCB,
+                    0,MOVEFILE_WRITE_THROUGH),
+                "Failed to move file %s to %s\n",source,dest);

This test doesn't actually guarantee that the callback was ever called.
I would recommend using one or more static variables to check this.

The MOVEFILE_WRITE_THROUGH flag also seems superfluous. I'm not sure
it's possible to reliably test that flag, although I could be wrong.

+/*
+ * Code temporarily disabled to pass the automated tests
+ * as the code to hanlde PROGRESS_STOP/PROGRESS_CANCEL needs
+ * to be installed on the vms
+ */

MoveFileWithProgress() is supported since Windows XP; I would be
surprised if those flags are not implemented on native Windows.

Rather, I suspect that your tests make assumptions about native
implementation of CopyFile based on Wine's implementation. It's not
necessarily true that Windows copies files in blocks of 65536 bytes. Did
you check, when testing, that your callback ever actually ran?

ἔρρωσο,
Zeb



More information about the wine-devel mailing list