From: Stefan Sperling <stsp@stsp.name>
Subject: Re: diff.git: Add classic diff output
To: Tom Jones <thj@freebsd.org>, gameoftrees@openbsd.org
Date: Fri, 2 Sep 2022 10:57:26 +0200

On Tue, Aug 30, 2022 at 05:32:28PM +0200, Stefan Sperling wrote:
> On Tue, Aug 30, 2022 at 02:50:24PM +0100, Tom Jones wrote:
> > Working from diff_output_edscript add support for classic diff output.
> > The main differences between these two formats is the header
> > construction for a hunk and selection around which lines that are
> > output.
> 
> This is nice, thank you. I have committed it.
> 
> There is currently no way to enable plain mode when running a compiled
> diff binary from diff/diff.c. I briefly hacked my local copy to run
> your code and tested it. It would be nice if this output mode was
> reachable from the CLI so it could be tested by scripts.

As a follow-up fix, consider the following patch.
We must update the line offsets array whenever we generate a line
of diff output. Otherwise, tools like tog would have to process
the entire diff output again in order to figure out where the
line-breaks occur. For large diffs which are many mega-bytes
in size, this can be a huge performance hit.

ok?

diff /home/stsp/src/diff
commit - 810479a563ffd6176105f64cb49f9829d45d36df
path + /home/stsp/src/diff
blob - 1ead91212f5b867edc7e7187ec03e3d6eaf1f565
file + lib/diff_output_plain.c
--- lib/diff_output_plain.c
+++ lib/diff_output_plain.c
@@ -31,9 +31,9 @@ static int
 output_plain_chunk(struct diff_output_info *outinfo,
     FILE *dest, const struct diff_input_info *info,
     const struct diff_result *result,
-    struct diff_chunk_context *cc)
+    struct diff_chunk_context *cc, off_t *outoff)
 {
-	off_t outoff = 0, *offp;
+	off_t *offp;
 	int left_start, left_len, right_start, right_len;
 	int rc;
 	bool change = false;
@@ -95,6 +95,15 @@ output_plain_chunk(struct diff_output_info *outinfo,
 			    cc->left.end, right_start, cc->right.end);
 		}
 	}
+	if (rc < 0)
+		return errno;
+	if (outinfo) {
+		ARRAYLIST_ADD(offp, outinfo->line_offsets);
+		if (offp == NULL)
+			return ENOMEM;
+		*outoff += rc;
+		*offp = *outoff;
+	}
 
 	/*
 	 * Now write out all the joined chunks.
@@ -126,16 +135,6 @@ output_plain_chunk(struct diff_output_info *outinfo,
 		}
 	}
 
-	if (rc < 0)
-		return errno;
-	if (outinfo) {
-		ARRAYLIST_ADD(offp, outinfo->line_offsets);
-		if (offp == NULL)
-			return ENOMEM;
-		outoff += rc;
-		*offp = outoff;
-	}
-
 	return DIFF_RC_OK;
 }
 
@@ -153,6 +152,7 @@ diff_output_plain(struct diff_output_info **output_inf
 	bool force_text = (flags & DIFF_FLAG_FORCE_TEXT_DATA);
 	bool have_binary = (atomizer_flags & DIFF_ATOMIZER_FOUND_BINARY_DATA);
 	int i, rc;
+	off_t outoff = 0, *offp;
 
 	if (!result)
 		return EINVAL;
@@ -174,9 +174,18 @@ diff_output_plain(struct diff_output_info **output_inf
 			if (t != CHUNK_MINUS && t != CHUNK_PLUS)
 				continue;
 
-			fprintf(dest, "Binary files %s and %s differ\n",
+			rc = fprintf(dest, "Binary files %s and %s differ\n",
 			    diff_output_get_label_left(info),
 			    diff_output_get_label_right(info));
+			if (rc < 0)
+				return errno;
+			if (outinfo) {
+				ARRAYLIST_ADD(offp, outinfo->line_offsets);
+				if (offp == NULL)
+					return ENOMEM;
+				outoff += rc;
+				*offp = outoff;
+			}
 			break;
 		}
 
@@ -212,12 +221,14 @@ diff_output_plain(struct diff_output_info **output_inf
 			 * loop */
 			continue;
 		}
-		rc = output_plain_chunk(outinfo, dest, info, result, &cc);
+		rc = output_plain_chunk(outinfo, dest, info, result, &cc,
+		    &outoff);
 		if (rc != DIFF_RC_OK)
 			return rc;
 		cc = next;
 	}
 	if (!diff_chunk_context_empty(&cc))
-		return output_plain_chunk(outinfo, dest, info, result, &cc);
+		return output_plain_chunk(outinfo, dest, info, result, &cc,
+		    &outoff);
 	return DIFF_RC_OK;
 }