Christian Couder [off-list ref] writes:
This factorises some code and make a big function smaller.
I think the refactoring itself makes sense, especially where it
simplifies the clean-up of weight array in early-return
codepath. But I have a couple of comments, though.
+static struct commit_list *do_find_bisection(struct commit_list *list,
+ int nr, int *weights);
+
/*
* zero or positive weight is the number of interesting commits it can
* reach, including itself. Especially, weight = 0 means it does not
The comment whose top part we can see here talks about the magic
values -1 and -2 used while do_find_bisection() after the
refactoring does its work, and these magic values are never
visible to the calling function. You should move the comment to
the top of do_find_bisection() as well.
Also this forward declaration is unwarranted. A bottom-up
sequence to define do_find_bisection() first, then to define its
sole caller find_bisection() next is easier to read at least for
me.
The latter comment also applies to your other patch.