Re: [PATCH 1/2] compat/mingw: handle O_CLOEXEC in `mingw_open_existing()`

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> Our MinGW emulation of the open(3p) syscall uses one of three different
> code paths depending on the flags passed by the caller. Ideally, we
> would just use `_wopen()` for all of these directly and instead rely on
> the Windows SDK to implement the logic for us. But unfortunately, this
> interface does not allow us to set the `FILE_SHARING_*` flags, which we
> need to have control over to implement POSIX semantics.
>
> One of the code paths is for opening existing files, where we end up
> calling `mingw_open_existing()`. While this code path is executed when
> the user passes `O_NOINHERIT`, we don't know to handle `O_CLOEXEC` yet,
> which causes a couple of code paths that use the flag to not use the
> emulation. The consequence is that those code paths do not support POSIX
> semantics because we don't know to set the sharing mode correctly.
>
> Supporting `O_CLOEXEC` is quite trivial: we don't have to do anything,
> as Windows already closes the file handle by default when exec'ing into
> another process. This is further supported by the fact that we indeed
> define `O_CLOEXEC` as `O_NOINHERIT` in case the former isn't defined in
> "compat/mingw.h".
>
> Adapt the code so that we know to handle `O_CLOEXEC` in case it has a
> different definition than `O_NOINHERIT` to improve our POSIX semantics
> handling.

This one looks quite sensible and straight-forward even to a
non-Windows person like me.

> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  compat/mingw.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/compat/mingw.c b/compat/mingw.c
> index f524c54d06d..101e380c5a3 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -560,7 +560,7 @@ static int mingw_open_existing(const wchar_t *filename, int oflags, ...)
>  	int fd;
>  
>  	/* We only support basic flags. */
> -	if (oflags & ~(O_ACCMODE | O_NOINHERIT)) {
> +	if (oflags & ~(O_ACCMODE | O_NOINHERIT | O_CLOEXEC)) {
>  		errno = ENOSYS;
>  		return -1;
>  	}
> @@ -632,7 +632,7 @@ int mingw_open (const char *filename, int oflags, ...)
>  
>  	if ((oflags & O_APPEND) && !is_local_named_pipe_path(filename))
>  		open_fn = mingw_open_append;
> -	else if (!(oflags & ~(O_ACCMODE | O_NOINHERIT)))
> +	else if (!(oflags & ~(O_ACCMODE | O_NOINHERIT | O_CLOEXEC)))
>  		open_fn = mingw_open_existing;
>  	else
>  		open_fn = _wopen;