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

[PATCH 1/2] commit: fix "author_ident" leak

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Feb 16, 2022, 08:21 UTC
Message-ID
<patch-1.2-25202a1d07f-20220216T081844Z-avarab@gmail.com>
In-Reply-To
<cover-0.2-00000000000-20220216T081844Z-avarab@gmail.com>

Fix a leak in cmd_commit(), since 4c28e4ada03 (commit: die before asking to edit the log message, 2010-12-20) we have been freeing the "author_ident" "struct strbuf", but not in the case where prepare_to_commit() returns non-zero.

This fixes a leak demonstrated by e.g. "t3505-cherry-pick-empty.sh", but unfortunately we cannot mark it or other affected tests as passing now with "TEST_PASSES_SANITIZE_LEAK=true" as we'll need to fix many other memory leaks before doing so.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/commit.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 6b99ac276d8..696b3527adf 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1689,6 +1689,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	struct commit *current_head = NULL;
 	struct commit_extra_header *extra = NULL;
 	struct strbuf err = STRBUF_INIT;
+	int ret = 0;
 
 	if (argc == 2 && !strcmp(argv[1], "-h"))
 		usage_with_options(builtin_commit_usage, builtin_commit_options);
@@ -1723,8 +1724,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	   running hooks, writing the trees, and interacting with the user.  */
 	if (!prepare_to_commit(index_file, prefix,
 			       current_head, &s, &author_ident)) {
+		ret = 1;
 		rollback_index_files();
-		return 1;
+		goto cleanup;
 	}
 
 	/* Determine parents */
@@ -1822,7 +1824,6 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		rollback_index_files();
 		die(_("failed to write commit object"));
 	}
-	strbuf_release(&author_ident);
 	free_commit_extra_headers(extra);
 
 	if (update_head_with_reflog(current_head, &oid, reflog_msg, &sb,
@@ -1863,7 +1864,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 
 	apply_autostash(git_path_merge_autostash(the_repository));
 
+cleanup:
+	strbuf_release(&author_ident);
 	UNLEAK(err);
 	UNLEAK(sb);
-	return 0;
+	return ret;
 }
-- 
2.35.1.1028.g2d2d4be19de
Previous: Ævar Arnfjörð BjarmasonNext: Junio C Hamano
Message 2 of 12 in “commit: trivial leak fix, add 2 tests to linux-leaks CI”
  1. 0/2 commit: trivial leak fix, add 2 tests to linux-leaks CIÆvar Arnfjörð Bjarmason, Feb 16, 2022
  2. 1/2 commit: fix "author_ident" leakÆvar Arnfjörð Bjarmason, Feb 16, 2022
  3. Junio C HamanoFeb 16, 2022
  4. 2/2 commit: use strbuf_release() instead of UNLEAK()Ævar Arnfjörð Bjarmason, Feb 16, 2022
  5. Junio C HamanoFeb 16, 2022
  6. Junio C HamanoFeb 16, 2022
  7. Whether to keep using UNLEAK() in built-ins (was: [PATCH 2/2] commit: use strbuf_release() instead of UNLEAK())Ævar Arnfjörð Bjarmason, Feb 18, 2022
  8. Junio C HamanoFeb 18, 2022
  9. Ævar Arnfjörð BjarmasonFeb 18, 2022
  10. commit: fix "author_ident" leakJunio C Hamano, May 12, 2022
  11. Ævar Arnfjörð BjarmasonMay 17, 2022
  12. Junio C HamanoMay 18, 2022

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.