Re: [PATCH 2/2] fsck: detect and warn a commit with embedded NUL

[Jeff King <peff@peff.net>]

On Thu, Apr 14, 2016 at 11:25:29AM -0700, Junio C Hamano wrote:

> > You need this "buffer_begin" because we move the "buffer" pointer
> > forward as we parse. But perhaps whole-buffer checks should simply go at
> > the top (next to verify_headers) before we start advancing the pointer.
> > To me, that makes the function's flow more natural.
> 
> That was my second iteration.  I didn't want the function return
> with warning without checking more serious errors that may be in the
> object.

Ah, I didn't consider that. In general I'm not sure the distinction
between "warning" and "error" is all that important, or has been applied
all that consistently. transfer.fsckObjects will barf on either.

But I do agree in general that we should be checking as many things as
we can. And that we are already wrong to return an error immediately
when verify_headers() complains. We should be accumulating problems in
an error code and progressing as far as possible. I think fsck_tree() is
a good example of how to do this.

-Peff