Re: [PATCH] attr: don't confuse prefixes with leading directories
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 10, 2012, 19:23 UTC
- Message-ID
- <7vlipf9xbe.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <20120110182140.GB15273@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 11 quoted lines
> IOW, our loop breaks out when attr_stack is NULL, but then we go on to
> assume that attr_stack is _not_ NULL. This isn't a bug, because it turns
> out that we always leave something in the attr_stack: the root
> gitattributes file (and the builtins). But it is slightly confusing to
> a reader because of the useless loop condition.
>
> I'm not sure if the right solution is to change the popping loop to:
>
> /* we will never run out of stack, because we always have the root */
> while (attr_stack->origin) {
> ...Yeah, that makes sense, as that existing check "attr_stack &&" was a misguided defensive coding, that was _not_ defensive at all as we didn't do anything after we stop iterating from that loop and without checking dereferenced attr_stack->origin, which was a simple bogosity.
Show 9 quoted lines
>
> Or to be extra defensive and put:
>
> if (!attr_stack)
> die("BUG: we ran out of attr stack!?");
>
> after the loop, or to somehow handle the case of an empty attr stack
> below (which is hard to do, because it can't be triggered, so I have no
> idea what it would mean).And this is even more so.