Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 18 Feb 2024 17:41:40 GMT
From:      Dag-Erling =?utf-8?Q?Sm=C3=B8rgrav?= <des@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: ad7bef8b8907 - main - sdiff: Fix binary case.
Message-ID:  <202402181741.41IHfexD060731@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by des:

URL: https://cgit.FreeBSD.org/src/commit/?id=ad7bef8b890768e68a48bbfa6b92ebf635068504

commit ad7bef8b890768e68a48bbfa6b92ebf635068504
Author:     Dag-Erling Smørgrav <des@FreeBSD.org>
AuthorDate: 2024-02-18 17:39:31 +0000
Commit:     Dag-Erling Smørgrav <des@FreeBSD.org>
CommitDate: 2024-02-18 17:39:50 +0000

    sdiff: Fix binary case.
    
    MFC after:      1 week
    Sponsored by:   Klara, Inc.
    Reviewed by:    markj
    Differential Revision:  https://reviews.freebsd.org/D43942
---
 usr.bin/sdiff/sdiff.c             | 100 ++++++++++++++++++++------------------
 usr.bin/sdiff/tests/sdiff_test.sh |  18 +++++++
 2 files changed, 72 insertions(+), 46 deletions(-)

diff --git a/usr.bin/sdiff/sdiff.c b/usr.bin/sdiff/sdiff.c
index 41284d54f869..05e3ee82212c 100644
--- a/usr.bin/sdiff/sdiff.c
+++ b/usr.bin/sdiff/sdiff.c
@@ -50,7 +50,7 @@ static void astrcat(char **, const char *);
 static void enqueue(char *, char, char *);
 static char *mktmpcpy(const char *);
 static int istextfile(FILE *);
-static void binexec(char *, char *, char *) __dead2;
+static int bindiff(FILE *, char *, FILE *, char *);
 static void freediff(struct diffline *);
 static void int_usage(void);
 static int parsecmd(FILE *, FILE *, FILE *);
@@ -209,7 +209,7 @@ main(int argc, char **argv)
 {
 	FILE *diffpipe, *file1, *file2;
 	size_t diffargc = 0, flagc = 0, wval = WIDTH;
-	int ch, fd[2], i, status;
+	int ch, fd[2], i, ret, status;
 	pid_t pid;
 	const char *errstr, *outfile = NULL;
 	char **diffargv, *diffprog = diff_path, *flagv;
@@ -355,6 +355,15 @@ main(int argc, char **argv)
 			filename2 = tmp2;
 	}
 
+	if ((file1 = fopen(filename1, "r")) == NULL)
+		err(2, "could not open %s", filename1);
+	if ((file2 = fopen(filename2, "r")) == NULL)
+		err(2, "could not open %s", filename2);
+	if (!istextfile(file1) || !istextfile(file2)) {
+		ret = bindiff(file1, filename1, file2, filename2);
+		goto done;
+	}
+
 	diffargv[diffargc++] = filename1;
 	diffargv[diffargc++] = filename2;
 	/* Add NULL to end of array to indicate end of array. */
@@ -392,26 +401,6 @@ main(int argc, char **argv)
 	if ((diffpipe = fdopen(fd[0], "r")) == NULL)
 		err(2, "could not open diff pipe");
 
-	if ((file1 = fopen(filename1, "r")) == NULL)
-		err(2, "could not open %s", filename1);
-	if ((file2 = fopen(filename2, "r")) == NULL)
-		err(2, "could not open %s", filename2);
-	if (!istextfile(file1) || !istextfile(file2)) {
-		/* Close open files and pipe, delete temps */
-		fclose(file1);
-		fclose(file2);
-		if (diffpipe != NULL)
-			fclose(diffpipe);
-		if (tmp1)
-			if (unlink(tmp1))
-				warn("Error deleting %s.", tmp1);
-		if (tmp2)
-			if (unlink(tmp2))
-				warn("Error deleting %s.", tmp2);
-		free(tmp1);
-		free(tmp2);
-		binexec(diffprog, filename1, filename2);
-	}
 	/* Line numbers start at one. */
 	file1ln = file2ln = 1;
 
