Unlink temp file as soon as it is opened and return a FILE * for

it so we don't have to worry about cleanup.  This means the quit()
signal handler and error/errorx can go away too.

Move splice() out of diffreg() and into diff.c where it belongs
since we don't want to be calling splice() for a directory diff.
Add a check for mismatched paths (one file, one dir) in diffreg.c.

deraadt@ OK

3 files changed, 67 insertions, 123 deletions

diff --git a/usr.bin/diff/diff.c b/usr.bin/diff/diff.c
index ef929d1ff3b..15c112bab53 100644
--- a/usr.bin/diff/diff.c
+++ b/usr.bin/diff/diff.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: diff.c,v 1.27 2003/07/09 00:07:44 millert Exp $	*/
+/*	$OpenBSD: diff.c,v 1.28 2003/07/09 00:39:25 millert Exp $	*/
 
 /*
  * Copyright (c) 2003 Todd C. Miller <Todd.Miller@courtesan.com>
@@ -21,7 +21,7 @@
  */
 
 #ifndef lint
-static const char rcsid[] = "$OpenBSD: diff.c,v 1.27 2003/07/09 00:07:44 millert Exp $";
+static const char rcsid[] = "$OpenBSD: diff.c,v 1.28 2003/07/09 00:39:25 millert Exp $";
 #endif /* not lint */
 
 #include <sys/param.h>
@@ -30,6 +30,7 @@ static const char rcsid[] = "$OpenBSD: diff.c,v 1.27 2003/07/09 00:07:44 millert
 #include <err.h>
 #include <errno.h>
 #include <getopt.h>
+#include <signal.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include <stdarg.h>
@@ -120,6 +121,7 @@ main(int argc, char **argv)
 			break;
 		case 'l':
 			lflag = 1;
+			signal(SIGPIPE, SIG_IGN);
 			break;
 		case 'N':
 			Nflag = 1;
@@ -183,43 +185,43 @@ main(int argc, char **argv)
 		fstat(STDIN_FILENO, &stb1);
 		gotstdin = 1;
 	} else if (stat(argv[0], &stb1) != 0)
-		error("%s", argv[0]);
+		err(2, "%s", argv[0]);
 	if (strcmp(argv[1], "-") == 0) {
 		fstat(STDIN_FILENO, &stb2);
 		gotstdin = 1;
 	} else if (stat(argv[1], &stb2) != 0)
-		error("%s", argv[1]);
+		err(2, "%s", argv[1]);
 	if (gotstdin && (S_ISDIR(stb1.st_mode) || S_ISDIR(stb2.st_mode)))
-		errorx("can't compare - to a directory");
+		errx(2, "can't compare - to a directory");
 	set_argstr(oargv, argv);
 	if (S_ISDIR(stb1.st_mode) && S_ISDIR(stb2.st_mode)) {
 		if (format == D_IFDEF)
-			errorx("-D option not supported with directories");
+			errx(2, "-D option not supported with directories");
 		diffdir(argv[0], argv[1]);
 	} else {
+		if (S_ISDIR(stb1.st_mode)) {
+			argv[0] = splice(argv[0], argv[1]);
+			if (stat(argv[0], &stb1) < 0)
+				err(2, "%s", argv[0]);
+		}
+		if (S_ISDIR(stb2.st_mode)) {
+			argv[1] = splice(argv[1], argv[0]);
+			if (stat(argv[1], &stb2) < 0)
+				err(2, "%s", argv[1]);
+		}
 		print_status(diffreg(argv[0], argv[1], 0), argv[0], argv[1],
 		    NULL);
 	}
 	exit(status);
 }
 
-void
-quit(int signo)
-{
-	if (tempfiles[0] != NULL)
-		unlink(tempfiles[0]);
-	if (tempfiles[1] != NULL)
-		unlink(tempfiles[1]);
-	_exit(status);
-}
-
 void *
 emalloc(size_t n)
 {
 	void *p;
 
 	if ((p = malloc(n)) == NULL)
-		error(NULL);
+		err(2, NULL);
 	return (p);
 }
 
