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

[PATCH v2 5/9] bisect: avoid using the rev_info flag leak_pending

From
René Scharfe <l.s.r@web.de>
Date
Dec 25, 2017, 17:45 UTC
Message-ID
<07d0e0c6-d023-bd28-511b-78092cb35b4f@web.de>
In-Reply-To
<ac4c77a1-6403-ca20-2021-50c99201915a@web.de>

The leak_pending flag is so awkward to use that multiple comments had to be added around each occurrence. We only use it for remembering the commits whose marks we have to clear after checking if all of the good ones are ancestors of the bad one. This is easy, though: We need to do that for the bad and good commits, of course.

Let check_good_are_ancestors_of_bad() create and own the array of bad and good commits, and use it to clear the commit marks as well.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 bisect.c | 30 +++++++++---------------------
 1 file changed, 9 insertions(+), 21 deletions(-)
diff --git a/bisect.c b/bisect.c
index 0fca17c02b..c02accaf3c 100644
--- a/bisect.c
+++ b/bisect.c
@@ -790,100 +790,88 @@ static void handle_skipped_merge_base(const struct object_id *mb)
  * - If one is "skipped", we can't know but we should warn.
  * - If we don't know, we should check it out and ask the user to test.
  */
-static void check_merge_bases(int no_checkout)
+static void check_merge_bases(int rev_nr, struct commit **rev, int no_checkout)
 {
 	struct commit_list *result;
-	int rev_nr;
-	struct commit **rev = get_bad_and_good_commits(&rev_nr);
 
 	result = get_merge_bases_many(rev[0], rev_nr - 1, rev + 1);
 
 	for (; result; result = result->next) {
 		const struct object_id *mb = &result->item->object.oid;
 		if (!oidcmp(mb, current_bad_oid)) {
 			handle_bad_merge_base();
 		} else if (0 <= oid_array_lookup(&good_revs, mb)) {
 			continue;
 		} else if (0 <= oid_array_lookup(&skipped_revs, mb)) {
 			handle_skipped_merge_base(mb);
 		} else {
 			printf(_("Bisecting: a merge base must be tested\n"));
 			exit(bisect_checkout(mb, no_checkout));
 		}
 	}
 
-	free(rev);
 	free_commit_list(result);
 }
 
-static int check_ancestors(const char *prefix)
+static int check_ancestors(int rev_nr, struct commit **rev, const char *prefix)
 {
 	struct rev_info revs;
-	struct object_array pending_copy;
 	int res;
 
 	bisect_rev_setup(&revs, prefix, "^%s", "%s", 0);
 
-	/* Save pending objects, so they can be cleaned up later. */
-	pending_copy = revs.pending;
-	revs.leak_pending = 1;
-
-	/*
-	 * bisect_common calls prepare_revision_walk right away, which
-	 * (together with .leak_pending = 1) makes us the sole owner of
-	 * the list of pending objects.
-	 */
 	bisect_common(&revs);
 	res = (revs.commits != NULL);
 
 	/* Clean up objects used, as they will be reused. */
-	clear_commit_marks_for_object_array(&pending_copy, ALL_REV_FLAGS);
-
-	object_array_clear(&pending_copy);
+	clear_commit_marks_many(rev_nr, rev, ALL_REV_FLAGS);
 
 	return res;
 }
 
 /*
  * "check_good_are_ancestors_of_bad" checks that all "good" revs are
  * ancestor of the "bad" rev.
  *
  * If that's not the case, we need to check the merge bases.
  * If a merge base must be tested by the user, its source code will be
  * checked out to be tested by the user and we will exit.
  */
 static void check_good_are_ancestors_of_bad(const char *prefix, int no_checkout)
 {
 	char *filename = git_pathdup("BISECT_ANCESTORS_OK");
 	struct stat st;
-	int fd;
+	int fd, rev_nr;
+	struct commit **rev;
 
 	if (!current_bad_oid)
 		die(_("a %s revision is needed"), term_bad);
 
 	/* Check if file BISECT_ANCESTORS_OK exists. */
 	if (!stat(filename, &st) && S_ISREG(st.st_mode))
 		goto done;
 
 	/* Bisecting with no good rev is ok. */
 	if (good_revs.nr == 0)
 		goto done;
 
 	/* Check if all good revs are ancestor of the bad rev. */
-	if (check_ancestors(prefix))
-		check_merge_bases(no_checkout);
+	rev = get_bad_and_good_commits(&rev_nr);
+	if (check_ancestors(rev_nr, rev, prefix))
+		check_merge_bases(rev_nr, rev, no_checkout);
+	free(rev);
 
 	/* Create file BISECT_ANCESTORS_OK. */
 	fd = open(filename, O_CREAT | O_TRUNC | O_WRONLY, 0600);
 	if (fd < 0)
 		warning_errno(_("could not create file '%s'"),
 			      filename);
 	else
 		close(fd);
  done:
 	free(filename);
 }
 
 /*
  * This does "git diff-tree --pretty COMMIT" without one fork+exec.
  */
-- 
2.15.1
Previous: Jeff KingNext: Jeff King
Message 20 of 32 in “revision: introduce prepare_revision_walk_extended()”
  1. revision: introduce prepare_revision_walk_extended()René Scharfe, Dec 16, 2017
  2. Martin ÅgrenDec 17, 2017
  3. Jeff KingDec 18, 2017
  4. René ScharfeDec 18, 2017
  5. Jeff KingDec 19, 2017
  6. Junio C HamanoDec 19, 2017
  7. Jeff KingDec 20, 2017
  8. René ScharfeDec 21, 2017
  9. Jeff KingDec 24, 2017
  10. René ScharfeDec 25, 2017
  11. 0/9 revision: get rid of the flag leak_pendingRené Scharfe, Dec 25, 2017
  12. 1/9 commit: avoid allocation in clear_commit_marks_many()René Scharfe, Dec 25, 2017
  13. Jeff KingJan 10, 2018
  14. 2/9 commit: use clear_commit_marks_many() in remove_redundant()René Scharfe, Dec 25, 2017
  15. 3/9 ref-filter: use clear_commit_marks_many() in do_merge_filter()René Scharfe, Dec 25, 2017
  16. 4/9 object: add clear_commit_marks_all()René Scharfe, Dec 25, 2017
  17. Jeff KingJan 10, 2018
  18. René ScharfeJan 11, 2018
  19. Jeff KingJan 12, 2018
  20. 5/9 bisect: avoid using the rev_info flag leak_pendingRené Scharfe, Dec 25, 2017
  21. Jeff KingJan 10, 2018
  22. René ScharfeJan 11, 2018
  23. Jeff KingJan 12, 2018
  24. 6/9 bundle: avoid using the rev_info flag leak_pendingRené Scharfe, Dec 25, 2017
  25. Junio C HamanoDec 28, 2017
  26. Jeff KingJan 10, 2018
  27. 7/9 checkout: avoid using the rev_info flag leak_pendingRené Scharfe, Dec 25, 2017
  28. Junio C HamanoDec 28, 2017
  29. 8/9 revision: remove the unused flag leak_pendingRené Scharfe, Dec 25, 2017
  30. 9/9 commit: remove unused function clear_commit_marks_for_object_array()René Scharfe, Dec 25, 2017
  31. Junio C HamanoDec 28, 2017
  32. Jeff KingJan 10, 2018

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.