Re: [PATCH 4/5] tree-walk: unroll get_mode since loop boundaries are well-known

[Junio C Hamano <gitster@pobox.com>]

Dan McGee <dpmcgee@gmail.com> writes:

> We know our mode entry in our tree objects should be 5 or 6 characters
> long. This change both enforces this fact...

I find the implementation later shown in the thread is cleaner, but I'd
comment on the word "enforces" here.

It is more like "versions of git we know how to read from writes mode bits
with 5 or 6 characters---if we see something else, either the tree object
is corrupt, or we are too old to read the new type of entry in the tree
object".

So returning NULL is fine and it tells the caller that we do not
understand the tree object.  The caller says "corrupt tree file" when we
do so here, but this change needs to rephrase it.  If we stopped because
we saw something other than ' ' after the run of octal digits, then we
know the tree is corrupt.  If we saw a three octal digits 644 in the mode
field, terminated with ' ', maybe we are seeing a new kind of tree entry
generated from later versions of git.

Ideally, I'd rather see error checking done even higher layer than
decode_tree_entry() for the "we are too old" case, though.  We should
return mode 0644 in such a case, and let the caller suggest that the
version of git we are running might be too old, or the tree may have been
written by a broken system that tried to mimic git in its error message.