[PATCH 1/4] msvcp110: Add tr2_sys__File_size implementation and test.(try 3)

YongHao Hu christopherwuy at gmail.com
Thu Jun 11 04:31:09 CDT 2015


Hi,


On 2015年06月11日 16:21, Piotr Caban wrote:
> The _Equivalent tests shows that applications probably shouldn't 
> depend on GetLastError value. The function sometimes sets error even 
> if it succeeds, sometimes the error depends on arguments order. In 
> general while using msvcp/msvcrt functions applications are supposed 
> to check errno value that is not set in this case.
>
Do you mean that  _Equivalent sets ERROR_SUCCESS when it succeeds?
> The tests shows that Microsoft is probably using different functions 
> internally. E.g. this code looks suspicious:
> +ULONGLONG __cdecl tr2_sys__File_size(const char* path)
> +{
> +    WIN32_FILE_ATTRIBUTE_DATA fad;
> +
> +    TRACE("(%p)\n", path);
> +    if(!path) {
> +        SetLastError(ERROR_INVALID_PARAMETER);
> +        return 0;
> +    }
> I don't think msvcp should set the error explicitly. It's why I think 
> it's better to just skip GetLastError tests and revisit it if we find 
> any application that depends on GetLastError value. I don't think we 
> will find any.
>
I had made two implementation before, another one is like this:
     HANDLE h1;
     h1 = CreateFileA(path, 0, FILE_SHARE_DELETE | FILE_SHARE_READ | 
FILE_SHARE_WRITE,
             NULL, OPEN_EXISTING, 0, 0);
     if(h1 == INVALID_HANDLE_VALUE)
         return 0;
     DWORD size;
     LPDWORD high;
     size = GetFileSize( h1, &high );
     TRACE("(%p)\n", path);
     return ((ULONGLONG)(high) << 32) + size;

(As I had showed it here, could you please also tell me which 
implementation is better and why?)

This one also needs to set the error explicitly, which I didn't do it.
Do you think that we should ignore GetLastError in these series of patches?
Ps: My tests[1] show that _Copy_file and _Rename return GetLastError, 
but they doesn't matter at all.

Thank you.

[1]: 
https://github.com/YongHaoWu/wine-hub/commit/8a5af834d63914e7eaed75553399001458964e74#diff-bb7627a0293c2f936059d8808567b2f3R14344
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20150611/251c6a20/attachment.html>


More information about the wine-devel mailing list