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

From: Eric Wong <e@80x24.org>
Date: Oct 25, 2016, 21:33 UTC
Message-ID: <20161025213318.GB8683@starla>
In-Reply-To: <20161025181621.4201-4-gitster@pobox.com>

Junio C Hamano <gitster@pobox.com> wrote:

Show 13 quoted lines

> @@ -156,7 +156,14 @@ void fill_stat_cache_info(struct cache_entry *ce, struct stat *st)
>  static int ce_compare_data(const struct cache_entry *ce, struct stat *st)
>  {
>  	int match = -1;
> -	int fd = open(ce->name, O_RDONLY);
> +	static int cloexec = O_CLOEXEC;
> +	int fd = open(ce->name, O_RDONLY | cloexec);
> +
> +	if ((cloexec & O_CLOEXEC) && fd < 0 && errno == EINVAL) {
> +		/* Try again w/o O_CLOEXEC: the kernel might not support it */
> +		cloexec &= ~O_CLOEXEC;
> +		fd = open(ce->name, O_RDONLY | cloexec);
> +	}

Seems fine, I prefer not using recursion so it's easier-to-review.

But I have a _slight_ preference towards Dscho's version in <alpine.DEB.2.20.1610251230150.3264@virtualbox> in case we decide to start using another O_* flag in here. (but I'm not usually a C programmer)