Re: [PATCH] attr: don't confuse prefixes with leading directories

Jeff King [off-list ref] writes:

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.

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.