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