[2/3] kernel32/tests: test CopyFileEx callback and cancellation (resend)

Juan Lang juan.lang at gmail.com
Mon Sep 30 10:09:34 CDT 2013


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

> 2013/9/30 Nikolay Sivov <bunglehead at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20130930/5ca05e11/attachment-0001.html>


More information about the wine-devel mailing list