Re: [PATCH 1/2] t6101: add a test for rev-parse $garbage^@

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

 



On Wed, May 23, 2018 at 01:46:12PM -0700, Elijah Newren wrote:

> diff --git a/t/t6101-rev-parse-parents.sh b/t/t6101-rev-parse-parents.sh
> index 8c617981a3..7b1b2dbdf2 100755
> --- a/t/t6101-rev-parse-parents.sh
> +++ b/t/t6101-rev-parse-parents.sh
> @@ -214,4 +214,8 @@ test_expect_success 'rev-list merge^-1x (garbage after ^-1)' '
>  	test_must_fail git rev-list merge^-1x
>  '
>  
> +test_expect_failure 'rev-parse $garbage^@ should not segfault' '
> +	git rev-parse ffffffffffffffffffffffffffffffffffffffff^@
> +'

Two small nits. :)

It may just be me, but for a trivial test+fix like this, I'd rather see
them in the same commit (both for reviewing, and when I'm digging in the
history later).

The second nit is that we may want to use something a little more
symbolic and easier to read here. Thirty-nine f's behaves quite
differently than forty. And eventually we'd like to move away from
having hard-coded commit ids anyway (this is obviously a fake one, but
the length may end up changing).

Perhaps "git rev-parse $EMPTY_TREE^@", which triggers the same bug?

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

  Powered by Linux