[PATCH] git.c: fix help.autocorrect after 57ea712 breaks it
From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-15 23:07:54
Subsystem:
the rest · Maintainer:
Linus Torvalds
Commit 57ea712 (git.c: make sure we do not leak GIT_* to alias scripts - 2015-12-20) does not realize that handle_alias() can be called multiple times because of the forever loop in run_argv(). The commit breaks alias chains. Suppose you have an alias "abc" that resolves to another alias "def", which finally resolve to "git status". handle_alias() is called twice. save_env() and restore_env() are also called twice. But because of the check save_env_before_alias in save_env(), we save once while trying to restore twice. Consequences are left for the reader's imagination. Fortunately, you cannot make an alias of another alias. At least not yet. Unfortunately it can still happen with help.autocorrect, where your alias typo is treated as the first "alias", and it can be resolved to the second alias. Then boom. Make sure we call save_env() and restore_env() in pairs. While at there, set orig_cwd to NULL after freeing it for hygiene. Reported-by: Michael J Gruber <redacted> Signed-off-by: Nguyễn Thái Ngọc Duy <redacted> --- git.c | 11 ++++++++--- t/t1300-repo-config.sh | 8 ++++++++ 2 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/git.c b/git.c
index da278c3..cd733f7 100644
--- a/git.c
+++ b/git.c@@ -25,14 +25,15 @@ static const char *env_names[] = { GIT_PREFIX_ENVIRONMENT }; static char *orig_env[4]; -static int saved_env_before_alias; +static int saved_env_before_alias, saved; static void save_env_before_alias(void) { int i; - if (saved_env_before_alias) - return; + if (saved) + die("BUG: uneven pair of save_env/restore_env calls"); saved_env_before_alias = 1; + saved = 1; orig_cwd = xgetcwd(); for (i = 0; i < ARRAY_SIZE(env_names); i++) { orig_env[i] = getenv(env_names[i]);
@@ -44,9 +45,13 @@ static void save_env_before_alias(void) static void restore_env(int external_alias) { int i; + if (saved != 1) + die("BUG: uneven pair of save_env/restore_env calls"); if (!external_alias && orig_cwd && chdir(orig_cwd)) die_errno("could not move to %s", orig_cwd); free(orig_cwd); + orig_cwd = NULL; + saved = 0; for (i = 0; i < ARRAY_SIZE(env_names); i++) { if (external_alias && !strcmp(env_names[i], GIT_PREFIX_ENVIRONMENT))
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 52678e7..3f95285 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh@@ -1201,4 +1201,12 @@ test_expect_success POSIXPERM,PERL 'preserves existing permissions' ' "die q(badrename) if ((stat(q(.git/config)))[2] & 07777) != 0600" ' +test_expect_success 'autocorrect and save_env/restore_env' ' + git config alias.ss status && + git config help.autocorrect 1 && + git sss --porcelain | grep actual >actual && + echo "?? actual" >expected && + test_cmp expected actual +' + test_done
--
2.7.0.288.g1d8ad15