git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] git.c: fix help.autocorrect after 57ea712 breaks it

From
Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Date
Jan 26, 2016, 13:26 UTC
Message-ID
<1453814801-1925-1-git-send-email-pclouds@gmail.com>
In-Reply-To
<56A72235.9080602@drmicha.warpmail.net>

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@drmicha.warpmail.net>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 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
Previous: Duy NguyenNext: Michael J Gruber
Message 3 of 22 in “[BUG] typo DWIMery with alias broken (cd to random dir)”
  1. Michael J GruberJan 26, 2016
  2. Duy NguyenJan 26, 2016
  3. git.c: fix help.autocorrect after 57ea712 breaks itNguyễn Thái Ngọc Duy, Jan 26, 2016
  4. Michael J GruberJan 26, 2016
  5. Junio C HamanoJan 26, 2016
  6. Junio C HamanoJan 26, 2016
  7. Junio C HamanoJan 26, 2016
  8. Junio C HamanoJan 27, 2016
  9. 2/3 git: protect against unbalanced calls to {save,restore}_env()Junio C Hamano, Jan 27, 2016
  10. 2/3 git: protect against unbalanced calls to {save,restore}_env()Junio C Hamano, Jan 27, 2016
  11. 3/3 git: simplify environment save/restore logicJunio C Hamano, Jan 27, 2016
  12. 3/3 git: simplify environment save/restore logicJunio C Hamano, Jan 27, 2016
  13. Junio C HamanoJan 27, 2016
  14. Duy NguyenJan 27, 2016
  15. Junio C HamanoJan 27, 2016
  16. 1/3 git: remove an early return from save_env_before_alias()Junio C Hamano, Jan 27, 2016
  17. 1/4 git: remove an early return from save_env_before_alias()Junio C Hamano, Feb 2, 2016
  18. 2/4 git: protect against unbalanced calls to {save,restore}_env()Junio C Hamano, Feb 2, 2016
  19. 3/4 git: simplify environment save/restore logicJunio C Hamano, Feb 2, 2016
  20. 4/4 restore_env(): free the saved environment variable once we are doneJunio C Hamano, Feb 2, 2016
  21. Eric SunshineFeb 3, 2016
  22. Junio C HamanoFeb 3, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.