From: Stefan Sperling <stsp@stsp.name>
Subject: fix diff3 for files which lack newlines
To: gameoftrees@openbsd.org
Date: Thu, 25 Jun 2020 14:31:07 +0200

While working on symlink support I discovered a bug in diff3 code.

To handle merge conflicts which involve symlinks I am dumping the symlink
"content" (i.e. the target path) into a file and pass that to diff3 as
content to be merged. Symlinks do not contain a newline character and
it turns out that diff3 fails to merge such files.

This can be easily reproduced with merge(1) and diff3(1), too:
$ echo foo > foo
$ echo bar > bar
$ echo -n baz > baz
$ merge -p foo bar baz
merge: failed to merge
$ diff3 -E foo bar baz
1a
=======
diff3prog: logic error
$

It's only the last file argument which triggers the problem. The other
files may or may not contain newlines and everything works as expected.

The first patch below fixes the problem for got.

Patches for diff3, openrcs, and opencvs follow, even though tech@ would be
a more appropriate forum for those. There is only one place to fix here
because these tools lack the 3-way conflict output that got provides.

I have added tests on got's symlink branch which confirm that this fix
works as expected. And I have verified that merge(1) and diff3(1) now
produce the expected output by comparing the new output to the already
working case where 'baz' does contain a newline.

diff 2fa0278ff0206405007b62f538d6a1529f1efc87 75771bb4765fcd7d5a6317960219414a37378dcd
blob - f17c676ca747debbb6b3004920d6d5712cef2810
blob + e8841fa9d921c07d456480829fe90a68bca1f84a
--- lib/diff3.c
+++ lib/diff3.c
@@ -1002,10 +1002,17 @@ edscript(int n, struct diff3_state *d3s)
 				return got_error_from_errno("fseeko");
 			k = (size_t)(d3s->de[n].oldo.to - d3s->de[n].oldo.from);
 			for (; k > 0; k -= len) {
+				size_t r;
 				len = k > BUFSIZ ? BUFSIZ : k;
-				if (fread(block, 1, len, d3s->fp[1]) != len)
+				r = fread(block, 1, len, d3s->fp[1]);
+				if (r == 0) {
+					if (feof(d3s->fp[1]))
+						break;
 					return got_ferror(d3s->fp[1],
 					    GOT_ERR_IO);
+				}
+				if (r != len)
+					len = r;
 				block[len] = '\0';
 				err = diff_output(d3s->diffbuf, "%s", block);
 				if (err)
@@ -1026,9 +1033,17 @@ edscript(int n, struct diff3_state *d3s)
 			return got_error_from_errno("fseek");
 		k = (size_t)(d3s->de[n].newo.to - d3s->de[n].newo.from);
 		for (; k > 0; k -= len) {
+			size_t r;
 			len = k > BUFSIZ ? BUFSIZ : k;
-			if (fread(block, 1, len, d3s->fp[2]) != len)
-				return got_ferror(d3s->fp[2], GOT_ERR_IO);
+			r = fread(block, 1, len, d3s->fp[2]);
+			if (r == 0) {
+				if (feof(d3s->fp[2]))
+					break;
+				return got_ferror(d3s->fp[2],
+				    GOT_ERR_IO);
+			}
+			if (r != len)
+				len = r;
 			block[len] = '\0';
 			err = diff_output(d3s->diffbuf, "%s", block);
 			if (err)



diff 86d9d45324cfd7bd7f0fbaebe1421842bac5c78d /usr/src
blob - e8e0512d85a77a5c1ac7f652d3304a1474fe05e2
file + usr.bin/cvs/diff3.c
--- usr.bin/cvs/diff3.c
+++ usr.bin/cvs/diff3.c
@@ -798,9 +798,16 @@ edscript(int n)
 			diff_output("%da\n=======\n", de[n].old.to -1);
 		(void)fseek(fp[2], (long)de[n].new.from, SEEK_SET);
 		for (k = de[n].new.to-de[n].new.from; k > 0; k-= j) {
+			size_t r;
 			j = k > BUFSIZ ? BUFSIZ : k;
-			if (fread(block, 1, j, fp[2]) != (size_t)j)
+			r = fread(block, 1, j, fp[2]);
+			if (r == 0) {
+				if (feof(fp[2]))
+					break;
 				return (-1);
+			}
+			if (r != (size_t)j)
+				j = r;
 			block[j] = '\0';
 			diff_output("%s", block);
 		}
blob - 992037b6d850876744e12f246d9885fed7e5627e
file + usr.bin/diff3/diff3prog.c
--- usr.bin/diff3/diff3prog.c
+++ usr.bin/diff3/diff3prog.c
@@ -555,9 +555,16 @@ edscript(int n)
 			printf("%da\n=======\n", de[n].old.to -1);
 		(void)fseek(fp[2], (long)de[n].new.from, SEEK_SET);
 		for (k = de[n].new.to-de[n].new.from; k > 0; k-= j) {
+			size_t r;
 			j = k > BUFSIZ ? BUFSIZ : k;
-			if (fread(block, 1, j, fp[2]) != j)
+			r = fread(block, 1, j, fp[2]);
+			if (r == 0) {
+				if (feof(fp[2]))
+					break;
 				trouble();
+			}
+			if (r != j)
+				j = r;
 			(void)fwrite(block, 1, j, stdout);
 		}
 		if (!oflag || !overlap[n])
blob - 38d41c19e0268d073ed588f0b60c5a5812a544a9
file + usr.bin/rcs/diff3.c
--- usr.bin/rcs/diff3.c
+++ usr.bin/rcs/diff3.c
@@ -893,9 +893,16 @@ edscript(int n)
 			diff_output("%da\n=======\n", de[n].old.to -1);
 		(void)fseek(fp[2], (long)de[n].new.from, SEEK_SET);
 		for (k = de[n].new.to-de[n].new.from; k > 0; k-= j) {
+			size_t r;
 			j = k > BUFSIZ ? BUFSIZ : k;
-			if (fread(block, 1, j, fp[2]) != (size_t)j)
-				return (-1);
+			r = fread(block, 1, j, fp[2]);
+			if (r == 0) {
+				if (feof(fp[2]))
+					break;
+				return -1;
+			}
+			if (r != (size_t)j)
+				j = r;
 			block[j] = '\0';
 			diff_output("%s", block);
 		}