Karthik Nayak <karthik.188@xxxxxxxxx> writes: > @@ -1176,7 +1181,11 @@ static int process_parents(struct rev_info *revs,... > break; > continue; > } > - return -1; > + > + if (!revs->do_not_die_on_missing_objects) > + return -1; > + else > + oidset_insert(&revs->missing_objects, &p->object.oid); I would suspect that swapping if/else would make it easier to follow. Everybody else in the patch guards the use of the oidset with "were we told not to die on missing objects?", i.e., if (revs->do_not_die_on_missing_objects) oidset_insert(&revs->missing_objects, &p->object.oid); else return -1; /* corrupt repository */ > @@ -3800,6 +3809,9 @@ int prepare_revision_walk(struct rev_info *revs) > FOR_EACH_OBJECT_PROMISOR_ONLY); > } > > + if (revs->do_not_die_on_missing_objects) > + oidset_init(&revs->missing_objects, 0); I read the patch to make sure that .missing_objects oidset is used only when .do_not_die_on_missing_objects is set and the oidset is untouched unless it is initialized. Well done. I know I floated "perhaps oidset can replace the object bits, especially because the number of objects that need marking is expected to be small", but I am curious what the performance implication of this would be. Is this something we can create comparison easily? I noticed that nobody releases the resource held by this new oidset. Shouldn't we do so in revision.c:release_revisions()? Thanks.
