Re: [PATCH] fsck: properly bound "invalid tag name" error message

From: Jeff King <peff@peff.net>
Date: Dec 8, 2014, 11:22 UTC
Message-ID: <20141208112240.GA3191@peff.net>
In-Reply-To: <alpine.DEB.1.00.1412081201180.13845@s15462909.onlinehome-server.info>

On Mon, Dec 08, 2014 at 12:17:10PM +0100, Johannes Schindelin wrote:

Show 10 quoted lines

> > I do admit that I am tempted to teach index-pack to always NUL-terminate
> > objects in memory that we feed to fsck, just to be on the safe side. It
> > doesn't cost much, and could prevent a silly mistake (either in the
> > future, or one that I missed in my analysis). The fsck code otherwise
> > generally expects to get the output of read_sha1_file, which has the
> > safety-NUL appended.
> 
> If we do that, we have to NUL-terminate all of the objects, correct? I
> mean, even the blobs and the trees and stuff, because we cannot know
> beforehand what type of object we're gonna read, right?

I think that is right. It should be a pretty simple change. It looks like we already use xmallocz when creating deltas, and we just need to handle regular objects. I think it could be as simple as this one-liner, but I didn't test anything:

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index a369f55..390845d 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -447,7 +447,7 @@ static void *unpack_entry_data(unsigned long offset, unsigned long size,
 	if (type == OBJ_BLOB && size > big_file_threshold)
 		buf = fixed_buf;
 	else
-		buf = xmalloc(size);
+		buf = xmallocz(size);
 
 	memset(&stream, 0, sizeof(stream));
 	git_inflate_init(&stream);

-Peff