bisect.c: partially fix bisect_rev_setup() memory leak

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 <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Aug 2, 2022 at 17:33 UTC 57efebb9b96847adcd25bab0d2c21c599b8f1954
1 file changed +13 -9
bisect.c
+13 -9
@@ -648,11 +648,11 @@ static struct commit_list *managed_skipped(struct commit_list *list,
648 }
649
650 static void bisect_rev_setup(struct repository *r, struct rev_info *revs,
651 + struct strvec *rev_argv,
652 const char *prefix,
653 const char *bad_format, const char *good_format,
654 int read_paths)
655 {
655 - struct strvec rev_argv = STRVEC_INIT;
656 int i;
657
658 repo_init_revisions(r, revs, prefix);
@@ -660,16 +660,16 @@ static void bisect_rev_setup(struct repository *r, struct rev_info *revs,
660 revs->commit_format = CMIT_FMT_UNSPECIFIED;
661
662 /* rev_argv.argv[0] will be ignored by setup_revisions */
663 - strvec_push(&rev_argv, "bisect_rev_setup");
664 - strvec_pushf(&rev_argv, bad_format, oid_to_hex(current_bad_oid));
663 + strvec_push(rev_argv, "bisect_rev_setup");
664 + strvec_pushf(rev_argv, bad_format, oid_to_hex(current_bad_oid));
665 for (i = 0; i < good_revs.nr; i++)
666 - strvec_pushf(&rev_argv, good_format,
666 + strvec_pushf(rev_argv, good_format,
667 oid_to_hex(good_revs.oid + i));
668 - strvec_push(&rev_argv, "--");
668 + strvec_push(rev_argv, "--");
669 if (read_paths)
670 - read_bisect_paths(&rev_argv);
670 + read_bisect_paths(rev_argv);
671
672 - setup_revisions(rev_argv.nr, rev_argv.v, revs, NULL);
672 + setup_revisions(rev_argv->nr, rev_argv->v, revs, NULL);
673 /* XXX leak rev_argv, as "revs" may still be pointing to it */
674 }
675
@@ -873,10 +873,11 @@ static enum bisect_error check_merge_bases(int rev_nr, struct commit **rev, int
873 static int check_ancestors(struct repository *r, int rev_nr,
874 struct commit **rev, const char *prefix)
875 {
876 + struct strvec rev_argv = STRVEC_INIT;
877 struct rev_info revs;
878 int res;
879
879 - bisect_rev_setup(r, &revs, prefix, "^%s", "%s", 0);
880 + bisect_rev_setup(r, &revs, &rev_argv, prefix, "^%s", "%s", 0);
881
882 bisect_common(&revs);
883 res = (revs.commits != NULL);
@@ -885,6 +886,7 @@ static int check_ancestors(struct repository *r, int rev_nr,
886 clear_commit_marks_many(rev_nr, rev, ALL_REV_FLAGS);
887
888 release_revisions(&revs);
889 + strvec_clear(&rev_argv);
890 return res;
891 }
892
@@ -1010,6 +1012,7 @@ void read_bisect_terms(const char **read_bad, const char **read_good)
1012 */
1013 enum bisect_error bisect_next_all(struct repository *r, const char *prefix)
1014 {
1015 + struct strvec rev_argv = STRVEC_INIT;
1016 struct rev_info revs = REV_INFO_INIT;
1017 struct commit_list *tried;
1018 int reaches = 0, all = 0, nr, steps;
@@ -1037,7 +1040,7 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix)
1040 if (res)
1041 goto cleanup;
1042
1040 - bisect_rev_setup(r, &revs, prefix, "%s", "^%s", 1);
1043 + bisect_rev_setup(r, &revs, &rev_argv, prefix, "%s", "^%s", 1);
1044
1045 revs.first_parent_only = !!(bisect_flags & FIND_BISECTION_FIRST_PARENT_ONLY);
1046 revs.limited = 1;
@@ -1112,6 +1115,7 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix)
1115 res = bisect_checkout(bisect_rev, no_checkout);
1116 cleanup:
1117 release_revisions(&revs);
1118 + strvec_clear(&rev_argv);
1119 return res;
1120 }
1121