@@ -229,7 +231,7 @@ erealloc(void *p, size_t n)
 	void *q;
 
 	if ((q = realloc(p, n)) == NULL)
-		error(NULL);
+		err(2, NULL);
 	return (q);
 }
 
@@ -244,40 +246,10 @@ easprintf(char **ret, const char *fmt, ...)
 	va_end(ap);
 
 	if (len == -1)
-		error(NULL);
+		err(2, NULL);
 	return(len);
 }
 
-__dead void
-error(const char *fmt, ...)
-{
-	va_list ap;
-	int sverrno = errno;
-
-	if (tempfiles[0] != NULL)
-		unlink(tempfiles[0]);
-	if (tempfiles[1] != NULL)
-		unlink(tempfiles[1]);
-	errno = sverrno;
-	va_start(ap, fmt);
-	verr(2, fmt, ap);
-	va_end(ap);
-}
-
-__dead void
-errorx(const char *fmt, ...)
-{
-	va_list ap;
-
-	if (tempfiles[0] != NULL)
-		unlink(tempfiles[0]);
-	if (tempfiles[1] != NULL)
-		unlink(tempfiles[1]);
-	va_start(ap, fmt);
-	verrx(2, fmt, ap);
-	va_end(ap);
-}
-
 void
 set_argstr(char **av, char **ave)
 {
@@ -308,7 +280,7 @@ read_excludes_file(char *file)
 	if (strcmp(file, "-") == 0)
 		fp = stdin;
 	else if ((fp = fopen(file, "r")) == NULL)
-		error("%s", file);
+		err(2, "%s", file);
 	while ((buf = fgetln(fp, &len)) != NULL) {
 		if (buf[len - 1] == '\n')
 			len--;
@@ -362,6 +334,10 @@ print_status(int val, char *path1, char *path2, char *entry)
 			    path1, entry ? entry : "",
 			    path2, entry ? entry : "");
 		break;
+	case D_MISMATCH:
+		printf("File %s/%s is a directory but file %s/%s is not\n",
+		    path1, entry ? entry : "", path2, entry ? entry : "");
+		break;
 	}
 }
 
diff --git a/usr.bin/diff/diff.h b/usr.bin/diff/diff.h
index b08f10120f2..9ff20a8e913 100644
--- a/usr.bin/diff/diff.h
+++ b/usr.bin/diff/diff.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: diff.h,v 1.18 2003/07/09 00:07:44 millert Exp $	*/
+/*	$OpenBSD: diff.h,v 1.19 2003/07/09 00:39:25 millert Exp $	*/
 
 /*-
  * Copyright (c) 1991, 1993
@@ -59,7 +59,8 @@
 #define	D_BINARY	2	/* Binary files are different */
 #define	D_COMMON	3	/* Subdirectory common to both dirs */
 #define	D_ONLY		4	/* Only exists in one directory */
