On Sun, Sep 29, 2013 at 11:10 PM, Daniel Jeli��ski <djelinski1@gmail.com> wrote:

2013/9/30 Nikolay Sivov <bunglehead@gmail.com>

On 9/30/2013 00:51, Daniel Jeli��ski wrote:

+struct progress_list {
+ �� const DWORD progress_retval_init; ��/* value to return from progress routine */
+ �� const BOOL cancel_init; �� /* value to set Cancel flag to */
+ �� const DWORD progress_retval_end; �� /* value to return from progress routine */
+ �� const BOOL cancel_end; �� /* value to set Cancel flag to */
+ �� const DWORD progress_count; �� /* number of times progress is invoked */
+ �� const BOOL copy_retval; �� /* expected CopyFileEx result */
+ �� const DWORD lastError; �� /* expected CopyFileEx error code */
+} ;

��I don't see a point making them 'const'.
I'm matching the formatting of existing code:
http://source.winehq.org/source/dlls/kernel32/tests/file.c#L65

Also, what's the point of not making them const?

It's a little strange, stylewise. More consistent with C++ style would be to make the entire struct constant. But in a test, we often eschew with such things if they distract, and here I think they do.

+static DWORD WINAPI progress(LARGE_INTEGER TotalFileSize,
+ �� LARGE_INTEGER TotalBytesTransferred,
+ �� LARGE_INTEGER StreamSize,
+ �� LARGE_INTEGER StreamBytesTransferred,
+ �� DWORD dwStreamNumber,
+ �� DWORD dwCallbackReason,
+ �� HANDLE hSourceFile,
+ �� HANDLE hDestinationFile,
+ �� LPVOID lpData)
+{
+ �� progressInvoked++;

Please pass all globals as context data with lpData, and please use 'void*' instead of LPVOID.

Good point about lpData. Still, does that make the patch invalid? Why didn't you mention that on the first review?
About LPVOID - I'm matching the headers:
http://source.winehq.org/source/include/winbase.h#L910
for the third patch:
http://source.winehq.org/source/include/winbase.h#L1018

LPVOID is just an uglier #define of void*. It's easier to read as void*, so please use that instead.

--Juan