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

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 26, 2016, 16:23 UTC
Message-ID: <xmqqa8drcc5i.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20161026042555.neaxvnmggtcku5cc@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 18 quoted lines

>> +		/* Try again w/o O_CLOEXEC: the kernel might not support it */
>> +		if ((sha1_file_open_flag & O_CLOEXEC) && errno == EINVAL) {
>> +			sha1_file_open_flag &= ~O_CLOEXEC;
>>  			continue;
>>  		}
>
> So if we start with O_CLOEXEC|O_NOATIME, we drop CLOEXEC here and try
> again with just O_NOATIME. And then if _that_ fails...
>
>> +		/* Might the failure be due to O_NOATIME? */
>> +		if (errno != ENOENT && (sha1_file_open_flag & O_NOATIME)) {
>> +			sha1_file_open_flag &= ~O_NOATIME;
>> +			continue;
>> +		}
>
> We drop O_NOATIME, and end up with an empty flag field.
>
> But we will never have tried just O_CLOEXEC, which might have worked.

Yes, doing so would smudge atime, so one question is which one between noatime or cloexec is more important to be done at open(2) time.

It may be possible to open(2) only with cloexec and then fcntl(2) FD_SET noatime immediately after, but going that way would explode the combination even more, as it may not be possible to set these two flags the other way around.

> I'm not sure it's worth worrying about or not; I don't know which
> systems are actually lacking either of the flags, or if they tend to
> have both.