Re: [PATCH v2 2/3] compat/mingw: allow deletion of most opened files

> On Windows, we emulate open(3p) via `mingw_open()`. This function
> implements handling of some platform-specific quirks that are required
> to make it behave as closely as possible like open(3p) would, but for
> most cases we just call the Windows-specific `_wopen()` function.
> 
> This function has a major downside though: it does not allow us to
> specify the sharing mode. While there is `_wsopen()` that allows us to
> pass sharing flags, those sharing flags are not the same `FILE_SHARE_*`
> flags as `CreateFileW()` accepts. Instead, `_wsopen()` only allows
> concurrent read- and write-access, but does not allow for concurrent
> deletions. Unfortunately though, we have to allow concurrent deletions
> if we want to have POSIX-style atomic renames on top of an existing file
> that has open file handles.
> 
> Implement a new function that emulates open(3p) for existing files via
> `CreateFileW()` such that we can set the required sharing flags.
> 
> While we have the same issue when calling open(3p) with `O_CREAT`,
> implementing that mode would be more complex due to the required
> permission handling. Furthermore, atomic updates via renames typically
> write to exclusive lockfile and then perform the rename, and thus we
> don't have to handle the case where the locked path has been created
> with `O_CREATE`. So while it would be nice to have proper POSIX
> semantics in all paths, we instead aim for a minimum viable fix here.

> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  compat/mingw.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
> 
> diff --git a/compat/mingw.c b/compat/mingw.c
> index e326c6fcd2d..6c75fe36b15 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -532,6 +532,62 @@ static int mingw_open_append(wchar_t const *wfilename, int oflags, ...)
>  	return fd;
>  }
>  
> +/*
> + * Ideally, we'd use `_wopen()` to implement this functionality so that we
> + * don't have to reimplement it, but unfortunately we do not have tight control
> + * over the share mode there. And while `_wsopen()` and friends exist that give
> + * us _some_ control over the share mode, this family of functions doesn't give
> + * us the ability to enable FILE_SHARE_DELETE, either. But this is a strict
> + * requirement for us though so that we can unlink or rename over files that
> + * are held open by another process.
> + *
> + * We are thus forced to implement our own emulation of `open()`. To make our
> + * life simpler we only implement basic support for this, namely opening
> + * existing files for reading and/or writing. This means that newly created
> + * files won't have their sharing mode set up correctly, but for now I couldn't
> + * find any case where this matters. We may have to revisit that in the future
> + * though based on our needs.
> + */
> +static int mingw_open_existing(const wchar_t *filename, int oflags, ...)
> +{
> +	SECURITY_ATTRIBUTES security_attributes = {
> +		.nLength = sizeof(security_attributes),
> +		.bInheritHandle = !(oflags & O_NOINHERIT),
> +	};
> +	HANDLE handle;
> +	int access;

> +	int fd;
> +
> +	/* We only support basic flags. */
> +	if (oflags & ~(O_ACCMODE | O_NOINHERIT)) {
> +		errno = ENOSYS;
> +		return -1;
> +	}
> +
> +	if (oflags & O_RDWR)
> +		access = GENERIC_READ | GENERIC_WRITE;
> +	else if (oflags & O_WRONLY)
> +		access = GENERIC_WRITE;
> +	else
> +		access = GENERIC_READ;

> +
> +	handle = CreateFileW(filename, access,
> +			     FILE_SHARE_WRITE | FILE_SHARE_READ | FILE_SHARE_DELETE,
> +			     &security_attributes, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
> +	if (handle == INVALID_HANDLE_VALUE) {
> +		DWORD err = GetLastError();
> +		if (err == ERROR_INVALID_PARAMETER)
> +			err = ERROR_PATH_NOT_FOUND;

> +		errno = err_win_to_posix(err);
> +		return -1;
> +	}
> +
> +	fd = _open_osfhandle((intptr_t)handle, oflags | O_BINARY);
> +	if (fd < 0)
> +		CloseHandle(handle);
> +	return fd;
> +}
> +
>  /*
>   * Does the pathname map to the local named pipe filesystem?
>   * That is, does it have a "//./pipe/" prefix?
> @@ -567,6 +623,8 @@ 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)))
> +		open_fn = mingw_open_existing;
>  	else
>  		open_fn = _wopen;
>