| Message ID | 20250313-b4-pks-mingw-lockfile-flake-v1-1-bc5d3e70f516@pks.im (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | compat/mingw: fix EACCESS when opening files with `O_CREAT | O_EXCL` | expand |
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;
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;
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. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- compat/mingw.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)