Re: [PATCH v2 2/2] read-cache: make sure file handles are not inherited by child processes

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

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> That still looks overly complicated, repeatedly ORing cloexec and
> recursing without need. How about this instead?
>
> 	static int oflags = O_RDONLY | O_CLOEXEC;
> 	int fd = open(ce->name, oflags);
>
> 	if ((O_CLOEXEC & oflags) && fd < 0 && errno == EINVAL) {
>   		/* Try again w/o O_CLOEXEC: the kernel might not support it */
> 		oflags &= ~O_CLOEXEC;
> 		fd = open(ce->name, oflags);
> 	}

I deliberately separated the part that can and designed to be
toggled (O_CLOEXEC) and the part that is meant to be constant
(O_RDONLY), and I do not think the first part of suggestion is
particularly a good idea.

I didn't write the same open twice, but I agree that an extra "we
fallback to opening with a different flags" inside the if () { }
block that is there exactly for implementing that fallback is an
excellent idea.  I like that part of the suggestion.

Thanks.