Hi Junio,
On Mon, 24 Oct 2016, Junio C Hamano wrote:
> Eric Wong <e@xxxxxxxxx> writes:
>
> > larsxschneider@xxxxxxxxx wrote:
> >> +++ b/read-cache.c
> >> @@ -156,7 +156,11 @@ 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);
> >> + int fd = open(ce->name, O_RDONLY | O_CLOEXEC);
> >> +
> >> + if (O_CLOEXEC && fd < 0 && errno == EINVAL)
> >> + /* Try again w/o O_CLOEXEC: the kernel might not support it */
> >> + fd = open(ce->name, O_RDONLY);
> >
> > In the case of O_CLOEXEC != 0 and repeated EINVALs,
> > it'd be good to use something like sha1_file_open_flag as in 1/2
> > so we don't repeatedly hit EINVAL. Thanks.
>
> Sounds sane.
>
> It's just only once, so perhaps we do not mind a recursion like
> this?
>
> read-cache.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/read-cache.c b/read-cache.c
> index b594865d89..a6978b9321 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -156,11 +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 | O_CLOEXEC);
> + static int cloexec = O_CLOEXEC;
> + int fd = open(ce->name, O_RDONLY | cloexec);
>
> - if (O_CLOEXEC && fd < 0 && errno == EINVAL)
> + if ((cloexec & O_CLOEXEC) && fd < 0 && errno == EINVAL) {
> /* Try again w/o O_CLOEXEC: the kernel might not support it */
> - fd = open(ce->name, O_RDONLY);
> + cloexec &= ~O_CLOEXEC;
> + return ce_compare_data(ce, st);
> + }
>
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);
}
Ciao,
Dscho