Re: [PATCH v2 6/9] bundle: avoid using the rev_info flag leak_pending

René Scharfe [off-list ref] writes:

The leak_pending flag is so awkward to use that multiple comments had to
be added around each occurrence.  We use it for remembering the
prerequisites for the bundle.  That is easy, though: We have the
ref_list named "prerequisites" in the header for just that purpose.

Hmph.

 int verify_bundle(struct bundle_header *header, int verbose)
 {
 ...
 	struct rev_info revs;
 	const char *argv[] = {NULL, "--all", NULL};
-	struct object_array refs;
 	struct commit *commit;
 	int i, ret = 0, req_nr;
 	const char *message = _("Repository lacks these prerequisite commits:");
 
 	init_revisions(&revs, NULL);
 	for (i = 0; i < p->nr; i++) {
 		struct ref_list_entry *e = p->list + i;
 		struct object *o = parse_object(&e->oid);
 		if (o) {
 			o->flags |= PREREQ_MARK;
 			add_pending_object(&revs, o, e->name);
 			continue;
 		}

We mark the prereq objects with PREREQ_MARK and then ...

 		if (++ret == 1)
 			error("%s", message);
 		error("%s %s", oid_to_hex(&e->oid), e->name);
 	}
 	if (revs.pending.nr != p->nr)
 		return ret;
 	req_nr = revs.pending.nr;
 	setup_revisions(2, argv, &revs, NULL);

... run "rev-list" with "--all" plus these prereq objects, and ...

...
 	i = req_nr;
 	while (i && (commit = get_revision(&revs)))
 		if (commit->object.flags & PREREQ_MARK)
 			i--;

... let the traversal run until we see all the objects with PREREQ_MARK
set (i.e. those that appear in p->list[]).

...
+	for (i = 0; i < p->nr; i++) {
+		struct ref_list_entry *e = p->list + i;
+		struct object *o = parse_object(&e->oid);
+		assert(o); /* otherwise we'd have returned early */
+		if (o->flags & SHOWN)
+			continue;
+		if (++ret == 1)
+			error("%s", message);
+		error("%s %s", oid_to_hex(&e->oid), e->name);
+	}

And then make sure that all of those that appear in p->list[] are
already shown.

Doesn't that mean that these SHOWN and other flags are first given
to the commits at the tip of "--all" refs and propagated down to
those in p->list[]?  Would it be sufficient to clear those that can
be reached from them, like the following?

 
 	/* Clean up objects used, as they will be reused. */
-	clear_commit_marks_for_object_array(&refs, ALL_REV_FLAGS);
-
-	object_array_clear(&refs);
+	for (i = 0; i < p->nr; i++) {
+		struct ref_list_entry *e = p->list + i;
+		commit = lookup_commit_reference_gently(&e->oid, 1);
+		if (commit)
+			clear_commit_marks(commit, ALL_REV_FLAGS);
+	}

I do not think the patch in question changes behaviour.  The set of
objects the code starts the clearing does not change.  So I am more
puzzled by the original than the conversion done with this change.