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

[PATCH v3 02/27] bisect: fix leaking good/bad terms when reading multipe times

From
Patrick Steinhardt <ps@pks.im>
Date
Nov 20, 2024, 13:39 UTC
Message-ID
<20241120-b4-pks-leak-fixes-pt10-v3-2-d67f08f45c74@pks.im>
In-Reply-To
<20241120-b4-pks-leak-fixes-pt10-v3-0-d67f08f45c74@pks.im>

Even though `read_bisect_terms()` is declared as assigning string constants, it in fact assigns allocated strings to the `read_bad` and `read_good` out parameters. The only callers of this function assign the result to global variables and thus don't have to free them in order to be leak-free. But that changes when executing the function multiple times because we'd then overwrite the previous value and thus make it unreachable.

Fix the function signature and free the previous values. This leak is exposed by t0630, but plugging it does not make the whole test suite pass.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 bisect.c   | 14 +++++++++-----
 bisect.h   |  2 +-
 revision.c |  4 ++--
 3 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/bisect.c b/bisect.c
index 4713226fad4e16ebbda4ba2e8d13c6eaf7d4bfb2..9c52241050c14ecbf3f7950a0c5e7b962fcaa541 100644
--- a/bisect.c
+++ b/bisect.c
@@ -28,8 +28,8 @@ static struct oid_array skipped_revs;
 
 static struct object_id *current_bad_oid;
 
-static const char *term_bad;
-static const char *term_good;
+static char *term_bad;
+static char *term_good;
 
 /* Remember to update object flag allocation in object.h */
 #define COUNTED		(1u<<16)
@@ -985,7 +985,7 @@ static void show_commit(struct commit *commit)
  * We read them and store them to adapt the messages accordingly.
  * Default is bad/good.
  */
-void read_bisect_terms(const char **read_bad, const char **read_good)
+void read_bisect_terms(char **read_bad, char **read_good)
 {
 	struct strbuf str = STRBUF_INIT;
 	const char *filename = git_path_bisect_terms();
@@ -993,16 +993,20 @@ void read_bisect_terms(const char **read_bad, const char **read_good)
 
 	if (!fp) {
 		if (errno == ENOENT) {
-			*read_bad = "bad";
-			*read_good = "good";
+			free(*read_bad);
+			*read_bad = xstrdup("bad");
+			free(*read_good);
+			*read_good = xstrdup("good");
 			return;
 		} else {
 			die_errno(_("could not read file '%s'"), filename);
 		}
 	} else {
 		strbuf_getline_lf(&str, fp);
+		free(*read_bad);
 		*read_bad = strbuf_detach(&str, NULL);
 		strbuf_getline_lf(&str, fp);
+		free(*read_good);
 		*read_good = strbuf_detach(&str, NULL);
 	}
 	strbuf_release(&str);
diff --git a/bisect.h b/bisect.h
index ee3fd65f3bdb89611d7575239ee79e8c50bcd703..944439bfac729cf092d6c5962c9edff2321002e4 100644
--- a/bisect.h
+++ b/bisect.h
@@ -75,7 +75,7 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix);
 
 int estimate_bisect_steps(int all);
 
-void read_bisect_terms(const char **bad, const char **good);
+void read_bisect_terms(char **bad, char **good);
 
 int bisect_clean_state(void);
 
diff --git a/revision.c b/revision.c
index 8df75b82249b36e7e3ee9a4a151f085c189acc45..347dabf7f9a1a87765150ec998deae9b6709b5f6 100644
--- a/revision.c
+++ b/revision.c
@@ -51,8 +51,8 @@
 
 volatile show_early_output_fn_t show_early_output;
 
-static const char *term_bad;
-static const char *term_good;
+static char *term_bad;
+static char *term_good;
 
 implement_shared_commit_slab(revision_sources, char *);
 
-- 
2.47.0.274.g962d0b743d.dirty
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 3 of 33 in “Memory leak fixes (pt.10, final)”
  1. 00/27 Memory leak fixes (pt.10, final)Patrick Steinhardt, Nov 20, 2024
  2. 01/27 builtin/blame: fix leaking blame entries with `--incremental`Patrick Steinhardt, Nov 20, 2024
  3. 02/27 bisect: fix leaking good/bad terms when reading multipe timesPatrick Steinhardt, Nov 20, 2024
  4. 04/27 bisect: fix leaking `current_bad_oid`Patrick Steinhardt, Nov 20, 2024
  5. 03/27 bisect: fix leaking string in `handle_bad_merge_base()`Patrick Steinhardt, Nov 20, 2024
  6. 05/27 bisect: fix multiple leaks in `bisect_next_all()`Patrick Steinhardt, Nov 20, 2024
  7. 06/27 bisect: fix leaking commit list items in `check_merge_base()`Patrick Steinhardt, Nov 20, 2024
  8. 07/27 bisect: fix various cases where we leak commit list itemsPatrick Steinhardt, Nov 20, 2024
  9. Jeff KingNov 25, 2024
  10. Patrick SteinhardtNov 25, 2024
  11. Jeff KingNov 25, 2024
  12. Patrick SteinhardtNov 25, 2024
  13. 08/27 line-log: fix leak when rewriting commit parentsPatrick Steinhardt, Nov 20, 2024
  14. 09/27 strvec: introduce new `strvec_splice()` functionPatrick Steinhardt, Nov 20, 2024
  15. 10/27 git: refactor alias handling to use a `struct strvec`Patrick Steinhardt, Nov 20, 2024
  16. 11/27 git: refactor builtin handling to use a `struct strvec`Patrick Steinhardt, Nov 20, 2024
  17. 12/27 split-index: fix memory leak in `move_cache_to_base_index()`Patrick Steinhardt, Nov 20, 2024
  18. 13/27 builtin/sparse-checkout: fix leaking sanitized patternsPatrick Steinhardt, Nov 20, 2024
  19. 14/27 help: refactor to not use globals for reading configPatrick Steinhardt, Nov 20, 2024
  20. 15/27 help: fix leaking `struct cmdnames`Patrick Steinhardt, Nov 20, 2024
  21. 16/27 help: fix leaking return value from `help_unknown_cmd()`Patrick Steinhardt, Nov 20, 2024
  22. 17/27 builtin/help: fix leaks in `check_git_cmd()`Patrick Steinhardt, Nov 20, 2024
  23. 18/27 builtin/init-db: fix leaking directory pathsPatrick Steinhardt, Nov 20, 2024
  24. 20/27 t/helper: fix leaking commit graph in "read-graph" subcommandPatrick Steinhardt, Nov 20, 2024
  25. 19/27 builtin/branch: fix leaking sorting optionsPatrick Steinhardt, Nov 20, 2024
  26. 21/27 global: drop `UNLEAK()` annotationPatrick Steinhardt, Nov 20, 2024
  27. 22/27 git-compat-util: drop now-unused `UNLEAK()` macroPatrick Steinhardt, Nov 20, 2024
  28. 23/27 t5601: work around leak sanitizer issuePatrick Steinhardt, Nov 20, 2024
  29. 24/27 t: mark some tests as leak freePatrick Steinhardt, Nov 20, 2024
  30. 25/27 t: remove unneeded !SANITIZE_LEAK prerequisitesPatrick Steinhardt, Nov 20, 2024
  31. 26/27 test-lib: unconditionally enable leak checkingPatrick Steinhardt, Nov 20, 2024
  32. 27/27 t: remove TEST_PASSES_SANITIZE_LEAK annotationsPatrick Steinhardt, Nov 20, 2024
  33. Toon ClaesNov 21, 2024

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.