Hi Junio, On Mon, 24 Oct 2016, Junio C Hamano wrote:
> Eric Wong <[email protected]> writes: > > > [email protected] 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
