Copy editit() from sendbug, synchronizing editor calling code.

OK otto, millert, beck, mbalmer, deraadt.

3 files changed, 189 insertions, 116 deletions

diff --git a/usr.bin/mail/edit.c b/usr.bin/mail/edit.c
index a74e7be4d2c..9ac54cf0481 100644
--- a/usr.bin/mail/edit.c
+++ b/usr.bin/mail/edit.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: edit.c,v 1.14 2006/10/10 21:38:16 cloder Exp $	*/
+/*	$OpenBSD: edit.c,v 1.15 2007/08/31 23:14:21 ray Exp $	*/
 /*	$NetBSD: edit.c,v 1.5 1996/06/08 19:48:20 christos Exp $	*/
 
 /*
@@ -34,14 +34,20 @@
 #if 0
 static const char sccsid[] = "@(#)edit.c	8.1 (Berkeley) 6/6/93";
 #else
-static const char rcsid[] = "$OpenBSD: edit.c,v 1.14 2006/10/10 21:38:16 cloder Exp $";
+static const char rcsid[] = "$OpenBSD: edit.c,v 1.15 2007/08/31 23:14:21 ray Exp $";
 #endif
 #endif /* not lint */
 
+#include <sys/types.h>
+#include <sys/wait.h>
+
 #include "rcv.h"
+#include <errno.h>
 #include <fcntl.h>
 #include "extern.h"
 
+int editit(const char *, const char *);
+
 /*
  * Mail -- a mail program
  *
@@ -184,9 +190,16 @@ run_editor(FILE *fp, off_t size, int type, int readonly)
 		goto out;
 	}
 	nf = NULL;
-	if ((edit = value(type == 'e' ? "EDITOR" : "VISUAL")) == NULL)
-		edit = type == 'e' ? _PATH_EX : _PATH_VI;
-	if (run_command(edit, 0, 0, -1, tempname, NULL, NULL) < 0) {
+	if (type == 'e') {
+		edit = value("EDITOR");
+		if (edit == NULL || edit[0] == '\0')
+			edit = _PATH_EX;
+	} else {
+		edit = value("VISUAL");
+		if (edit == NULL || edit[0] == '\0')
+			edit = _PATH_VI;
+	}
+	if (editit(edit, tempname) == -1) {
 		(void)rm(tempname);
 		goto out;
 	}
@@ -218,3 +231,59 @@ run_editor(FILE *fp, off_t size, int type, int readonly)
 out:
 	return(nf);
 }
+
+/*
+ * Execute an editor on the specified pathname, which is interpreted
+ * from the shell.  This means flags may be included.
+ *
+ * Returns -1 on error, or the exit value on success.
+ */
+int
+editit(const char *ed, const char *pathname)
+{
+	char *argp[] = {"sh", "-c", NULL, NULL}, *p;
+	sig_t sighup, sigint, sigquit;
+	pid_t pid;
+	int saved_errno, st;
+
+	if (ed == NULL)
+		ed = getenv("VISUAL");
+	if (ed == NULL || ed[0] == '\0')
+		ed = getenv("EDITOR");
+	if (ed == NULL || ed[0] == '\0')
+		ed = _PATH_VI;
+	if (asprintf(&p, "%s %s", ed, pathname) == -1)
+		return (-1);
+	argp[2] = p;
+
+	sighup = signal(SIGHUP, SIG_IGN);
+	sigint = signal(SIGINT, SIG_IGN);
+	sigquit = signal(SIGQUIT, SIG_IGN);
+	if ((pid = fork()) == -1)
+		goto fail;
+	if (pid == 0) {
+		execv(_PATH_BSHELL, argp);
+		_exit(127);
+	}
+	while (waitpid(pid, &st, 0) == -1)
+		if (errno != EINTR)
+			goto fail;
+	free(p);
+	(void)signal(SIGHUP, sighup);
+	(void)signal(SIGINT, sigint);
+	(void)signal(SIGQUIT, sigquit);
+	if (!WIFEXITED(st)) {
+		errno = EINTR;
+		return (-1);
+	}
+	return (WEXITSTATUS(st));
+
+ fail:
+	saved_errno = errno;
+	(void)signal(SIGHUP, sighup);
+	(void)signal(SIGINT, sigint);
+	(void)signal(SIGQUIT, sigquit);
+	free(p);
+	errno = saved_errno;
+	return (-1);
+}
diff --git a/usr.sbin/cron/crontab.c b/usr.sbin/cron/crontab.c
index b95e3a2b79c..27d9893a84f 100644
--- a/usr.sbin/cron/crontab.c
+++ b/usr.sbin/cron/crontab.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: crontab.c,v 1.51 2007/02/19 00:08:38 jmc Exp $	*/
+/*	$OpenBSD: crontab.c,v 1.52 2007/08/31 23:14:21 ray Exp $	*/
 
 /* Copyright 1988,1990,1993,1994 by Paul Vixie
  * All rights reserved
@@ -21,7 +21,7 @@
  * OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
-static char const rcsid[] = "$OpenBSD: crontab.c,v 1.51 2007/02/19 00:08:38 jmc Exp $";
+static char const rcsid[] = "$OpenBSD: crontab.c,v 1.52 2007/08/31 23:14:21 ray Exp $";
 
 /* crontab - install and manage per-user crontab files
  * vix 02may87 [RCS has the rest of the log]
@@ -50,6 +50,7 @@ static	FILE		*NewCrontab;
 static	int		CheckErrorCount;
 static	enum opt_t	Option;
 static	struct passwd	*pw;
+int			editit(const char *);
 static	void		list_cmd(void),
 			delete_cmd(void),
 			edit_cmd(void),
@@ -279,7 +280,7 @@ check_error(const char *msg) {
 
 static void
 edit_cmd(void) {
-	char n[MAX_FNAME], q[MAX_TEMPSTR], *editor;
+	char n[MAX_FNAME], q[MAX_TEMPSTR];
 	FILE *f;
 	int ch, t, x;
 	struct stat statbuf, xstatbuf;
@@ -371,11 +372,6 @@ edit_cmd(void) {
 		exit(ERROR_EXIT);
 	}
 
-	if (((editor = getenv("VISUAL")) == NULL || *editor == '\0') &&
-	    ((editor = getenv("EDITOR")) == NULL || *editor == '\0')) {
-		editor = EDITOR;
-	}
-
 	/* we still have the file open.  editors will generally rewrite the
 	 * original file rather than renaming/unlinking it and starting a
 	 * new one; even backup files are supposed to be made by copying
@@ -383,64 +379,11 @@ edit_cmd(void) {
 	 * then don't use it.  the security problems are more severe if we
 	 * close and reopen the file around the edit.
 	 */
