Hi Junio,

On Mon, 1 Aug 2016, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schinde...@gmx.de> writes:
> 
> > On Fri, 29 Jul 2016, Junio C Hamano wrote:
> >
> >> Kevin Willford <kcwillf...@gmail.com> writes:
> >> 
> >> >  static int patch_id_cmp(struct patch_id *a,
> >> >                          struct patch_id *b,
> >> > -                        void *keydata)
> >> > +                        struct diff_options *opt)
> >> >  {
> >> > +        if (is_null_sha1(a->patch_id) &&
> >> > +            commit_patch_id(a->commit, opt, a->patch_id, 0))
> >> > +                return error("Could not get patch ID for %s",
> >> > +                        oid_to_hex(&a->commit->object.oid));
> >> > +        if (is_null_sha1(b->patch_id) &&
> >> > +            commit_patch_id(b->commit, opt, b->patch_id, 0))
> >> > +                return error("Could not get patch ID for %s",
> >> > +                        oid_to_hex(&b->commit->object.oid));
> >> >          return hashcmp(a->patch_id, b->patch_id);
> >> >  }
> >> 
> >> These error returns initially looks slightly iffy in that in general
> >> the caller of any_cmp_fn() wants to know how a/b compares, but by
> >> returning error(), it always says "a is smaller than b".
> >
> > I am to blame, as this is my design.
> >
> > And yes, it is kind of funny that we require a cmpfn that returns <0, ==0
> > and >0 for comparisons, when hashmaps try to avoid any order.
> 
> Perhaps hashmap API needs fixing in the longer term not to call this
> type hashmap_cmp_fn; instead it should lose cmp and say something
> like hashmap_eq_fn or something.

Maybe.

But to make sure: you do not expect Kevin to do that in the context of
this here patch series, right?

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

Reply via email to