Re: [[PATCH v2] 4/4] rebase: avoid computing unnecessary patch IDs

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

 



Hi Junio,

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.

Do you want a note in the commit message about this "abuse" of a negative
return value, or a code comment?

> The idea of using the two level hash, computing the more expensive
> one only when the hashmap hashes of the result of the cheaper hash
> function collide, is excellent.

Thanks :-)

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



[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]