FD_CLOEXEC set twice on systems that support O_CLOEXEC
rittneje opened this issue · 5 comments
#275 added an unconditional call to fcntl
to set FD_CLOEXEC
. However, this call should only be made on systems that do not support O_CLOEXEC
. While setting it a second time won't cause a problem per se, it is not necessary.
One possible fix is to change this to #if defined(FD_CLOEXEC) && !defined(O_CLOEXEC)
.
Lines 422 to 427 in 26fac50
cc @oschwald
Sorry for the confusion on this point by the way. When I wrote that snippet in #273 I was thinking of it more in isolation.
I actually did have #if !defined(O_CLOEXEC) && defined(FD_CLOEXEC)
before committing, but looking at other project, I noticed most others did not bother with the O_CLOEXEC
check. I removed it as it seemed harmless to use fcntl
in this case and I wasn't sure if it was ever possible for O_CLOEXEC
to be defined but for open
to not support the flag.
I believe that if O_CLOEXEC
were defined but open
somehow did not support it, you could also get EINVAL
back at runtime.
I guess another thing you could do is check the flags that come back from F_GETFD
and only call F_SETFD
if FD_CLOEXEC
isn't already set. That at least cuts out one extraneous syscall.
Yeah, checking the flags might be a good compromise to eliminate the extra syscall. Things like this OpenJDK check are what gave me pause about O_CLOEXEC
being available but not working. I assume it would likely only happen with a very old Linux kernel, but my confidence is pretty low.
That is describing a bug in AIX where open
will return EINVAL
. https://www.ibm.com/support/pages/apar/IV90804
Additional changes would be needed to make that work since it is a runtime failure (and it would have been broken since this library started using O_CLOEXEC
in 1.3.2 anyway).