Hi Junio, On Mon, 1 Aug 2016, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes: > > > On Fri, 29 Jul 2016, Junio C Hamano wrote: > > > >> Kevin Willford <kcwillford@xxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html