-#define	D_ERROR		5	/* An error ocurred */
+#define	D_MISMATCH	5	/* One path was a dir, the other a file */
+#define	D_ERROR		6	/* An error ocurred */
 
 struct excludes {
 	char *pattern;
@@ -68,13 +69,11 @@ struct excludes {
 
 extern int	aflag, bflag, iflag, lflag, Nflag, Pflag, rflag, sflag,
 		tflag, wflag;
-extern char	*start, *ifdefname;
 extern int	format, context, status, anychange;
-extern char	*tempfiles[], *diffargs;
+extern char	*start, *ifdefname, *diffargs;
 extern struct	stat stb1, stb2;
 extern struct	excludes *excludes_list;
 
-char	*copytemp(const char *, int);
 char	*splice(char *, char *);
 int	diffreg(char *, char *, int);
 int	easprintf(char **, const char *, ...);
@@ -82,6 +81,3 @@ void	*emalloc(size_t);
 void	*erealloc(void *, size_t);
 void	diffdir(char *, char *);
 void	print_status(int, char *, char *, char *);
-void	quit(int);
-__dead void error(const char *, ...);
-__dead void errorx(const char *, ...);
diff --git a/usr.bin/diff/diffreg.c b/usr.bin/diff/diffreg.c
index e8b14d785e5..576bc38f12b 100644
--- a/usr.bin/diff/diffreg.c
+++ b/usr.bin/diff/diffreg.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: diffreg.c,v 1.31 2003/07/09 00:07:44 millert Exp $	*/
+/*	$OpenBSD: diffreg.c,v 1.32 2003/07/09 00:39:26 millert Exp $	*/
 
 /*
  * Copyright (C) Caldera International Inc.  2001-2002.
@@ -65,18 +65,18 @@
  */
 
 #ifndef lint
-static const char rcsid[] = "$OpenBSD: diffreg.c,v 1.31 2003/07/09 00:07:44 millert Exp $";
+static const char rcsid[] = "$OpenBSD: diffreg.c,v 1.32 2003/07/09 00:39:26 millert Exp $";
 #endif /* not lint */
 
-#include <sys/types.h>
+#include <sys/param.h>
 #include <sys/stat.h>
 #include <sys/wait.h>
 
 #include <ctype.h>
 #include <err.h>
+#include <errno.h>
 #include <fcntl.h>
 #include <libgen.h>
-#include <signal.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -179,6 +179,7 @@ static struct cand *clist;	/* merely a free storage pot for candidates */
 static struct line *sfile[2];	/* shortened by pruning common prefix/suffix */
 static u_char *chrtran;		/* translation table for case-folding */
 
+static FILE *opentemp(const char *);
 static void fetch(long *, int, int, FILE *, char *, int);
 static void output(char *, FILE *, char *, FILE *);
 static void check(char *, FILE *, char *, FILE *);
@@ -271,29 +272,22 @@ diffreg(char *ofile1, char *ofile2, int flags)
 
 	anychange = 0;
 	chrtran = (iflag ? cup2low : clow2low);
+	if (S_ISDIR(stb1.st_mode) != S_ISDIR(stb2.st_mode))
+		return (D_MISMATCH);
 	if (strcmp(file1, "-") == 0 && strcmp(file2, "-") == 0)
 		goto notsame;
 
-	/* XXX - only make temp file for stdin if not seekable? (millert) */
 	if (flags & D_EMPTY1)
 		f1 = fopen(_PATH_DEVNULL, "r");
 	else {
-		if (S_ISDIR(stb1.st_mode)) {
-			file1 = splice(file1, file2);
-			if (stat(file1, &stb1) < 0) {
+		if (!S_ISREG(stb1.st_mode)) {
+			if ((f1 = opentemp(file1)) == NULL ||
+			    fstat(fileno(f1), &stb1) < 0) {
 				warn("%s", file1);
 				status |= 2;
 				goto closem;
 			}
-		} else if (!S_ISREG(stb1.st_mode)) {
-			file1 = copytemp(file1, 1);
-			if (file1 == NULL || stat(file1, &stb1) < 0) {
-				warn("%s", file1);
-				status |= 2;
-				goto closem;
-			}
-		}
-		if (strcmp(file1, "-") == 0)
+		} else if (strcmp(file1, "-") == 0)
 			f1 = stdin;
 		else
 			f1 = fopen(file1, "r");
@@ -307,22 +301,14 @@ diffreg(char *ofile1, char *ofile2, int flags)
 	if (flags & D_EMPTY2)
 		f2 = fopen(_PATH_DEVNULL, "r");
 	else {
-		if (S_ISDIR(stb2.st_mode)) {
-			file2 = splice(file2, file1);
-			if (stat(file2, &stb2) < 0) {
-				warn("%s", file2);
-				status |= 2;
-				goto closem;
-			}
-		} else if (!S_ISREG(stb2.st_mode)) {
-			file2 = copytemp(file2, 2);
-			if (file2 == NULL || stat(file2, &stb2) < 0) {
+		if (!S_ISREG(stb2.st_mode)) {
+			if ((f2 = opentemp(file2)) == NULL ||
+			    fstat(fileno(f2), &stb2) < 0) {
 				warn("%s", file2);
 				status |= 2;
 				goto closem;
 			}
-		}
-		if (strcmp(file2, "-") == 0)
+		} else if (strcmp(file2, "-") == 0)
 			f2 = stdin;
 		else
 			f2 = fopen(file2, "r");
@@ -450,17 +436,9 @@ closem:
 		fclose(f1);
 	if (f2 != NULL)
 		fclose(f2);
-	if (tempfiles[0] != NULL) {
-		unlink(tempfiles[0]);
-		free(tempfiles[0]);
-		tempfiles[0] = NULL;
-	} else if (file1 != ofile1)
+	if (file1 != ofile1)
 		free(file1);
-	if (tempfiles[1] != NULL) {
-		unlink(tempfiles[1]);
-		free(tempfiles[1]);
-		tempfiles[1] = NULL;
-	} else if (file2 != ofile2)
+	if (file2 != ofile2)
 		free(file2);
 	return (rval);
 }
@@ -494,17 +472,12 @@ files_differ(FILE *f1, FILE *f2, int flags)
 	}
 }
 
-char *tempfiles[2];
-
-/* XXX - pass back a FILE * too (millert) */
-char *
-copytemp(const char *file, int n)
+static FILE *
+opentemp(const char *file)
 {
-	char buf[BUFSIZ], *tempdir, *tempfile;
-	int i, ifd, ofd;
-
-	if (n != 1 && n != 2)
-		return (NULL);
+	char buf[BUFSIZ], *tempdir, tempfile[MAXPATHLEN];
+	ssize_t nread;
+	int ifd, ofd;
 
 	if (strcmp(file, "-") == 0)
 		ifd = STDIN_FILENO;
@@ -513,25 +486,25 @@ copytemp(const char *file, int n)
 
 	if ((tempdir = getenv("TMPDIR")) == NULL)
 		tempdir = _PATH_TMP;
-	if (asprintf(&tempfile, "%s/diff%d.XXXXXXXX", tempdir, n) == -1)
+	if (snprintf(tempfile, sizeof(tempfile), "%s/diff.XXXXXXXX",
+	    tempdir) >= sizeof(tempfile)) {
+		close(ifd);
+		errno = ENAMETOOLONG;
 		return (NULL);
-	tempfiles[n - 1] = tempfile;
-
-	signal(SIGHUP, quit);
-	signal(SIGINT, quit);
-	signal(SIGPIPE, quit);
-	signal(SIGTERM, quit);
-	signal(SIGPIPE, SIG_IGN);
-	ofd = mkstemp(tempfile);
-	if (ofd < 0)
+	}
+
+	if ((ofd = mkstemp(tempfile)) < 0)
 		return (NULL);
-	while ((i = read(ifd, buf, BUFSIZ)) > 0) {
-		if (write(ofd, buf, i) != i)
+	unlink(tempfile);
+	while ((nread = read(ifd, buf, BUFSIZ)) > 0) {
+		if (write(ofd, buf, nread) != nread) {
+			close(ifd);
+			close(ofd);
 			return (NULL);
+		}
 	}
 	close(ifd);
-	close(ofd);
-	return (tempfile);
+	return (fdopen(ofd, "r"));
 }
 
 char *
@@ -539,8 +512,7 @@ splice(char *dir, char *file)
 {
 	char *tail, *buf;
 
-	tail = strrchr(file, '/');
-	if (tail == NULL)
+	if ((tail = strrchr(file, '/')) == NULL)
 		tail = file;
 	else
 		tail++;