If there is an error in the configuration file, don't just exit(1) as this can

cause the client to hang. Instead, send the error message, then mark the client
as bad and start a normal shutdown so the server exits once the error is
written.

This also allows some code duplicating daemon(3) to be trimmed and logging to
begin earlier.

Prompted by Theo noticing the behaviour on error wasn't documented.

4 files changed, 56 insertions, 42 deletions

diff --git a/usr.bin/tmux/server-fn.c b/usr.bin/tmux/server-fn.c
index 50616517f5c..761d137d518 100644
--- a/usr.bin/tmux/server-fn.c
+++ b/usr.bin/tmux/server-fn.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: server-fn.c,v 1.11 2009/07/29 14:17:26 nicm Exp $ */
+/* $OpenBSD: server-fn.c,v 1.12 2009/08/07 15:39:10 nicm Exp $ */
 
 /*
  * Copyright (c) 2007 Nicholas Marriott <nicm@users.sourceforge.net>
@@ -62,6 +62,8 @@ server_write_client(
 {
 	struct hdr	 hdr;
 
+	if (c->flags & CLIENT_BAD)
+		return;
 	log_debug("writing %d to client %d", type, c->fd);
 
 	hdr.type = type;
diff --git a/usr.bin/tmux/server.c b/usr.bin/tmux/server.c
index 48ea7660f7f..ae06d571913 100644
--- a/usr.bin/tmux/server.c
+++ b/usr.bin/tmux/server.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: server.c,v 1.15 2009/07/28 07:03:32 nicm Exp $ */
+/* $OpenBSD: server.c,v 1.16 2009/08/07 15:39:10 nicm Exp $ */
 
 /*
  * Copyright (c) 2007 Nicholas Marriott <nicm@users.sourceforge.net>
@@ -131,9 +131,10 @@ server_client_index(struct client *c)
 int
 server_start(char *path)
 {
-	int	pair[2], srv_fd, null_fd;
-	char   *cause;
-	char	rpathbuf[MAXPATHLEN];
+	struct client	*c;
+	int		 pair[2], srv_fd;
+	char		*cause;
+	char		 rpathbuf[MAXPATHLEN];
 
 	/* The first client is special and gets a socketpair; create it. */
 	if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, pair) != 0)
@@ -154,9 +155,12 @@ server_start(char *path)
 	 * Must daemonise before loading configuration as the PID changes so
 	 * $TMUX would be wrong for sessions created in the config file.
 	 */
-	if (daemon(1, 1) != 0)
+	if (daemon(1, 0) != 0)
 		fatal("daemon failed");
 
+	logfile("server");
+	log_debug("server started, pid %ld", (long) getpid());
+
 	ARRAY_INIT(&windows);
 	ARRAY_INIT(&clients);
 	ARRAY_INIT(&sessions);
@@ -171,44 +175,36 @@ server_start(char *path)
 	start_time = time(NULL);
 	socket_path = path;
 
+	if (realpath(socket_path, rpathbuf) == NULL)
+		strlcpy(rpathbuf, socket_path, sizeof rpathbuf);
+	log_debug("socket path %s", socket_path);
+	setproctitle("server (%s)", rpathbuf);
+
+	srv_fd = server_create_socket();
+	server_create_client(pair[1]);
+
 	if (access(SYSTEM_CFG, R_OK) != 0) {
 		if (errno != ENOENT) {
-			log_warn("%s", SYSTEM_CFG);
-			exit(1);
-		}
-	} else {
-		if (load_cfg(SYSTEM_CFG, &cause) != 0) {
-			log_warnx("%s", cause);
-			exit(1);
+			xasprintf(
+			    &cause, "%s: %s", strerror(errno), SYSTEM_CFG);
+			goto error;
 		}
-	}
-	if (cfg_file != NULL && load_cfg(cfg_file, &cause) != 0) {
-		log_warnx("%s", cause);
-		exit(1);
-	}
-	logfile("server");
+	} else if (load_cfg(SYSTEM_CFG, &cause) != 0)
+		goto error;
+	if (cfg_file != NULL && load_cfg(cfg_file, &cause) != 0)
+		goto error;
 
