"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
fix tog diff between arbitrary commits
To:
gameoftrees@openbsd.org
Date:
Tue, 21 Feb 2023 22:25:31 +0100

Download raw body.

Thread
The cat_file() function in tog is unprepared for diffs between
commits that do not have a direct parent link in history.

To reproduce in the got.git repo, run:

$ tog diff 249b637c main
tog: fseek: Invalid argument
$

tog will either show an empty diff or fail as above with an error
from fseek when it tries to use a bogus line offset.

Line offsets for commit info are only created when diffing a commit
against its direct parent. Otherwise the value of *d_nlines will be zero.
But cat_file() will unconditionally use *d_nlines - 1 as an index into
the line offsets array to copy from. This ends up messing up the line
offsets array that is used when the diff gets rendered in draw_file().

Is this fix correct? Basically, this function somehow needs to deal
with zero-length commit info and/or zero-length diff text.

diff /home/stsp/src/got
commit - de51a12a5befe30cf15a089998d0136d52856dc2
path + /home/stsp/src/got
blob - 0d706f1c0a5161fd7a132b213b89c64e4cd17da0
file + tog/tog.c
--- tog/tog.c
+++ tog/tog.c
@@ -4578,14 +4578,20 @@ cat_diff(FILE *dst, FILE *src, struct got_diff_line **
 			return got_ferror(dst, GOT_ERR_IO);
 	}
 
+	if (s_nlines == 0 && *d_nlines == 0)
+		return NULL;
+
 	/*
 	 * The diff driver initialises the first line at offset zero when the
 	 * array isn't prepopulated, skip it; we already have it in *d_lines.
 	 */
-	for (i = 1; i < s_nlines; ++i)
-		s_lines[i].offset += (*d_lines)[*d_nlines - 1].offset;
+	if (*d_nlines > 0) {
+		for (i = 1; *d_nlines > 0 && i < s_nlines; ++i)
+			s_lines[i].offset += (*d_lines)[*d_nlines - 1].offset;
 
-	--s_nlines;
+		if (s_nlines > 0)
+			--s_nlines;
+	}
 
 	p = reallocarray(*d_lines, *d_nlines + s_nlines, sizeof(*p));
 	if (p == NULL) {