On Sat, Jan 25, 2020 at 05:06:49PM +0100, Stefan Sperling wrote:
> On Sat, Jan 25, 2020 at 09:00:34AM -0700, Tracey Emery wrote:
> > On Sat, Jan 25, 2020 at 11:14:36AM +0100, Stefan Sperling wrote:
> > > Won't this change leave empty files around in /tmp? The temp files will
> > > be closed but won't be removed from disk.
> > 
> > No files are visible to the filesystem with got_opentemp. The file is
> > unlinked immediately.
> 
> Ah, of course. Sorry, I forgot about that.
> 
> > > I think leaving f1 or f2 as NULL and checking for NULL everywhere else
> > > inside the diffreg code would be a better solution.
> > >  
> > 
> > I agree. I'm just not going to get time this weekend to hack on that, so
> > it is just a suggested temporary fix.
> 
> Yes, in that case feel free to commit it 

This avoids opening empty files in the first place. Did I miss anything?

The 'closem' label should now be renamed to 'done', and D_EMPTY1/D_EMPTY2
flags should be removed since a NULL pointer already implies an empty file.
But such mechanical changes are best done separately to keep this diff small.

diff --git a/lib/diffreg.c b/lib/diffreg.c
index 2087de2..6ababe7 100644
--- a/lib/diffreg.c
+++ b/lib/diffreg.c
@@ -92,12 +92,6 @@
 
 #include "got_lib_diff.h"
 
-/* 
- * XXX:band-aid patch include
- * remove when proper patch in place
- */
-#include <got_opentemp.h>
-
 #define MINIMUM(a, b)	(((a) < (b)) ? (a) : (b))
 #define MAXIMUM(a, b)	(((a) > (b)) ? (a) : (b))
 
@@ -201,7 +195,7 @@ static int	 skipline(FILE *);
 static int	 isqrt(int);
 static int	 stone(struct got_diff_state *, int *, int, int *, int *, int);
 static int	 readhash(struct got_diff_state *, FILE *, int);
-static int	 files_differ(struct got_diff_state *, FILE *, FILE *, int);
+static int	 files_differ(struct got_diff_state *, FILE *, FILE *);
 static char	*match_function(struct got_diff_state *, const long *, int, FILE *);
 
 /*
@@ -310,30 +304,17 @@ got_diffreg(int *rval, FILE *f1, FILE *f2, int flags,
 		*rval = (S_ISDIR(ds->stb1.st_mode) ? D_MISMATCH1 : D_MISMATCH2);
 		return NULL;
 	}
-	if (flags & D_EMPTY1) {
-		f1 = got_opentemp();
-		if (f1 == NULL) {
-			err = got_error_from_errno2("fopen", _PATH_DEVNULL);
-			goto closem;
-		}
-	}
-	else if (f1 == NULL) {
+	if (!(flags & D_EMPTY1) && f1 == NULL) {
 		args->status |= 2;
 		goto closem;
 	}
 
-	if (flags & D_EMPTY2) {
-		f2 = got_opentemp();
-		if (f2 == NULL) {
-			err = got_error_from_errno2("fopen", _PATH_DEVNULL);
-			goto closem;
-		}
-	} else if (f2 == NULL) {
+	if (!(flags & D_EMPTY2) && f2 == NULL) {
 		args->status |= 2;
 		goto closem;
 	}
 
-	switch (files_differ(ds, f1, f2, flags)) {
+	switch (files_differ(ds, f1, f2)) {
 	case 0:
 		goto closem;
 	case 1:
@@ -432,14 +413,6 @@ closem:
 		if (*rval == D_SAME)
 			*rval = D_DIFFER;
 	}
-	if ((flags & D_EMPTY1) && f1) {
-		if (fclose(f1) != 0 && err == NULL)
-			err = got_error_from_errno("fclose");
-	}
-	if ((flags & D_EMPTY2) && f2) {
-		if (fclose(f2) != 0 && err == NULL)
-			err = got_error_from_errno("fclose");
-	}
 	return (err);
 }
 
@@ -449,12 +422,12 @@ closem:
  * XXX - could use code from cmp(1) [faster]
  */
 static int
