Re: [PATCH 1/2] create_default_files(): don't set u+x bit on $GIT_DIR/config
- From
Michael Haggerty <mhagger@alum.mit.edu>
- Date
- Nov 16, 2014, 05:23 UTC
- Message-ID
- <546834ED.6050403@alum.mit.edu>
- In-Reply-To
- <546741AC.9030107@web.de>
On 11/15/2014 01:06 PM, Torsten Bögershausen wrote:
Show 10 quoted lines
> 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).
Show 17 quoted lines
>> 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