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

Andreas Ericsson [off-list ref] writes:

Johannes Schindelin wrote:

quoted

Hi,

On Thu, 4 Oct 2007, Martin Waitz wrote:

quoted

-		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?

The issue that started the thread was that chmod + stat check we
originally had would say executable bit "seems to be" kept,
while that is only true until the information is cached at VFS
layer.

We create config file without asking for executable bit, so if
we read it back as executable then that is a sure sign that the
filesystem does not know what it is talking about, and we set
filemode to zero in such a case.  Similarly, if the chmod + stat
check says we cannot set executable bit and read it back, then
we also know the filesystem does not know about filemode.

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 */
	    );

hoi :)

On Wed, Oct 03, 2007 at 11:23:22PM -0700, Junio C Hamano wrote:

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 */
	    );

that looks good.

-- 
Martin Waitz

Junio C Hamano wrote:

Andreas Ericsson [off-list ref] writes:

quoted

Johannes Schindelin wrote:

quoted

Hi,

On Thu, 4 Oct 2007, Martin Waitz wrote:

quoted

-		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.

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