git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Stefan Frühwirth" <stefan.fruehwirth@uni-graz.at>, git@vger.kernel.org
Subject: [PATCH 2/3] merge-tree: drop generate_common strategy
Date: Tue, 23 Feb 2016 01:06:55 -0500	[thread overview]
Message-ID: <20160223060655.GB3205@sigill.intra.peff.net> (raw)
In-Reply-To: <20160223060338.GA2912@sigill.intra.peff.net>

When merge_blobs sees an add/add conflict, it tries to
create a virtual base object for the 3-way merge that
consists of the common lines of each file. It inherited this
strategy from merge-one-file in 0c79938 (Improved three-way
blob merging code, 2006-06-28), and the point is to minimize
the size of the conflict hunks. That commit talks about "if
libxdiff were to ever grow a compatible three-way merge, it
could probably be directly plugged in".

That has long since happened. So as with merge-one-file in
the previous commit, this extra step is no longer necessary.
Our 3-way merge code is smart enough to do the minimizing
itself if we simply feed it an empty base, which is what the
more modern merge-recursive strategy already does.

Not only does this let us drop some code, but it removes an
overflow bug in generate_common_file(). We allocate a buffer
as large as the smallest of the two blobs, under the
assumption that there cannot be more common content than
what is in the smaller blob. However, xdiff may feed us
more: if neither file ends in a newline, it feeds us the
"\nNo newline at end of file" marker as common content, and
we write it into the output. If the differences between the
files are small than that string, we overflow the output
buffer.  This patch solves it by simply dropping the buggy
code entirely.

Signed-off-by: Jeff King <peff@peff.net>
---
 merge-blobs.c | 38 ++------------------------------------
 1 file changed, 2 insertions(+), 36 deletions(-)

diff --git a/merge-blobs.c b/merge-blobs.c
index ddca601..9b6eac2 100644
--- a/merge-blobs.c
+++ b/merge-blobs.c
@@ -48,40 +48,6 @@ static void *three_way_filemerge(const char *path, mmfile_t *base, mmfile_t *our
 	return res.ptr;
 }
 
-static int common_outf(void *priv_, mmbuffer_t *mb, int nbuf)
-{
-	int i;
-	mmfile_t *dst = priv_;
-
-	for (i = 0; i < nbuf; i++) {
-		memcpy(dst->ptr + dst->size, mb[i].ptr, mb[i].size);
-		dst->size += 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);
-	xpparam_t xpp;
-	xdemitconf_t xecfg;
-	xdemitcb_t ecb;
-
-	memset(&xpp, 0, sizeof(xpp));
-	xpp.flags = 0;
-	memset(&xecfg, 0, sizeof(xecfg));
-	xecfg.ctxlen = 3;
-	xecfg.flags = XDL_EMIT_COMMON;
-	ecb.outf = common_outf;
-
-	res->ptr = ptr;
-	res->size = 0;
-
-	ecb.priv = res;
-	return xdi_diff(f1, f2, &xpp, &xecfg, &ecb);
-}
-
 void *merge_blobs(const char *path, struct blob *base, struct blob *our, struct blob *their, unsigned long *size)
 {
 	void *res = NULL;
@@ -112,8 +78,8 @@ void *merge_blobs(const char *path, struct blob *base, struct blob *our, struct
 		if (fill_mmfile_blob(&common, base) < 0)
 			goto out_free_f2_f1;
 	} else {
-		if (generate_common_file(&common, &f1, &f2) < 0)
-			goto out_free_f2_f1;
+		common.ptr = xstrdup("");
+		common.size = 0;
 	}
 	res = three_way_filemerge(path, &common, &f1, &f2, size);
 	free_mmfile(&common);
-- 
2.7.2.645.g4e1306c

  parent reply	other threads:[~2016-02-23  6:07 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-17 22:34 What's cooking in git.git (Feb 2016, #05; Wed, 17) Junio C Hamano
2016-02-17 23:25 ` Jeff King
2016-02-18 17:51   ` Junio C Hamano
2016-02-22 22:12 ` whither merge-tree? (was: What's cooking in git.git (Feb 2016, #05; Wed, 17)) Jeff King
2016-02-22 22:45   ` whither merge-tree? Junio C Hamano
2016-02-23  5:02     ` Jeff King
2016-02-23  5:14       ` Jeff King
2016-02-23  6:03         ` Jeff King
2016-02-23  6:04           ` [PATCH 1/3] merge-one-file: use empty blob for add/add base Jeff King
2016-02-23  6:06           ` Jeff King [this message]
2016-02-23  6:07           ` [PATCH 3/3] xdiff: drop XDL_EMIT_COMMON Jeff King
2016-02-23  6:35           ` whither merge-tree? Junio C Hamano
2016-02-23  7:18             ` Jeff King
2016-02-23  9:49       ` Stefan Frühwirth
2016-02-24  7:28         ` Dennis Kaarsemaker
2016-02-24  7:57           ` Jeff King
2016-02-24  7:58         ` Jeff King
2016-02-23 12:36       ` Johannes Schindelin
2016-02-23 12:41         ` Duy Nguyen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160223060655.GB3205@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=stefan.fruehwirth@uni-graz.at \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).