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

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

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Fri, Oct 28, 2016 at 4:11 AM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>>
>> You guys. I mean: You guys! You sure make my life hard. A brief look at
>> mingw.h could have answered your implicit question:
>
> So here's what you guys should do:
>
>  - leave O_NOATIME damn well alone. It works. It has worked for 10+
> years. Stop arguing against it, people who do.
>
>  - get rid of all O_CLOEXEC games. They don't work. If you want to
> close file descriptors at execve(), you - gasp - close the file
> descriptor before doing an execve.
>
> So O_CLOEXEC or FD_CLOEXEC is broken.
>
> DO NOT BREAK O_NOATIME JUST TO ADD COMPLETELY NEW BREAKAGE.

Excuse me, but care to elaborate a bit more?

Did I botch the way I ask fcntl(2) to set O_NOATIME in *1*?  Its
follow-up in *2* is optional and as peff said in *3*, I do not think
we mind dropping that step and keeping O_NOATIME.  It is not that
much code to keep.

Setting FD_CLOEXEC with fcntl(2) may be racy, but in what way
it is broken to open(2) with O_CLOEXEC?

If we want to close file descriptors we opened at execve() time
ourselves, we'd need to somehow keep track of which file descriptors
are meant to be closed upon execve() time, and one way to mark that
may be to use O_CLOEXEC, but once we mark a file descriptor with the
bit, execve() would take care of "closing" it for us---wouldn't that
be the whole point of that bit?


*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
*3* http://public-inbox.org/git/20161028075104.la24zydnr3ogb6qv@sigill.intra.peff.net