Re: [PATCH 07/10] rev-list: call new "filter_skip" function

[Junio C Hamano <gitster@pobox.com>]

Christian Couder <chriscool@tuxfamily.org> writes:

> This patch implements a new "filter_skip" function in C in
> "bisect.c" that will later replace the existing implementation in
> shell in "git-bisect.sh".
>
> An array is used to store the skipped commits. But the array is
> not yet fed anything.
>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
>  bisect.c           |   65 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  bisect.h           |    6 ++++-
>  builtin-rev-list.c |   30 ++++++++++++++++++++----
>  3 files changed, 95 insertions(+), 6 deletions(-)
>
> diff --git a/bisect.c b/bisect.c
> index 27def7d..39189f2 100644
> --- a/bisect.c
> +++ b/bisect.c
> @@ -4,6 +4,11 @@
>  #include "revision.h"
>  #include "bisect.h"
>  
> +
> +static unsigned char (*skipped_sha1)[20];
> +static int skipped_sha1_nr;
> +static int skipped_sha1_alloc;
> +
>  /* bits #0-15 in revision.h */
>  
>  #define COUNTED		(1u<<16)
> @@ -386,3 +391,63 @@ struct commit_list *find_bisection(struct commit_list *list,
>  	return best;
>  }
>  
> +static int skipcmp(const void *a, const void *b)
> +{
> +	return hashcmp(a, b);
> +}

I've learned to suspect without reading a qsort() callback that does not
derefence its arguments.  Is this doing the right thing?

> +
> +static void prepare_skipped(void)
> +{
> +	qsort(skipped_sha1, skipped_sha1_nr, sizeof(*skipped_sha1), skipcmp);
> +}
> +
> +static int lookup_skipped(unsigned char *sha1)
> +{
> +	int lo, hi;
> +	lo = 0;
> +	hi = skipped_sha1_nr;
> +	while (lo < hi) {
> +		int mi = (lo + hi) / 2;
> +		int cmp = hashcmp(sha1, skipped_sha1[mi]);
> +		if (!cmp)
> +			return mi;
> +		if (cmp < 0)
> +			hi = mi;
> +		else
> +			lo = mi + 1;
> +	}
> +	return -lo - 1;
> +}