On Tue, Jan 10, 2012 at 01:08:21PM -0500, Jeff King wrote:
> diff --git a/attr.c b/attr.c
> index 76b079f..fa975da 100644
> --- a/attr.c
> +++ b/attr.c
> @@ -582,7 +582,8 @@ static void prepare_attr_stack(const char *path)
>
> elem = attr_stack;
> if (namelen <= dirlen &&
> - !strncmp(elem->origin, path, namelen))
> + !strncmp(elem->origin, path, namelen) &&
> + (!namelen || path[namelen] == '/'))
> break;
Side note. One thing that confused me about this code is that
prepare_attr_stack does a popping loop like this:
while (attr_stack && attr_stack->origin) {
if (/* attr_stack->origin is a prefix */)
break;
/* otherwise, pop */
elem = attr_stack;
attr_stack = elem->prev;
free(elem);
}
/* now push our new ones */
...
len = strlen(attr_stack->origin);
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) {
...
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).
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html