-files_differ(struct got_diff_state *ds, FILE *f1, FILE *f2, int flags)
+files_differ(struct got_diff_state *ds, FILE *f1, FILE *f2)
 {
 	char buf1[BUFSIZ], buf2[BUFSIZ];
 	size_t i, j;
 
-	if ((flags & (D_EMPTY1|D_EMPTY2)) || ds->stb1.st_size != ds->stb2.st_size ||
+	if (f1 == NULL || f2 == NULL || ds->stb1.st_size != ds->stb2.st_size ||
 	    (ds->stb1.st_mode & S_IFMT) != (ds->stb2.st_mode & S_IFMT))
 		return (1);
 	for (;;) {
@@ -478,7 +451,8 @@ prepare(struct got_diff_state *ds, int i, FILE *fd, off_t filesize, int flags)
 	int j, h;
 	size_t sz;
 
-	rewind(fd);
+	if (fd != NULL)
+		rewind(fd);
 
 	sz = (filesize <= SIZE_MAX ? filesize : SIZE_MAX) / 25;
 	if (sz < 100)
@@ -487,7 +461,7 @@ prepare(struct got_diff_state *ds, int i, FILE *fd, off_t filesize, int flags)
 	p = calloc(sz + 3, sizeof(*p));
 	if (p == NULL)
 		return (-1);
-	for (j = 0; (h = readhash(ds, fd, flags));) {
+	for (j = 0; fd != NULL && (h = readhash(ds, fd, flags));) {
 		if (j == sz) {
 			sz = sz * 3 / 2;
 			q = reallocarray(p, sz + 3, sizeof(*p));
@@ -701,8 +675,10 @@ check(struct got_diff_state *ds, FILE *f1, FILE *f2, int flags)
 	int i, j, jackpot, c, d;
 	long ctold, ctnew;
 
-	rewind(f1);
-	rewind(f2);
+	if (f1 != NULL)
+		rewind(f1);
+	if (f2 != NULL)
+		rewind(f2);
 	j = 1;
 	ds->ixold[0] = ds->ixnew[0] = 0;
 	jackpot = 0;
@@ -718,8 +694,8 @@ check(struct got_diff_state *ds, FILE *f1, FILE *f2, int flags)
 		}
 		if (flags & (D_FOLDBLANKS|D_IGNOREBLANKS|D_IGNORECASE)) {
 			for (;;) {
-				c = getc(f1);
-				d = getc(f2);
+				c = (f1 == NULL ? EOF : getc(f1));
+				d = (f2 == NULL ? EOF : getc(f2));
 				/*
 				 * GNU diff ignores a missing newline
 				 * in one file for -b or -w.
@@ -741,18 +717,18 @@ check(struct got_diff_state *ds, FILE *f1, FILE *f2, int flags)
 						if (c == '\n')
 							break;
 						ctold++;
-					} while (isspace(c = getc(f1)));
+					} while (f1 && isspace(c = getc(f1)));
 					do {
 						if (d == '\n')
 							break;
 						ctnew++;
-					} while (isspace(d = getc(f2)));
+					} while (f2 && isspace(d = getc(f2)));
 				} else if ((flags & D_IGNOREBLANKS)) {
-					while (isspace(c) && c != '\n') {
+					while (f1 && isspace(c) && c != '\n') {
 						c = getc(f1);
 						ctold++;
 					}
-					while (isspace(d) && d != '\n') {
+					while (f2 && isspace(d) && d != '\n') {
 						d = getc(f2);
 						ctnew++;
 					}
@@ -773,7 +749,9 @@ check(struct got_diff_state *ds, FILE *f1, FILE *f2, int flags)
 			for (;;) {
 				ctold++;
 				ctnew++;
-				if ((c = getc(f1)) != (d = getc(f2))) {
+				c = (f1 == NULL ? EOF : getc(f1));
+				d = (f2 == NULL ? EOF : getc(f2));
+				if (c != d) {
 					/* jackpot++; */
 					ds->J[i] = 0;
 					if (c != '\n' && c != EOF)
@@ -853,7 +831,7 @@ skipline(FILE *f)
 {
 	int i, c;
 
-	for (i = 1; (c = getc(f)) != '\n' && c != EOF; i++)
+	for (i = 1; f != NULL && (c = getc(f)) != '\n' && c != EOF; i++)
 		continue;
 	return (i);
 }
@@ -866,8 +844,10 @@ output(FILE *outfile, struct got_diff_changes *changes,
 	int m, i0, i1, j0, j1;
 	int error = 0;
 
-	rewind(f1);
-	rewind(f2);
+	if (f1 != NULL)
+		rewind(f1);
+	if (f2 != NULL)
+		rewind(f2);
 	m = ds->len[0];
 	ds->J[0] = 0;
 	ds->J[m + 1] = ds->len[1] + 1;
@@ -1002,7 +982,7 @@ fetch(FILE *outfile, struct got_diff_state *ds, struct got_diff_args *args,
 {
 	int i, j, c, col, nc;
 
-	if (a > b)
+	if (lb == NULL || a > b)
 		return;
 	for (i = a; i <= b; i++) {
 		fseek(lb, f[i - 1], SEEK_SET);
@@ -1183,7 +1163,7 @@ dump_unified_vec(FILE *outfile, struct got_diff_changes *changes,
 	diff_output(outfile, " +");
 	uni_range(outfile, lowc, upd);
 	diff_output(outfile, " @@");
-	if ((flags & D_PROTOTYPE)) {
+	if (f1 != NULL && (flags & D_PROTOTYPE)) {
 		f = match_function(ds, ds->ixold, lowa-1, f1);
 		if (f != NULL)
 			diff_output(outfile, " %s", f);