Re: Redefine semantics of find_unique_abbrev()
- From
Jeff King <peff@peff.net>
- Date
- Mar 1, 2008, 05:06 UTC
- Message-ID
- <20080301050641.GB8969@coredump.intra.peff.net>
- In-Reply-To
- <7vpruf9q5a.fsf@gitster.siamese.dyndns.org>
On Fri, Feb 29, 2008 at 05:41:53PM -0800, Junio C Hamano wrote:
Show 9 quoted lines
> The function returned NULL when no object that matches the name > was found, but that made the callers more complicated, as nobody > used that NULL return as an indication that no object with such > a name exists. They (at least the careful ones) instead took > the full 40-hexdigit and used in such a case, and the careless > ones segfaulted. > > With this "git rev-parse --short 5555555555555555555555555555555555555555" > would stop segfaulting.
I have been meaning to clean up and submit a similar patch from the 1.5.4 freeze period. However, your patch will always print the 40-hexdigit version, which looks quite ugly in status output. Instead, we can do much better by finding the longest subsequence we _do_ know about, and adding one digit to it.
So you get:
# this is the current master, abbreviated $ git rev-parse --short 97b97c58e609a1cd23b3c2514f41cdb0405870ee 97b97c5
# this is an object we don't have, but similar to that $ git rev-parse --short 97b97c58e609a1cd23b3c2514f41cdb0405870ff 97b97c58e609a1cd23b3c2514f41cdb0405870f
# and less similar means we can abbreviate more $ git rev-parse --short 97b97c58e609a1cd23b3c2514fffffffffffffff 97b97c58e609a1cd23b3c2514ff
Implementation is below:
Show 20 quoted lines
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -202,16 +202,14 @@ const char *find_unique_abbrev(const unsigned char *sha1, int len)
> while (len < 40) {
> unsigned char sha1_ret[20];
> status = get_short_sha1(hex, len, sha1_ret, 1);
> - if (!status ||
> - (is_null && status != SHORT_NAME_AMBIGUOUS)) {
> + if ((!is_null && !status) ||
> + (is_null && status == SHORT_NAME_NOT_FOUND)) {
> hex[len] = 0;
> return hex;
> }
> - if (status != SHORT_NAME_AMBIGUOUS)
> - return NULL;
> len++;
> }
> - return NULL;
> + return hex;
> }All we have to do is treat the "missing" case like we treat "is_null" (and in fact, is_null is just a specialized case of something we don't have). So replace this hunk with:
diff --git a/sha1_name.c b/sha1_name.c index 05c2e7a..9c71d1b 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -192,18 +192,18 @@ static int get_short_sha1(const char *name, int len, unsigned char *sha1, const char *find_unique_abbrev(const unsigned char *sha1, int len) { - int status, is_null; + int status, missing; static char hex[41]; - is_null = is_null_sha1(sha1); + missing = !has_sha1_file(sha1); memcpy(hex, sha1_to_hex(sha1), 40); if (len == 40 || !len) return hex; while (len < 40) { unsigned char sha1_ret[20]; status = get_short_sha1(hex, len, sha1_ret, 1); - if ((!is_null && !status) || - (is_null && status == SHORT_NAME_NOT_FOUND)) { + if ((!missing && !status) || + (missing && status == SHORT_NAME_NOT_FOUND)) { hex[len] = 0; return hex; } -Peff