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 <git@xxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
---
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html