On Tue, Oct 25, 2016 at 11:16:20AM -0700, Junio C Hamano wrote:

> diff --git a/sha1_file.c b/sha1_file.c
> index 5d2bcd3ed1..09045df1dc 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -1571,12 +1571,17 @@ int git_open(const char *name)
>               if (fd >= 0)
>                       return fd;
>  
> -             /* Might the failure be due to O_NOATIME? */
> -             if (errno != ENOENT && sha1_file_open_flag) {
> -                     sha1_file_open_flag = 0;
> +             /* 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.

Because of the order here, this would not be a regression (i.e., any
system that used to work will still eventually find a working comb), but
it does mean that systems without O_NOATIME do not get the benefit of
the new O_CLOEXEC protection.

Unfortunately, I think covering all of the cases would be 2^nr_flags.
That's only 4 right now, but it does not bode well as a pattern.

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.

-Peff

Reply via email to