@@ -423,20 +412,10 @@ main(int argc, char **argv)
 	/* Wait for diff to exit. */
 	if (waitpid(pid, &status, 0) == -1 || !WIFEXITED(status) ||
 	    WEXITSTATUS(status) >= 2)
-		err(2, "diff exited abnormally.");
+		errx(2, "diff exited abnormally");
+	ret = WEXITSTATUS(status);
 
-	/* Delete and free unneeded temporary files. */
-	if (tmp1)
-		if (unlink(tmp1))
-			warn("Error deleting %s.", tmp1);
-	if (tmp2)
-		if (unlink(tmp2))
-			warn("Error deleting %s.", tmp2);
-	free(tmp1);
-	free(tmp2);
-	filename1 = filename2 = tmp1 = tmp2 = NULL;
-
-	/* No more diffs, so print common lines. */
+	/* No more diffs, so enqueue common lines. */
 	if (lflag)
 		while ((s1 = xfgets(file1)))
 			enqueue(s1, ' ', NULL);
@@ -454,26 +433,55 @@ main(int argc, char **argv)
 	/* Process unmodified lines. */
 	processq();
 
+done:
+	/* Delete and free unneeded temporary files. */
+	if (tmp1 != NULL) {
+		if (unlink(tmp1) != 0)
+			warn("failed to delete %s", tmp1);
+		free(tmp1);
+	}
+	if (tmp2 != NULL) {
+		if (unlink(tmp2) != 0)
+			warn("failed to delete %s", tmp2);
+		free(tmp2);
+	}
+
 	/* Return diff exit status. */
 	free(diffargv);
 	if (flagc > 0)
 		free(flagv);
-	return (WEXITSTATUS(status));
+	return (ret);
 }
 
 /*
- * When sdiff detects a binary file as input, executes them with
- * diff to maintain the same behavior as GNU sdiff with binary input.
+ * When sdiff detects a binary file as input.
  */
-static void
-binexec(char *diffprog, char *f1, char *f2)
+static int
+bindiff(FILE *f1, char *fn1, FILE *f2, char *fn2)
 {
-
-	char *args[] = {diffprog, f1, f2, (char *) 0};
-	execv(diffprog, args);
-
-	/* If execv() fails, sdiff's execution will continue below. */
-	errx(1, "could not execute diff process");
+	int ch1, ch2;
+
+	flockfile(f1);
+	flockfile(f2);
+	do {
+		ch1 = getc_unlocked(f1);
+		ch2 = getc_unlocked(f2);
+	} while (ch1 != EOF && ch2 != EOF && ch1 == ch2);
+	funlockfile(f2);
+	funlockfile(f1);
+	if (ferror(f1)) {
+		warn("%s", fn1);
+		return (2);
+	}
+	if (ferror(f2)) {
+		warn("%s", fn2);
+		return (2);
+	}
+	if (ch1 != EOF || ch2 != EOF) {
+		printf("Binary files %s and %s differ\n", fn1, fn2);
+		return (1);
+	}
+	return (0);
 }
 
 /*
diff --git a/usr.bin/sdiff/tests/sdiff_test.sh b/usr.bin/sdiff/tests/sdiff_test.sh
index c701c1704d3c..83ed93503f18 100755
--- a/usr.bin/sdiff/tests/sdiff_test.sh
+++ b/usr.bin/sdiff/tests/sdiff_test.sh
@@ -208,6 +208,23 @@ tflag_body()
 		  sdiff -t --tabsize 4 a b
 }
 
+atf_test_case binary
+binary_head()
+{
+	atf_set "descr" "Checks binary file handling"
+}
+binary_body()
+{
+	printf "a\0\n" >a
+	printf "b\0\n" >b
+	atf_check -o empty sdiff a a
+	atf_check -o empty sdiff a - <a
+	atf_check -s exit:1 -o match:"Binary files .* differ" \
+		  sdiff a b
+	atf_check -s exit:1 -o match:"Binary files .* differ" \
+		  sdiff a - <b
+}
+
 atf_init_test_cases()
 {
 	atf_add_test_case flags
@@ -221,4 +238,5 @@ atf_init_test_cases()
 	atf_add_test_case stdin
 	atf_add_test_case short
 	atf_add_test_case tflag
+	atf_add_test_case binary
 }



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202402181741.41IHfexD060731>