Re: [PATCH] merge_blobs: use strbuf instead of manually-sized mmfile_t

On Mon, Feb 15, 2016 at 10:39:39PM +0100, Stefan Frühwirth wrote:

quoted

in one specific circumstance, git-merge-tree exits with a segfault caused by
"*** Error in `git': malloc(): memory corruption (fast)":

There is a test case[1] kindly provided by chrisrossi, which he crafted
after I discovered the problem[2] in the context of Pylons/acidfs.

-- >8 --
Subject: merge_blobs: use strbuf instead of manually-sized mmfile_t

[...]
It does so by calling xdiff with XDIFF_EMIT_COMMON, and
stores the result in a buffer that is as big as the smaller
of "ours" and "theirs".

In theory, this is right; we cannot have more common content
than is in the smaller of the two blobs. But in practice,
xdiff may give us more: if neither file ends in a newline,
we get the "\nNo newline at end of file" marker.
[...]

Reported-by: Stefan Frühwirth <redacted>
Signed-off-by: Jeff King <redacted>
---

diff --git a/merge-blobs.c b/merge-blobs.c

@@ -51,19 +51,16 @@ static void *three_way_filemerge(const char *path, mmfile_t *base, mmfile_t *our
 static int common_outf(void *priv_, mmbuffer_t *mb, int nbuf)
 {
 	int i;
-	mmfile_t *dst = priv_;
+	struct strbuf *dst = priv_;
 
-	for (i = 0; i < nbuf; i++) {
-		memcpy(dst->ptr + dst->size, mb[i].ptr, mb[i].size);
-		dst->size += mb[i].size;
-	}
+	for (i = 0; i < nbuf; i++)
+		strbuf_add(dst, mb[i].ptr, mb[i].size);
 	return 0;
 }
 
 static int generate_common_file(mmfile_t *res, mmfile_t *f1, mmfile_t *f2)
 {
-	unsigned long size = f1->size < f2->size ? f1->size : f2->size;
-	void *ptr = xmalloc(size);
+	struct strbuf out = STRBUF_INIT;
 	xpparam_t xpp;
 	xdemitconf_t xecfg;
 	xdemitcb_t ecb;

@@ -75,11 +72,15 @@ static int generate_common_file(mmfile_t *res, mmfile_t *f1, mmfile_t *f2)
 	xecfg.flags = XDL_EMIT_COMMON;
 	ecb.outf = common_outf;
 
-	res->ptr = ptr;
-	res->size = 0;
+	ecb.priv = &out;
+	if (xdi_diff(f1, f2, &xpp, &xecfg, &ecb) < 0) {
+		strbuf_release(&out);
+		return -1;
+	}
 
-	ecb.priv = res;
-	return xdi_diff(f1, f2, &xpp, &xecfg, &ecb);
+	res->size = out.len; /* avoid long/size_t pointer mismatch below */