Re: [PATCH] diff: handle --no-abbrev outside of repository

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

 



On Mon, Nov 28, 2016 at 11:25:08AM -0700, Jack Bates wrote:

> diff --git a/diff.c b/diff.c
> index ec87283..0447eff 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -3106,7 +3106,8 @@ static const char *diff_abbrev_oid(const struct object_id *oid, int abbrev)
>  			abbrev = FALLBACK_DEFAULT_ABBREV;
>  		if (abbrev > GIT_SHA1_HEXSZ)
>  			die("BUG: oid abbreviation out of range: %d", abbrev);
> -		hex[abbrev] = '\0';
> +		if (abbrev)
> +			hex[abbrev] = '\0';
>  		return hex;
>  	}

This hunk made me wonder if there is a regression in v2.11, as this
fallback code is new in that version. But I don't think so.

This new code doesn't handle abbrev==0 the same as find_unique_abbrev()
does, and that's clearly a bug. But I couldn't find any way to trigger
it with the existing code.

The obvious way is with --no-abbrev, but that doesn't work without yet
without your patch.

A less obvious way is --abbrev=0, but that gets munged internally to
MINIMUM_ABBREV.

The most obscure is "git -c core.abbrev=0", but that barfs completely on
a too-small abbreviation (without even giving a good error message,
which is something we might want to fix).

So I think there is no regression, and we only have to worry about it as
part of the feature you are adding here (it might be worth calling out
the bug fix specifically in the commit message, though, or even putting
it in its own patch).

-Peff



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