-
-	switch (pid = fork()) {
-	case -1:
-		perror("fork");
+	if (editit(Filename) == -1) {
+		warn("error starting editor");
 		goto fatal;
-	case 0:
-		/* child */
-		if (setgid(MY_GID(pw)) < 0) {
-			perror("setgid(getgid())");
-			exit(ERROR_EXIT);
-		}
-		if (chdir(_PATH_TMP) < 0) {
-			perror(_PATH_TMP);
-			exit(ERROR_EXIT);
-		}
-		if (snprintf(q, sizeof q, "%s %s", editor, Filename) >= sizeof(q)) {
-			fprintf(stderr, "%s: editor command line too long\n",
-			    ProgramName);
-			exit(ERROR_EXIT);
-		}
-		execlp(_PATH_BSHELL, _PATH_BSHELL, "-c", q, (char *)NULL);
-		perror(editor);
-		exit(ERROR_EXIT);
-		/*NOTREACHED*/
-	default:
-		/* parent */
-		break;
 	}
 
-	/* parent */
-	for (;;) {
-		xpid = waitpid(pid, &waiter, WUNTRACED);
-		if (xpid == -1) {
-			if (errno != EINTR)
-				fprintf(stderr, "%s: waitpid() failed waiting for PID %ld from \"%s\": %s\n",
-					ProgramName, (long)pid, editor, strerror(errno));
-		} else if (xpid != pid) {
-			fprintf(stderr, "%s: wrong PID (%ld != %ld) from \"%s\"\n",
-				ProgramName, (long)xpid, (long)pid, editor);
-			goto fatal;
-		} else if (WIFSTOPPED(waiter)) {
-			kill(getpid(), WSTOPSIG(waiter));
-		} else if (WIFEXITED(waiter) && WEXITSTATUS(waiter)) {
-			fprintf(stderr, "%s: \"%s\" exited with status %d\n",
-				ProgramName, editor, WEXITSTATUS(waiter));
-			goto fatal;
-		} else if (WIFSIGNALED(waiter)) {
-			fprintf(stderr,
-				"%s: \"%s\" killed; signal %d (%score dumped)\n",
-				ProgramName, editor, WTERMSIG(waiter),
-				WCOREDUMP(waiter) ?"" :"no ");
-			goto fatal;
-		} else
-			break;
-	}
-	(void)signal(SIGHUP, SIG_DFL);
-	(void)signal(SIGINT, SIG_DFL);
-	(void)signal(SIGQUIT, SIG_DFL);
 	if (fstat(t, &statbuf) < 0) {
 		perror("fstat");
 		goto fatal;
@@ -645,6 +588,63 @@ done:
 	return (error);
 }
 
+/*
+ * Execute an editor on the specified pathname, which is interpreted
+ * from the shell.  This means flags may be included.
+ *
+ * Returns -1 on error, or the exit value on success.
+ */
+int
+editit(const char *pathname)
+{
+	char *argp[] = {"sh", "-c", NULL, NULL}, *ed, *p;
+	sig_t sighup, sigint, sigquit;
+	pid_t pid;
+	int saved_errno, st;
+
+	ed = getenv("VISUAL");
+	if (ed == NULL || ed[0] == '\0')
+		ed = getenv("EDITOR");
+	if (ed == NULL || ed[0] == '\0')
+		ed = _PATH_VI;
+	if (asprintf(&p, "%s %s", ed, pathname) == -1)
+		return (-1);
+	argp[2] = p;
+
+	sighup = signal(SIGHUP, SIG_IGN);
+	sigint = signal(SIGINT, SIG_IGN);
+	sigquit = signal(SIGQUIT, SIG_IGN);
+	if ((pid = fork()) == -1)
+		goto fail;
+	if (pid == 0) {
+		setgid(getgid());
+		setuid(getuid());
+		execv(_PATH_BSHELL, argp);
+		_exit(127);
+	}
+	while (waitpid(pid, &st, 0) == -1)
+		if (errno != EINTR)
+			goto fail;
+	free(p);
+	(void)signal(SIGHUP, sighup);
+	(void)signal(SIGINT, sigint);
+	(void)signal(SIGQUIT, sigquit);
+	if (!WIFEXITED(st)) {
+		errno = EINTR;
+		return (-1);
+	}
+	return (WEXITSTATUS(st));
+
+ fail:
+	saved_errno = errno;
+	(void)signal(SIGHUP, sighup);
+	(void)signal(SIGINT, sigint);
+	(void)signal(SIGQUIT, sigquit);
+	free(p);
+	errno = saved_errno;
+	return (-1);
+}
+
 static void
 die(int x) {
 	if (TempFilename[0])
diff --git a/usr.sbin/edquota/edquota.c b/usr.sbin/edquota/edquota.c
index be3f981b3f1..cb04b0c067a 100644
--- a/usr.sbin/edquota/edquota.c
+++ b/usr.sbin/edquota/edquota.c
@@ -38,7 +38,7 @@ static char copyright[] =
 
 #ifndef lint
 /*static char sccsid[] = "from: @(#)edquota.c	8.1 (Berkeley) 6/6/93";*/
-static char *rcsid = "$Id: edquota.c,v 1.46 2007/08/06 19:16:07 sobrado Exp $";
+static char *rcsid = "$Id: edquota.c,v 1.47 2007/08/31 23:14:21 ray Exp $";
 #endif /* not lint */
 
 /*
@@ -81,7 +81,7 @@ int	getentry(char *, int, u_int *);
 struct quotause *
 	getprivs(u_int, int);
 void	putprivs(long, int, struct quotause *);
-int	editit(char *);
+int	editit(const char *);
 int	writeprivs(struct quotause *, int, char *, int);
 int	readprivs(struct quotause *, int);
 int	writetimes(struct quotause *, int, int);
@@ -162,7 +162,11 @@ main(int argc, char *argv[])
 			unlink(tmpfil);
 			exit(1);
 		}
-		if (editit(tmpfil) && readtimes(protoprivs, tmpfd))
+		if (editit(tmpfil) == -1) {
+			unlink(tmpfil);
+			err(1, "error starting editor");
+		}
+		if (readtimes(protoprivs, tmpfd))
 			putprivs(0, quotatype, protoprivs);
 		freeprivs(protoprivs);
 		unlink(tmpfil);
@@ -174,7 +178,11 @@ main(int argc, char *argv[])
 		curprivs = getprivs(id, quotatype);
 		if (writeprivs(curprivs, tmpfd, *argv, quotatype) == 0)
 			continue;
-		if (editit(tmpfil) && readprivs(curprivs, tmpfd))
+		if (editit(tmpfil) == -1) {
+			warn("error starting editor");
+			continue;
+		}
+		if (readprivs(curprivs, tmpfd))
 			putprivs(id, quotatype, curprivs);
 		freeprivs(curprivs);
 	}
@@ -344,62 +352,58 @@ putprivs(long id, int quotatype, struct quotause *quplist)
 }
 
 /*
- * Take a list of privileges and get it edited.
+ * Execute an editor on the specified pathname, which is interpreted
+ * from the shell.  This means flags may be included.
+ *
+ * Returns -1 on error, or the exit value on success.
  */
 int
-editit(char *tmpfile)
+editit(const char *pathname)
 {
-	pid_t pid, xpid;
-	char *argp[] = {"sh", "-c", NULL, NULL};
-	char *ed, *p;
-	sigset_t mask, omask;
-	int stat;
-
-	if ((ed = getenv("EDITOR")) == (char *)0)
+	char *argp[] = {"sh", "-c", NULL, NULL}, *ed, *p;
+	sig_t sighup, sigint, sigquit;
+	pid_t pid;
+	int saved_errno, st;
+
+	ed = getenv("VISUAL");
+	if (ed == NULL || ed[0] == '\0')
+		ed = getenv("EDITOR");
+	if (ed == NULL || ed[0] == '\0')
 		ed = _PATH_VI;
-	if (asprintf(&p, "%s %s", ed, tmpfile) == -1)
-		return (0);
+	if (asprintf(&p, "%s %s", ed, pathname) == -1)
+		return (-1);
 	argp[2] = p;
 
-	sigemptyset(&mask);
-	sigaddset(&mask, SIGINT);
-	sigaddset(&mask, SIGQUIT);
-	sigaddset(&mask, SIGHUP);
-	sigprocmask(SIG_SETMASK, &mask, &omask);
- top:
-	if ((pid = fork()) < 0) {
-		if (errno == EPROCLIM) {
-			warnx("you have too many processes");
-			free(p);
-			return(0);
-		}
-		if (errno == EAGAIN) {
-			sleep(1);
-			goto top;
-		}
-		perror("fork");
-		free(p);
-		return(0);
-	}
+	sighup = signal(SIGHUP, SIG_IGN);
+	sigint = signal(SIGINT, SIG_IGN);
+	sigquit = signal(SIGQUIT, SIG_IGN);
+	if ((pid = fork()) == -1)
+		goto fail;
 	if (pid == 0) {
-		sigprocmask(SIG_SETMASK, &omask, NULL);
-		setgid(getgid());
-		setuid(getuid());
 		execv(_PATH_BSHELL, argp);
 		_exit(127);
 	}
+	while (waitpid(pid, &st, 0) == -1)
+		if (errno != EINTR)
+			goto fail;
 	free(p);
-	for (;;) {
-		xpid = waitpid(pid, (int *)&stat, WUNTRACED);
-		if (WIFSTOPPED(stat))
-			raise(WSTOPSIG(stat));
-		else if (WIFEXITED(stat))
-			break;
+	(void)signal(SIGHUP, sighup);
+	(void)signal(SIGINT, sigint);
+	(void)signal(SIGQUIT, sigquit);
+	if (!WIFEXITED(st)) {
+		errno = EINTR;
+		return (-1);
 	}
-	sigprocmask(SIG_SETMASK, &omask, NULL);
-	if (!WIFEXITED(stat) || WEXITSTATUS(stat) != 0)
-		return(0);
-	return(1);
+	return (WEXITSTATUS(st));
+
+ fail:
+	saved_errno = errno;
+	(void)signal(SIGHUP, sighup);
+	(void)signal(SIGINT, sigint);
+	(void)signal(SIGQUIT, sigquit);
+	free(p);
+	errno = saved_errno;
+	return (-1);
 }
 
 /*