[PATCH v2 0/2] corner cases with "rev-list --use-bitmap-index -n"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jun 02, 2016 at 07:10:31PM -0400, Jeff King wrote:

> diff --git a/builtin/rev-list.c b/builtin/rev-list.c
> index 275da0d..aaa79a3 100644
> --- a/builtin/rev-list.c
> +++ b/builtin/rev-list.c
> @@ -360,6 +360,8 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
>  			uint32_t commit_count;
>  			if (!prepare_bitmap_walk(&revs)) {
>  				count_bitmap_commit_list(&commit_count, NULL, NULL, NULL);
> +				if (revs.max_count && revs.max_count < commit_count)
> +					commit_count = revs.max_count;

...aaaaand this patch is totally wrong. For one thing, the "not set"
sentinel value for max_count is "-1", not "0", so it didn't kick in for
"-n0".

And for another, any fallback traversal will actually decrement
revs.max_count, so we have to save the value before traversing.

And as luck would have it, those two bugs cancel each other out in many
cases (including the ones in the test suite!). If you pass "-n1", gets
decremented to "0".  That would give us a bogus adjustment, but instead
we decide that "0" means no adjustement is necessary, and return the
number of commits we traversed, which is the right answer as long as we
didn't hit any bitmaps on the way down (or we hit one right away, in
which case we didn't decrement at all).

Updated patches in a moment (the second has the same sentinel problem,
too).

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]