Re: [PATCH 2/2] fsck: detect and warn a commit with embedded NUL
- From
Jeff King <peff@peff.net>
- Date
- Apr 14, 2016, 18:29 UTC
- Message-ID
- <20160414182900.GD22068@sigill.intra.peff.net>
- In-Reply-To
- <xmqq1t68gi92.fsf@gitster.mtv.corp.google.com>
On Thu, Apr 14, 2016 at 11:25:29AM -0700, Junio C Hamano wrote:
Show 8 quoted lines
> > 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