From: Stefan Sperling Subject: fix tog diff between arbitrary commits To: gameoftrees@openbsd.org Date: Tue, 21 Feb 2023 22:25:31 +0100 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) {