Re: [PATCH 1/2] create_default_files(): don't set u+x bit on $GIT_DIR/config

[Michael Haggerty <mhagger@alum.mit.edu>]

On 11/15/2014 01:06 PM, Torsten Bögershausen wrote:
> On 2014-11-15 08.26, Michael Haggerty wrote:
> The whole thing looks good to me, some minor comments below
>> git_config_set() copies the permissions from the old config file to
>> the new one. This is a good change in and of itself, but it interacts
>> badly with create_default_files()'s sloppiness, causing "git init" to
>> leave the executable bit set on $GIT_DIR/config.
>>
>> So change create_default_files() to reset the permissions on
> s/permissions/executable bit/ ?
>> $GIT_DIR/config after its test.

The code literally resets all of the permissions to their values before
the test, so I think the existing text is more accurate (even though
your version would have been fine, too).

>> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
>> ---
>>  builtin/init-db.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/builtin/init-db.c b/builtin/init-db.c
>> index 56f85e2..95ca5e4 100644
>> --- a/builtin/init-db.c
>> +++ b/builtin/init-db.c
>> @@ -255,6 +255,7 @@ static int create_default_files(const char *template_path)
>>  		filemode = (!chmod(path, st1.st_mode ^ S_IXUSR) &&
>>  				!lstat(path, &st2) &&
>>  				st1.st_mode != st2.st_mode);
>> +		chmod(path, st1.st_mode);
> A "blind" chmod() is good, but I think checking the return code is better.
> 
>                 filemode &= (!chmod(path, st1.st_mode));

I guess it is better to include this test, even though it is mostly
redundant with what was already determined by the previous line. I
suppose the only alternative would be to

    die("Your chmod() support is bonkers")

I will change this in v2.

Thanks for your comments!

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu