Re: [PATCH v3 0/2] Brown-bag fix on top of js/mingw-inherit-only-std-handles
From: Johannes Schindelin <hidden>
Date: 2019-12-03 12:05:08
Hi Hannes, On Mon, 2 Dec 2019, Johannes Sixt wrote:
Am 02.12.19 um 12:33 schrieb Johannes Schindelin via GitGitGadget:quoted
Range-diff vs v2: 1: 280b6d08a4 = 1: 280b6d08a4 mingw: do set `errno` correctly when trying to restrict handle inheritance 2: c3dea00fb1 ! 2: e04e1269b3 mingw: translate ERROR_SUCCESS to errno = 0 @@ -3,12 +3,15 @@ mingw: translate ERROR_SUCCESS to errno = 0The subject line would have to be adjusted as well. mingw: forbid translating ERROR_SUCCESS to an errno value or something.
Oy. Thank you for catching this! (It is always amazing to me how much I miss when I have stared at the same commits for a while.) For good measure, I force-pushed the branch of the PR to match what Junio has in `pu`, but if there are no other changes I need to make, I will refrain from submitting another iteration. Thanks, Dscho
quoted
Johannes Sixt pointed out that the `err_win_to_posix()` function - mishandles `ERROR_SUCCESS`. This commit fixes that. + mishandles `ERROR_SUCCESS`: it maps it to `ENOSYS`. - Technically, we try to only assign `errno` to the corresponding value of - `GetLastError()` (which translation is performed by that function) when - a Win32 API call failed, so this change is purely defensive and is not - expected to fix an existing bug in our code base. + The only purpose of this function is to map Win32 API errors to `errno` + ones, and there is actually no equivalent to `ERROR_SUCCESS`: the idea + of `errno` is that it will only be set in case of an error, and left + alone in case of success. + + Therefore, as pointed out by Junio Hamano, it is a bug to call this + function when there was not even any error to map. Signed-off-by: Johannes Schindelin [off-list ref] @@ -19,7 +22,7 @@ case ERROR_SHARING_BUFFER_EXCEEDED: error = ENFILE; break; case ERROR_SHARING_VIOLATION: error = EACCES; break; case ERROR_STACK_OVERFLOW: error = ENOMEM; break; -+ case ERROR_SUCCESS: error = 0; break; ++ case ERROR_SUCCESS: BUG("err_win_to_posix() called without an error!"); case ERROR_SWAPERROR: error = ENOENT; break; case ERROR_TOO_MANY_MODULES: error = EMFILE; break; case ERROR_TOO_MANY_OPEN_FILES: error = EMFILE; break;-- Hannes