Hi,
On Mon, 30 Mar 2009, Christian Couder wrote:
> Le jeudi 26 mars 2009, Junio C Hamano a écrit :
> > Christian Couder <chriscool@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
> > > ---
> > > 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?
>
> I think so.
I suspect something much worse: you are trying to build a list of sha1s of
commits that need to be skipped, later to look up every commit via
binary search.
But it has been proven a lot of times that using a hash set is superior to
that approach, and even better: we already have the framework in place in
the form of struct decorate.
Ciao,
Dscho "who is too busy to review patches ATM"
