Re: [PATCH] git-init: don't base core.filemode on the ability to chmod.

From: Andreas Ericsson <ae@op5.se>
Date: Oct 4, 2007, 10:33 UTC
Message-ID: <4704C161.3000006@op5.se>
In-Reply-To: <7vr6kbbdph.fsf@gitster.siamese.dyndns.org>

Junio C Hamano wrote:

Show 18 quoted lines

> Andreas Ericsson <ae@op5.se> writes:
> 
>> Johannes Schindelin wrote:
>>> Hi,
>>>
>>> On Thu, 4 Oct 2007, Martin Waitz wrote:
>>>
>>>> -		filemode = (!chmod(path, st1.st_mode ^ S_IXUSR) &&
>>>> +		/* test that new files are not created with X bit */
>>>> +		filemode = !(st1.st_mode & S_IXUSR);
>>>> +		/* test that we can modify the X bit */
>>>> +		filemode &= (!chmod(path, st1.st_mode ^ S_IXUSR) &&
>>> Should that not be &&=?
>>>
>> I should think |=
> 
> Is it?
>

Nopes. I misread the first expression and simply assumed that "filemode" should be != 0 for FS not supporting the x bit. I'd rename the variable to bogus_fs and flip the logic, but I have no strong opinion either way.

Show 12 quoted lines

> 
> So I think we can write it like this (indentation aside)...
> 
> filemode = !( (st1.st_mode & S_IXUSR)
>         	/* we did not ask for x-bit -- bogus FS */
> 	    || chmod(path, st1.st_mode & S_IXUSR)
>         	/* it does not let us flip x-bit -- bogus FS */
> 	    || lstat(path, &st2)
>         	/* it does not let us read back -- bogus FS */
> 	    || (st1.st_mode == st2.st_mode)
> 	        /* it forgets we flipped -- bogus FS */
> 	    );

For "filemode=0 means FS doesn't support x-bit" it looks about right, but kinda cumbersome to read.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231