Re: [PATCH 06/18] t5613: clarify "too deep" recursion tests

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

 



On Tue, Oct 04, 2016 at 01:44:23PM -0700, Jacob Keller wrote:

> On Tue, Oct 4, 2016 at 6:48 AM, Jeff King <peff@xxxxxxxx> wrote:
> > On Mon, Oct 03, 2016 at 10:57:48PM -0700, Jacob Keller wrote:
> >
> >> > diff --git a/t/t5613-info-alternate.sh b/t/t5613-info-alternate.sh
> >> > index 7bc1c3c..b393613 100755
> >> > --- a/t/t5613-info-alternate.sh
> >> > +++ b/t/t5613-info-alternate.sh
> >> > @@ -39,6 +39,18 @@ test_expect_success 'preparing third repository' '
> >> >         )
> >> >  '
> >> >
> >> > +# Note: These tests depend on the hard-coded value of 5 as "too deep". We start
> >> > +# the depth at 0 and count links, not repositories, so in a chain like:
> >> > +#
> >> > +#   A -> B -> C -> D -> E -> F -> G -> H
> >> > +#      0    1    2    3    4    5    6
> >> > +#
> >>
> >> Ok so we count links, but wouldn't we have 5 links when we hit F, and
> >> not G? Or am I missing something here?
> >
> > This is what I was trying to get at with the "start the depth at 0". We
> > disallow a depth greater than 5, but because we start at 0-counting,
> > it's really six links. I guess saying "5 as too deep" is really the
> > misleading part. It should be "5 as the maximum depth".
> >
> > -Peff
> 
> Right, but if A is 0, then:
> 
> B = 1
> C = 2
> D = 3
> E = 4
> F = 5
> G = 6  (UhOh??)
> H = 7
> 
> So do you mean that *B* = 0, and C = 1??? That is not clear from this commment.

No, we count links, not repositories. So the "A->B" link is "0", "B->C"
is "1", and so on.

> So either way it still feels like "6" links is what is allowed? Or the
> first link has to not count? That's really confusing.

Right, 6 links _are_ allowed. Because we count links, and because we
start the link-counting at "0" and allow through "5". The link labeled
"6" (which is really the seventh link!) is the one that is forbidden.

> Basically I G is the 7th letter, not the 6th, so even if we're
> subtractnig 1 it's still 6 which is 1 too deep? That means we not only
> discard 0 (the first repository) but we discount the 2nd one as well?

It's basically two off-by-ones from what you might think is correct.  I
agree it's unintuitive, but I'm just documenting what's there. We could
change it; it's not like anybody cares about the exact value except
"deep enough", but _since_ nobody cares, I preferred not to modify the
code.

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