[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