[v3,5/6] bisect.c: partially fix bisect_rev_setup() memory leak

Commit Message

Ævar Arnfjörð Bjarmason Aug. 2, 2022, 3:33 p.m. UTC

Partially fix the memory leak noted in in 8a534b61241 (bisect: use
argv_array API, 2011-09-13), which added the "XXX" comment seen in the
context. We can partially fix it by having the bisect_rev_setup()
function take a "struct strvec", rather than constructing it.

As the comment notes we need to keep the construct "rev_argv" around
while the "struct rev_info" is around, which as seen in the newly
added "strvec_clear()" calls here we do after "release_revisions()".

This "partially" fixes the memory leak because we're leaking the "--"
added to the "rev_argv" here still, which will be addressed in a
subsequent commit.

Signed-off-by: Ævar Arnfjörð Bjarmason <[e-mail]>
---
 bisect.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

Patch

diff --git a/bisect.c b/bisect.c
index 421470bfa59..6afb98be7a1 100644
--- a/bisect.c
+++ b/bisect.c
@@ -648,11 +648,11 @@  static struct commit_list *managed_skipped(struct commit_list *list,
 }
 
 static void bisect_rev_setup(struct repository *r, struct rev_info *revs,
+			     struct strvec *rev_argv,
 			     const char *prefix,
 			     const char *bad_format, const char *good_format,
 			     int read_paths)
 {
-	struct strvec rev_argv = STRVEC_INIT;
 	int i;
 
 	repo_init_revisions(r, revs, prefix);
@@ -660,16 +660,16 @@  static void bisect_rev_setup(struct repository *r, struct rev_info *revs,
 	revs->commit_format = CMIT_FMT_UNSPECIFIED;
 
 	/* rev_argv.argv[0] will be ignored by setup_revisions */
-	strvec_push(&rev_argv, "bisect_rev_setup");
-	strvec_pushf(&rev_argv, bad_format, oid_to_hex(current_bad_oid));
+	strvec_push(rev_argv, "bisect_rev_setup");
+	strvec_pushf(rev_argv, bad_format, oid_to_hex(current_bad_oid));
 	for (i = 0; i < good_revs.nr; i++)
-		strvec_pushf(&rev_argv, good_format,
+		strvec_pushf(rev_argv, good_format,
 			     oid_to_hex(good_revs.oid + i));
-	strvec_push(&rev_argv, "--");
+	strvec_push(rev_argv, "--");
 	if (read_paths)
-		read_bisect_paths(&rev_argv);
+		read_bisect_paths(rev_argv);
 
-	setup_revisions(rev_argv.nr, rev_argv.v, revs, NULL);
+	setup_revisions(rev_argv->nr, rev_argv->v, revs, NULL);
 	/* XXX leak rev_argv, as "revs" may still be pointing to it */
 }
 
@@ -873,10 +873,11 @@  static enum bisect_error check_merge_bases(int rev_nr, struct commit **rev, int
 static int check_ancestors(struct repository *r, int rev_nr,
 			   struct commit **rev, const char *prefix)
 {
+	struct strvec rev_argv = STRVEC_INIT;
 	struct rev_info revs;
 	int res;
 
-	bisect_rev_setup(r, &revs, prefix, "^%s", "%s", 0);
+	bisect_rev_setup(r, &revs, &rev_argv, prefix, "^%s", "%s", 0);
 
 	bisect_common(&revs);
 	res = (revs.commits != NULL);
@@ -885,6 +886,7 @@  static int check_ancestors(struct repository *r, int rev_nr,
 	clear_commit_marks_many(rev_nr, rev, ALL_REV_FLAGS);
 
 	release_revisions(&revs);
+	strvec_clear(&rev_argv);
 	return res;
 }
 
@@ -1010,6 +1012,7 @@  void read_bisect_terms(const char **read_bad, const char **read_good)
  */
 enum bisect_error bisect_next_all(struct repository *r, const char *prefix)
 {
+	struct strvec rev_argv = STRVEC_INIT;
 	struct rev_info revs = REV_INFO_INIT;
 	struct commit_list *tried;
 	int reaches = 0, all = 0, nr, steps;
@@ -1037,7 +1040,7 @@  enum bisect_error bisect_next_all(struct repository *r, const char *prefix)
 	if (res)
 		goto cleanup;
 
-	bisect_rev_setup(r, &revs, prefix, "%s", "^%s", 1);
+	bisect_rev_setup(r, &revs, &rev_argv, prefix, "%s", "^%s", 1);
 
 	revs.first_parent_only = !!(bisect_flags & FIND_BISECTION_FIRST_PARENT_ONLY);
 	revs.limited = 1;
@@ -1112,6 +1115,7 @@  enum bisect_error bisect_next_all(struct repository *r, const char *prefix)
 	res = bisect_checkout(bisect_rev, no_checkout);
 cleanup:
 	release_revisions(&revs);
+	strvec_clear(&rev_argv);
 	return res;
 }