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

From: Jeff King <[email-masked]>
Date: Mon, 8 Dec 2014 06:22:40 -0500

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

> > 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