On Sun, Sep 29, 2013 at 11:10 PM, Daniel Jeliński djelinski1@gmail.comwrote:
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