Subject: Re: [PATCH 2/6] Accept object data in the fsck_object() function

From: Jeff King <[email-masked]>
Date: Fri, 29 Aug 2014 19:05:46 -0400

On Thu, Aug 28, 2014 at 04:46:42PM +0200, Johannes Schindelin wrote:

> When fsck'ing an incoming pack, we need to fsck objects that cannot be
> read via read_sha1_file() because they are not local yet (and might even
> be rejected if transfer.fsckobjects is set to 'true').
> 
> For commits, there is a hack in place: we basically cache commit
> objects' buffers anyway, but the same is not true, say, for tag objects.
> 
> By refactoring fsck_object() to take the object buffer and size as
> optional arguments -- optional, because we still fall back to the
> previous method to look at the cached commit objects if the caller
> passes NULL -- we prepare the machinery for the upcoming handling of tag
> objects.
> 
> The assumption that such buffers are inherently NUL terminated is now
> wrong, of course, hence we pass the size of the buffer so that we can
> add a sanity check later, to prevent running past the end of the buffer.

After this, can we get rid of passing a "struct object" to the fsck code
entirely? It should be able to operate only on a type/buffer pair, along
with possibly a sha1 for error reporting.

The buffer management in index-pack's sha1_object function is
significantly complicated by attaching the temporary buffer to a "struct
commit", only to then have to detach it a few lines later. It is the
sole reason we had to add "detach_commit_buffer" a while ago.

-Peff