summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 60aa9cf)
raw | patch | inline | side by side (parent: 60aa9cf)
author | Kirill Smelkov <kirr@mns.spb.ru> | |
Thu, 11 Nov 2010 18:08:23 +0000 (21:08 +0300) | ||
committer | Junio C Hamano <gitster@pobox.com> | |
Sat, 13 Nov 2010 00:03:27 +0000 (16:03 -0800) |
getenv(3) returns not-permanent buffer which may be changed by e.g.
putenv(3) call (*).
In practice I've noticed this when trying to do `git commit -m abc`
inside msysgit under wine, getting
$ git commit -m abc
fatal: could not open 'DIR=.git/COMMIT_EDITMSG': No such file or directory
^^^^
(notice introduced 'DIR=' artifact.)
The problem was showing itself only with -m option, and actually, as
debugging showed, originally
git_dir = getenv("GIT_DIR")
returned pointer to
"GIT_DIR=.git\0"
^
git_dir
, we stored it in git_dir, than, after processing -m git-commit option,
we did setenv("GIT_EDITOR", ":") which as (*) says changed environment
variables memory layout - something like this
"...\0GIT_DIR=.git\0"
^
git_dir
and oops - we got wrong git_dir.
Avoid that by strdupping getenv("GIT_DIR") result like we did in 06f354
(setup: make sure git dir path is in a permanent buffer). Unfortunately
this also shows that other getenv usage inside git needs auditing...
(*) from man 3 getenv:
The implementation of getenv() is not required to be reentrant. The
string pointed to by the return value of getenv() may be statically
allocated, and can be modified by a subsequent call to getenv(),
putenv(3), setenv(3), or unsetenv(3).
Cc: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Kirill Smelkov <kirr@mns.spb.ru>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
putenv(3) call (*).
In practice I've noticed this when trying to do `git commit -m abc`
inside msysgit under wine, getting
$ git commit -m abc
fatal: could not open 'DIR=.git/COMMIT_EDITMSG': No such file or directory
^^^^
(notice introduced 'DIR=' artifact.)
The problem was showing itself only with -m option, and actually, as
debugging showed, originally
git_dir = getenv("GIT_DIR")
returned pointer to
"GIT_DIR=.git\0"
^
git_dir
, we stored it in git_dir, than, after processing -m git-commit option,
we did setenv("GIT_EDITOR", ":") which as (*) says changed environment
variables memory layout - something like this
"...\0GIT_DIR=.git\0"
^
git_dir
and oops - we got wrong git_dir.
Avoid that by strdupping getenv("GIT_DIR") result like we did in 06f354
(setup: make sure git dir path is in a permanent buffer). Unfortunately
this also shows that other getenv usage inside git needs auditing...
(*) from man 3 getenv:
The implementation of getenv() is not required to be reentrant. The
string pointed to by the return value of getenv() may be statically
allocated, and can be modified by a subsequent call to getenv(),
putenv(3), setenv(3), or unsetenv(3).
Cc: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Kirill Smelkov <kirr@mns.spb.ru>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
environment.c | patch | blob | history |
diff --git a/environment.c b/environment.c
index de5581fe51d532231b0121bd2ef2e46669015bda..18aded6dc433ebc19c8b34e8ebcf59979a1985f6 100644 (file)
--- a/environment.c
+++ b/environment.c
static void setup_git_env(void)
{
git_dir = getenv(GIT_DIR_ENVIRONMENT);
+ git_dir = git_dir ? xstrdup(git_dir) : NULL;
if (!git_dir) {
git_dir = read_gitfile_gently(DEFAULT_GIT_DIR_ENVIRONMENT);
git_dir = git_dir ? xstrdup(git_dir) : NULL;