Junio C Hamano <gitster@xxxxxxxxx> writes:
> Johannes Sixt <j6t@xxxxxxxx> writes:
>
>> Just like on POSIX the value of errno is indeterminate after a
>> successful system call, the value of GetLastError() indeterminate after
>> a successful Windows API call. Therefore, the err_win_to_posix() would
>> not be able to point at a bogus caller reliably. For this reason, let's
>> consider the function as a simple error code translator, and then
>> translating ERROR_SUCCESS to 0 (or is there ESUCCESS?) makes total sense.
>
> OK, that makes sense.
Actually, I do not think it makes that much sense, especially in the
context of this patch that claims to map success to 0 (assuming that
no E_ANYTHING has the value of zero---I am not sure POSIX gives such
a guarantee) "for good measure".
Even if Windows API makes the GetLastError() unusable after a
success call (unlike POSIX, where errno is left alone), the API
calls themselves would be signaling their own success or failure,
right? So I would have imagined that any kosher caller would be
doing this:
if (SomeWinAPI() != SUCCESS) {
errno = err_win_to_posix(GetLastError());
... possibly other reactions to the error ...
}
If using a value grabbed from GetLastError() after a successfull
Windows API call is a wrong thing to do as you taught us in your
message, then an unconditional
SomeWinAPI();
errno = err_win_to_posix(GetLastError());
would be wrong anyway, and touching errno unconditionally, when the
previous call may have succeeded without checking, makes the pattern
doubly wrong, no?
Having said that, I do not expect myself to be looking into and
fixing anything in compat/*win*.[ch], so I do not care too deeply
either way, but I thought that it would help keep the sanity of
developers involved if we touched errno only upon a failure from an
underlying system.
Thanks.
