On Thu, Jun 19, 2014 at 11:29:21AM -0700, Junio C Hamano wrote:
> On Thu, Jun 19, 2014 at 11:03 AM, Junio C Hamano <gits...@pobox.com> wrote:
> > You chose to use the one that loses the information by unifying
> > these two into the variant that only returns -1/0/+1. We know that
> > it does not matter for the current callers, but is it expected that
> > no future callers will benefit by having the magnitude information?
> Heh, I was being silly, partly fooled by your reference to
> You are not losing information at all, because the caller cannot
> tell if the return value came from an earlier memcmp(), whose only
> guarantee is that the sign of the returned value is all that
> matters, or from the later subtraction between lengths.
> So unifying to the -1/0/+1 variant is entirely justifiable. It is
> just your rationale was a bit misleading.
> We often represent our strings as a counted string, i.e. a pair of
> the pointer to the beginning of the string and its length, and the
> string may not be NUL terminated to that length.
> To compare a pair of such counted strings, unpack-trees.c and
> read-cache.c implement their own name_compare() functions
> identically. In addition, cache_name_compare() function in
> read-cache.c is nearly identical. The only difference is when one
> string is the prefix of the other string, in which case the former
> returns -1/+1 to show which one is longer and the latter returns the
> difference of the lengths to show the same information.
> Unify these three functions by using the implementation from
> cache_name_compare(). This does not make any difference to the
> existing and future callers, as they must be paying attention only
> to the sign of the returned value (and not the magnitude) because
> the original implementations of these two functions return values
> returned by memcmp(3) when the one string is not a prefix of the
> other string, and the only thing memcmp(3) guarantees its callers is
> the sign of the returned value, not the magnitude.
> or something like that, perhaps?
Yes, that looks good. It is a bit clearer than my message. I like how
you used "the prefix of the other string" to describe when the two
functions behave differently.
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