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

Dan McGee <dpmcgee@gmail.com> writes:

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.