From: Junio C Hamano <gitster@pobox.com>
To: Jeremiah Mahler <jmmahler@gmail.com>
Cc: Jonathan Nieder <jrnieder@gmail.com>,
Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH v4] cleanup duplicate name_compare() functions
Date: Thu, 19 Jun 2014 11:29:21 -0700 [thread overview]
Message-ID: <CAPc5daW5hsKeJsG8+2nxFzFL9gMnLR2gMBXG5__dhSCLKkBWmA@mail.gmail.com> (raw)
In-Reply-To: <xmqqionwhj54.fsf@gitster.dls.corp.google.com>
On Thu, Jun 19, 2014 at 11:03 AM, Junio C Hamano <gitster@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
"magnitude".
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?
next prev parent reply other threads:[~2014-06-19 18:29 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-06-19 8:07 [PATCH v4] cleanup duplicate name_compare() functions Jeremiah Mahler
2014-06-19 8:07 ` Jeremiah Mahler
2014-06-19 18:03 ` Junio C Hamano
2014-06-19 18:29 ` Junio C Hamano [this message]
2014-06-20 1:55 ` Jeremiah Mahler
2014-06-19 20:45 ` Jeremiah Mahler
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAPc5daW5hsKeJsG8+2nxFzFL9gMnLR2gMBXG5__dhSCLKkBWmA@mail.gmail.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=jmmahler@gmail.com \
--cc=jrnieder@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.