[PATCH 1/6]: msvcp110: Add tr2_sys__Copy_file implementation and test.

Piotr Caban piotr.caban at gmail.com
Tue Jun 30 07:25:48 CDT 2015


On 06/29/15 19:14, YongHaoHu wrote:
>
>
> On 15/6/29 下午10:51, Piotr Caban wrote:
>> On 06/29/15 16:40, YongHaoHu wrote:
>>> On 15/6/29 下午5:29, Piotr Caban wrote:
>>>> On 06/27/15 19:49, YongHaoHu wrote:
>>>>> +        ret = p_tr2_sys__Copy_file(tests[i].source, tests[i].dest,
>>>>> tests[i].fail_if_exists);
>>>>> +        ok(ret == GetLastError(), "test_tr2_sys__Copy_file(): test
>>>>> %d expect: %d, got %d\n", i+1, GetLastError(), ret);
>>>>> +        ok(errno == 0xdeadbeef, "test_tr2_sys__Copy_file(): test %d
>>>>> errno expect 0xdeadbeef, got %d\n", i+1, errno);
>>>> Please check the return value of the function explicitly. Currently
>>>> your implementation returns error while it should succeed and it's not
>>>> visible in the tests.
>>> Because the last error handling is not correct in CopyFile[1], I check
>>> GetLastError() instead of values. Do you have any good idea?
>> If testing exact return value of function is hard please at least test
>> if function succeeds/fails when it should.
> Sorry, could you explain a bit more details?
Please just check if function returns 0 (ERROR_SUCCESS) or something 
else. This way you don't need to check exact error value (only check if 
it's 0 or something else).

>> Currently your implementation sometimes returns error even so it
>> succeeds.
>>
> Which one do you mean ? As you know, last_error hadling is not correct
> in wine's CopyFile, was this happened in Wine platform?
{ "f1", "tr2_test_dir\\f1_copy", TRUE }
Your implementation returns error if f1_copy file already exists. It 
only makes sense to use GetLastError if function have failed.

Thanks,
Piotr



More information about the wine-devel mailing list