-	/*
-	 * Close stdin/stdout/stderr. Can't let daemon() do this as they are
-	 * needed until now to print configuration file errors.
-	 */
-        if ((null_fd = open(_PATH_DEVNULL, O_RDWR)) != -1) {
-                dup2(null_fd, STDIN_FILENO);
-                dup2(null_fd, STDOUT_FILENO);
-                dup2(null_fd, STDERR_FILENO);
-                if (null_fd > 2)
-                        close(null_fd);
-        }
+	exit(server_main(srv_fd));
 
-	log_debug("server started, pid %ld", (long) getpid());
-	log_debug("socket path %s", socket_path);
+error:
+	/* Write the error and shutdown the server. */
+	c = ARRAY_FIRST(&clients);
 
-	if (realpath(socket_path, rpathbuf) == NULL)
-		strlcpy(rpathbuf, socket_path, sizeof rpathbuf);
-	setproctitle("server (%s)", rpathbuf);
+	server_write_error(c, cause);
+	xfree(cause);
 
-	srv_fd = server_create_socket();
-	server_create_client(pair[1]);
+	server_shutdown();
+	c->flags |= CLIENT_BAD;
 
 	exit(server_main(srv_fd));
 }
@@ -419,8 +415,13 @@ server_shutdown(void)
 
 	for (i = 0; i < ARRAY_LENGTH(&clients); i++) {
 		c = ARRAY_ITEM(&clients, i);
-		if (c != NULL)
-			server_write_client(c, MSG_SHUTDOWN, NULL, 0);
+		if (c != NULL) {
+			if (c->flags & CLIENT_BAD)
+				server_lost_client(c);
+			else
+				server_write_client(c, MSG_SHUTDOWN, NULL, 0);
+			c->flags |= CLIENT_BAD;
+		}
 	}
 }
 
@@ -672,7 +673,8 @@ server_fill_clients(struct pollfd **pfd)
 			(*pfd)->fd = -1;
 		else {
 			(*pfd)->fd = c->fd;
-			(*pfd)->events = POLLIN;
+			if (!(c->flags & CLIENT_BAD))
+				(*pfd)->events = POLLIN;
 			if (BUFFER_USED(c->out) > 0)
 				(*pfd)->events |= POLLOUT;
 		}
@@ -720,6 +722,11 @@ server_handle_clients(struct pollfd **pfd)
 				server_lost_client(c);
 				(*pfd) += 2;
 				continue;
+			} else if (c->flags & CLIENT_BAD) {
+				if (BUFFER_USED(c->out) == 0)
+					server_lost_client(c);
+				(*pfd) += 2;
+				continue;			
 			} else
 				server_msg_dispatch(c);
 		}
diff --git a/usr.bin/tmux/tmux.1 b/usr.bin/tmux/tmux.1
index a7a4dca7f5c..a531c21554c 100644
--- a/usr.bin/tmux/tmux.1
+++ b/usr.bin/tmux/tmux.1
@@ -1,4 +1,4 @@
-.\" $OpenBSD: tmux.1,v 1.60 2009/08/07 12:24:16 nicm Exp $
+.\" $OpenBSD: tmux.1,v 1.61 2009/08/07 15:39:10 nicm Exp $
 .\"
 .\" Copyright (c) 2007 Nicholas Marriott <nicm@users.sourceforge.net>
 .\"
@@ -120,6 +120,10 @@ if present, then looks for a user configuration file at
 The configuration file is a set of
 .Nm
 commands which are executed in sequence when the server is first started.
+.Pp
+If a command in the configuration file fails,
+.Nm
+will report an error and exit without executing further commands.
 .It Fl L Ar socket-name
 .Nm
 stores the server socket in a directory under
diff --git a/usr.bin/tmux/tmux.h b/usr.bin/tmux/tmux.h
index ad454d6186d..fd93b884fa8 100644
--- a/usr.bin/tmux/tmux.h
+++ b/usr.bin/tmux/tmux.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: tmux.h,v 1.69 2009/08/07 00:12:13 nicm Exp $ */
+/* $OpenBSD: tmux.h,v 1.70 2009/08/07 15:39:10 nicm Exp $ */
 
 /*
  * Copyright (c) 2007 Nicholas Marriott <nicm@users.sourceforge.net>
@@ -900,6 +900,7 @@ struct client {
 #define CLIENT_STATUS 0x10
 #define CLIENT_REPEAT 0x20	/* allow command to repeat within repeat time */
 #define CLIENT_SUSPENDED 0x40
+#define CLIENT_BAD 0x80
 	int		 flags;
 
 	char		*message_string;