From: Junio C Hamano Date: Sun, 01 Dec 2019 06:26:42 GMT Subject: Re: [PATCH v2 0/2] Brown-bag fix on top of js/mingw-inherit-only-std-handles Message-ID: In-Reply-To: Junio C Hamano writes: > Johannes Sixt 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.