Re: [PATCH v3 2/3] sha1_file: open window into packfiles with O_CLOEXEC

Linus Torvalds [off-list ref] writes:

Apparently windows doesn't even support it, at least not mingw....

Assuming that the above was a misunderstanding, assuming that we can
do O_CLOEXEC (but not FD_CLOEXEC) on Windows just fine, where stray
file descriptors held open in the children matter more, and ...

In contrast, O_NOATIME isn't a maintenance problem, since it's purely
an optimization and has no semantic difference, so it not existing on
some platform is immaterial.

... assuming that I didn't screw up my use of fcntl() to set
O_NOATIME on a fd opened with O_CLOEXEC, are you OK with the
approach in patch *1*?  We can drop *2* to keep O_NOATIME, which has
been working for those with old kernels with many loose objects, and
it is not too much code to keep.

*1* http://public-inbox.org/git/xmqqh97w38gj.fsf@gitster.mtv.corp.google.com
*2* http://public-inbox.org/git/xmqqd1ik38f4.fsf@gitster.mtv.corp.google.com

Hi,

On Fri, 28 Oct 2016, Junio C Hamano wrote:

Linus Torvalds [off-list ref] writes:

quoted

Apparently windows doesn't even support it, at least not mingw....

Assuming that the above was a misunderstanding, assuming that we can
do O_CLOEXEC (but not FD_CLOEXEC) on Windows just fine, where stray
file descriptors held open in the children matter more, and ...

Correct. We cannot change an open file handle's state ("FD_CLOEXEC") on
Windows, but we can ask open() to open said file handle with the correct
flag ("O_CLOEXEC", which is mapped to O_NOINHERIT on Windows.

And for the record: I agree with Junio that we cannot simply drop this
O_CLOEXEC business.

Because it was introduced to fix bugs.

Thank you for your time,
Johannes

On Sat, Oct 29, 2016 at 1:25 AM, Johannes Schindelin
[off-list ref] wrote:

Correct. We cannot change an open file handle's state ("FD_CLOEXEC") on
Windows, but we can ask open() to open said file handle with the correct
flag ("O_CLOEXEC", which is mapped to O_NOINHERIT on Windows.

Ok. So then I have no issues with it, and let's use O_CLOEXEC if it
exists and fcntl(FD_CLOEXEC) if O_CLOEXEC doesn't exist.

              Linus