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

From: Jeff King <peff@peff.net>
Date: Aug 29, 2014, 23:05 UTC
Message-ID: <20140829230546.GC24834@peff.net>
In-Reply-To: <alpine.DEB.1.00.1408281646400.990@s15462909.onlinehome-server.info>

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

Show 16 quoted lines

> 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