add command sequence support

5 files changed, 305 insertions, 85 deletions

diff --git a/ChangeLog b/ChangeLog
index 9ec96d23..18fce2de 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,24 @@
+Sun Aug 23 17:29:06 BST 2009  Colin Watson  <cjwatson@debian.org>
+
+	* include/manconfig.h.in (ATTRIBUTE_NORETURN): Define to an
+	  attribute marking a function as non-returning if using GCC 2.5 or
+	  newer.
+	* lib/pipeline.c (command_new_sequence, command_sequence_command):
+	  New functions.
+	  (command_dup, command_dump, command_tostring, command_free):
+	  Handle commands of type COMMAND_SEQUENCE.
+	  (pipeline_start): Move command execution to ...
+	  (command_start_child): ... here (new function). Handle commands of
+	  type COMMAND_SEQUENCE.
+	* lib/pipeline.h (enum command_tag): Add COMMAND_SEQUENCE.
+	  (struct command): Add support for commands that consist of a
+	  sequence of commands.
+	  (command_new_sequence, command_sequence_command): Add prototypes.
+	* src/man.c (disable_hyphenation, locale_macros): Drop passthrough
+	  code.
+	  (display): Create a command sequence for hyphenation and
+	  locale-macro decompressor prefixes if necessary.
+
 Sun Aug 23 15:49:45 BST 2009  Colin Watson  <cjwatson@debian.org>
 
 	* lib/pipeline.c (command_dup): Fix newcmd->nenv assertion.
diff --git a/include/manconfig.h.in b/include/manconfig.h.in
index a7c7a87c..6407ca41 100644
--- a/include/manconfig.h.in
+++ b/include/manconfig.h.in
@@ -255,6 +255,13 @@
 #  define ATTRIBUTE_UNUSED
 #endif
 
+/* Does this compiler support marking functions as non-returning? */
+#if GNUC_PREREQ(2,5)
+#  define ATTRIBUTE_NORETURN __attribute__ ((__noreturn__))
+#else
+#  define ATTRIBUTE_NORETURN
+#endif
+
 /* Does this compiler support malloc return checking? */
 #if GNUC_PREREQ(2,96)
 #  define ATTRIBUTE_MALLOC __attribute__ ((__malloc__))
diff --git a/lib/pipeline.c b/lib/pipeline.c
index b7ebf658..5dbe7d57 100644
--- a/lib/pipeline.c
+++ b/lib/pipeline.c
@@ -272,6 +272,39 @@ command *command_new_function (const char *name,
 	return cmd;
 }
 
+command *command_new_sequence (const char *name, ...)
+{
+	command *cmd = XMALLOC (command);
+	struct command_sequence *cmds;
+	va_list cmdv;
+	command *child;
+
+	cmd->tag = COMMAND_SEQUENCE;
+	cmd->name = xstrdup (name);
+	cmd->nice = 0;
+	cmd->discard_err = 0;
+
+	cmd->nenv = 0;
+	cmd->env_max = 4;
+	cmd->env = xnmalloc (cmd->env_max, sizeof *cmd->env);
+
+	cmds = &cmd->u.sequence;
+
+	cmds->ncommands = 0;
+	cmds->commands_max = 4;
+	cmds->commands = xnmalloc (cmds->commands_max, sizeof *cmds->commands);
+
+	va_start (cmdv, name);
+	child = va_arg (cmdv, command *);
+	while (child) {
+		command_sequence_command (cmd, child);
+		child = va_arg (cmdv, command *);
+	}
+	va_end (cmdv);
+
+	return cmd;
+}
+
 command *command_dup (command *cmd)
 {
 	command *newcmd = XMALLOC (command);
@@ -320,6 +353,24 @@ command *command_dup (command *cmd)
 
 			break;
 		}
+
+		case COMMAND_SEQUENCE: {
+			struct command_sequence *cmds = &cmd->u.sequence;
+			struct command_sequence *newcmds = &newcmd->u.sequence;
+
+			newcmds->ncommands = cmds->ncommands;
+			newcmds->commands_max = cmds->commands_max;
+			assert (newcmds->ncommands <= newcmds->commands_max);
+			newcmds->commands = xmalloc
+				(newcmds->commands_max *
+				 sizeof *newcmds->commands);
+
+			for (i = 0; i < cmds->ncommands; ++i)
+				newcmds->commands[i] =
+					command_dup (cmds->commands[i]);
+
+			break;
+		}
 	}
 
 	return newcmd;
@@ -391,6 +442,23 @@ void command_setenv (command *cmd, const char *name, const char *value)
 	++cmd->nenv;
 }
 
+void command_sequence_command (command *cmd, command *child)
+{
+	struct command_sequence *cmds;
+
+	assert (cmd->tag == COMMAND_SEQUENCE);
+	cmds = &cmd->u.sequence;
+
+	if (cmds->ncommands >= cmds->commands_max) {
+		cmds->commands_max *= 2;
+		cmds->commands = xrealloc
+			(cmds->commands,
+			 cmds->commands_max * sizeof *cmds->commands);
+	}
+
+	cmds->commands[cmds->ncommands++] = child;
+}
+
 void command_dump (command *cmd, FILE *stream)
 {
 	int i;
@@ -416,6 +484,20 @@ void command_dump (command *cmd, FILE *stream)
 		case COMMAND_FUNCTION:
 			fputs (cmd->name, stream);
 			break;
+
+		case COMMAND_SEQUENCE: {
+			struct command_sequence *cmds = &cmd->u.sequence;
+
+			putc ('(', stream);
+			for (i = 0; i < cmds->ncommands; ++i) {
+				command_dump (cmds->commands[i], stream);
+				if (i < cmds->ncommands - 1)
+					fputs (" && ", stream);
+			}
+			putc (')', stream);
+
+			break;
+		}
 	}
 }
 
@@ -444,11 +526,161 @@ char *command_tostring (command *cmd)
 		case COMMAND_FUNCTION:
 			out = appendstr (out, cmd->name, NULL);
 			break;
+
+		case COMMAND_SEQUENCE: {
+			struct command_sequence *cmds = &cmd->u.sequence;
+
+			out = appendstr (out, "(", NULL);
+			for (i = 0; i < cmds->ncommands; ++i) {
+				char *subout = command_tostring
+					(cmds->commands[i]);
+				out = appendstr (out, subout, NULL);
+				free (subout);
+				if (i < cmds->ncommands - 1)
+					out = appendstr (out, " && ", NULL);
+			}
+			out = appendstr (out, ")", NULL);
+
+			break;
+		}
 	}
 
 	return out;
 }
 
+/* Children exit with this status if execvp fails. */
+#define EXEC_FAILED_EXIT_STATUS 0xff
+
+/* Start a command. This is called in the forked child process, with file
+ * descriptors already set up.
+ */
+static void command_start_child (command *cmd) ATTRIBUTE_NORETURN;
+static void command_start_child (command *cmd)
+{
+	int i;
+
+	if (cmd->nice)
+		if (nice (cmd->nice) < 0)
+			/* Don't worry too much. */
+			debug ("nice failed: %s", strerror (errno));
+
+	if (cmd->discard_err) {
+		int devnull = open ("/dev/null", O_WRONLY);
+		if (devnull != -1) {
+			dup2 (devnull, 2);
+			close (devnull);
+		}
+	}
+
+	for (i = 0; i < cmd->nenv; ++i)
+		setenv (cmd->env[i].name, cmd->env[i].value, 1);
+
+	switch (cmd->tag) {
+		case COMMAND_PROCESS: {
+			struct command_process *cmdp = &cmd->u.process;
+			execvp (cmd->name, cmdp->argv);
+			break;
+		}
+
+		/* TODO: ideally, could there be a facility
+		 * to execute non-blocking functions without
+		 * needing to fork?
+		 */
+		case COMMAND_FUNCTION: {
+			struct command_function *cmdf = &cmd->u.function;
+			(*cmdf->func) (cmdf->data);
+			/* pacify valgrind et al */
+			if (cmdf->free_func)
+				(*cmdf->free_func) (cmdf->data);
+			exit (0);
+		}
+
+		case COMMAND_SEQUENCE: {
+			struct command_sequence *cmds = &cmd->u.sequence;
+			struct sigaction sa;
+
+			/* pipeline_start will have blocked SIGCHLD. We like
+			 * it that way. Lose the parent's signal handler,
+			 * though.
+			 */
+			memset (&sa, 0, sizeof sa);
+			sa.sa_handler = SIG_DFL;
+			sigemptyset (&sa.sa_mask);
+			sa.sa_flags = 0;
+			if (sigaction (SIGCHLD, &sa, NULL) == -1)
+				error (FATAL, errno,
+				       _("can't install SIGCHLD handler"));
+
+			for (i = 0; i < cmds->ncommands; ++i) {
+				command *child = cmds->commands[i];
+				pid_t pid = fork ();
+				int status;
+
+				if (pid < 0)
+					error (FATAL, errno, _("fork failed"));
+				if (pid == 0)
+					command_start_child (child);
+				debug ("Started \"%s\", pid %d\n",
+				       child->name, pid);
+
+				while (waitpid (pid, &status, 0) < 0) {
+					if (errno == EINTR)
+						continue;
+					error (FATAL, errno,
+					       _("waitpid failed"));
+				}
+
+				debug ("  \"%s\" (%d) -> %d\n",
+				       child->name, pid, status);
+
+				if (WIFSIGNALED (status)) {
+					int sig = WTERMSIG (status);
+#ifdef SIGPIPE
+					if (sig == SIGPIPE)
+						status = 0;
+					else
+#endif /* SIGPIPE */
+					if (WCOREDUMP (status))
+						error (0, 0,
+						       _("%s: %s "
+							 "(core dumped)"),
+						       child->name,
+						       strsignal (sig));
+					else
+						error (0, 0, _("%s: %s"),
+						       child->name,
+						       strsignal (sig));
+				} else if (!WIFEXITED (status))
+					error (0, 0, "unexpected status %d",
+					       status);
+
+				if (child->tag == COMMAND_FUNCTION) {
+					struct command_function *cmdf =
+						&child->u.function;
+					if (cmdf->free_func)
+						(*cmdf->free_func)
+							(cmdf->data);
+				}
+
+				if (WIFSIGNALED (status)) {
+					raise (WTERMSIG (status));
+					exit (1); /* just to make sure */
+				} else if (status && WIFEXITED (status))
+					exit (WEXITSTATUS (status));
+			}
+
+			exit (0);
+		}
+	}
+
+	error (EXEC_FAILED_EXIT_STATUS, errno,
+	       _("can't execute %s"), cmd->name);
+	/* Never called, but gcc doesn't realise that error with non-zero
+	 * status always exits.
+	 */
+	exit (EXEC_FAILED_EXIT_STATUS);
+}
+
 void command_free (command *cmd)
 {
 	int i;
@@ -477,6 +709,16 @@ void command_free (command *cmd)
 
 		case COMMAND_FUNCTION:
 			break;
+
+		case COMMAND_SEQUENCE: {
+			struct command_sequence *cmds = &cmd->u.sequence;
+
+			for (i = 0; i < cmds->ncommands; ++i)
+				command_free (cmds->commands[i]);
+			free (cmds->commands);
+
+			break;
+		}
 	}
 
 	free (cmd);
@@ -754,9 +996,6 @@ static int n_active_pipelines = 0, max_active_pipelines = 0;
 static int ignored_signals = 0;
 static struct sigaction osa_sigint, osa_sigquit;
 
-/* Children exit with this status if execvp fails. */
-#define EXEC_FAILED_EXIT_STATUS 0xff
-
 void pipeline_start (pipeline *p)
 {
 	int i, j;
@@ -932,57 +1171,14 @@ void pipeline_start (pipeline *p)
 					close (active->outfd);
 			}
 
-			if (p->commands[i]->nice)
-				if (nice (p->commands[i]->nice) < 0)
-					/* Don't worry too much. */
-					debug ("nice failed: %s",
-					       strerror (errno));
-
-			if (p->commands[i]->discard_err) {
-				int devnull = open ("/dev/null", O_WRONLY);
-				if (devnull != -1) {
-					dup2 (devnull, 2);
-					close (devnull);
-				}
-			}
-
-			for (j = 0; j < p->commands[i]->nenv; ++j)
-				setenv (p->commands[i]->env[j].name,
-					p->commands[i]->env[j].value, 1);
-
 			/* Restore signals. */
 			if (p->ignore_signals) {
 				sigaction (SIGINT, &osa_sigint, NULL);
 				sigaction (SIGQUIT, &osa_sigquit, NULL);
 			}
 
-			switch (p->commands[i]->tag) {
-				case COMMAND_PROCESS: {
-					struct command_process *cmdp =
-						&p->commands[i]->u.process;
-					execvp (p->commands[i]->name,
-						cmdp->argv);
-					break;
-				}
-
-				/* TODO: ideally, could there be a facility
-				 * to execute non-blocking functions without
-				 * needing to fork?
-				 */
-				case COMMAND_FUNCTION: {
-					struct command_function *cmdf =
-						&p->commands[i]->u.function;
-					(*cmdf->func) (cmdf->data);
-					/* pacify valgrind et al */
-					if (cmdf->free_func)
-						(*cmdf->free_func)
-							(cmdf->data);
-					exit (0);
-				}
-			}
-
-			error (EXEC_FAILED_EXIT_STATUS, errno,
-			       _("can't execute %s"), p->commands[i]->name);
+			command_start_child (p->commands[i]);
+			/* never returns */
 		}
 
 		/* in the parent */
diff --git a/lib/pipeline.h b/lib/pipeline.h
index f880b898..f4909381 100644
--- a/lib/pipeline.h
+++ b/lib/pipeline.h
@@ -30,7 +30,8 @@
 
 enum command_tag {
 	COMMAND_PROCESS,
-	COMMAND_FUNCTION
+	COMMAND_FUNCTION,
+	COMMAND_SEQUENCE
 };
 
 typedef void command_function_type (void *);
@@ -60,6 +61,11 @@ typedef struct command {
 			command_function_free_type *free_func;
 			void *data;
 		} function;
+		struct command_sequence {
+			int ncommands;
+			int commands_max;
+			struct command **commands;
+		} sequence;
 	} u;
 } command;
 
@@ -157,6 +163,15 @@ command *command_new_function (const char *name,
 			       command_function_free_type *free_func,
 			       void *data);
 
+/* Construct a new command that runs a sequence of commands. The commands
+ * will be executed in forked children; if any exits non-zero then it will
+ * terminate the sequence, as with "&&" in shell.
+ *
+ * command_* functions that deal with arguments cannot be used with the
+ * command returned by this function.
+ */
+command *command_new_sequence (const char *name, ...) ATTRIBUTE_SENTINEL;
+
 /* Return a duplicate of a command. */
 command *command_dup (command *cmd);
 
@@ -180,6 +195,9 @@ void command_argstr (command *cmd, const char *argstr);
 /* Set an environment variable while running this command. */
 void command_setenv (command *cmd, const char *name, const char *value);
 
+/* Add a command to a sequence. */
+void command_sequence_command (command *cmd, command *child);
+
 /* Dump a string representation of a command to stream. */
 void command_dump (command *cmd, FILE *stream);
 
diff --git a/src/man.c b/src/man.c
index a380e87e..1b443c2f 100644
--- a/src/man.c
+++ b/src/man.c
@@ -2219,35 +2219,14 @@ static void display_catman (const char *cat_file, pipeline *decomp,
 	free (tmpcat);
 }
 
-/* TODO: This function would be more efficient if the disabling sequence
- * were simply written in sequence before starting the decompressor.
- * However, that requires a new COMMAND_SEQUENCE type in the pipeline
- * library. If that is ever needed for another reason, then this function
- * should be rewritten using it.
- */
 static void disable_hyphenation (void *data ATTRIBUTE_UNUSED)
 {
 	fputs (".nh\n"
 	       ".de hy\n"
 	       "..\n", stdout);
-
-	for (;;) {
-		char buffer[4096];
-		int r = read (STDIN_FILENO, buffer, 4096);
-		if (r <= 0)
-			break;
-		if (fwrite (buffer, 1, (size_t) r, stdout) < (size_t) r)
-			break;
-	}
 }
 
 #ifdef TROFF_IS_GROFF
-/* TODO: This function would be more efficient if the macro requests were
- * simply written in sequence before starting the decompressor. However,
- * that requires a new COMMAND_SEQUENCE type in the pipeline library. If
- * that is ever needed for another reason, then this function should be
- * rewritten using it.
- */
 static void locale_macros (void *data)
 {
 	printf (
@@ -2263,15 +2242,6 @@ static void locale_macros (void *data)
 		/*   and load the appropriate per-locale macros */
 		".  mso %s.tmac\n"
 		".\\}\n", (const char *) data);
-
-	for (;;) {
-		char buffer[4096];
-		int r = read (STDIN_FILENO, buffer, 4096);
-		if (r <= 0)
-			break;
-		if (fwrite (buffer, 1, (size_t) r, stdout) < (size_t) r)
-			break;
-	}
 }
 #endif /* TROFF_IS_GROFF */
 
@@ -2305,6 +2275,8 @@ static int display (const char *dir, const char *man_file,
 
 	/* define format_cmd */
 	if (man_file) {
+		command *seq = command_new_sequence ("decompressor", NULL);
+
 		if (*man_file)
 			decomp = decompress_open (man_file);
 		else
@@ -2312,9 +2284,9 @@ static int display (const char *dir, const char *man_file,
 
 		if (no_hyphenation) {
 			command *hcmd = command_new_function (
-				"(echo .nh && echo .de hy && echo .. && cat)",
+				"echo .nh && echo .de hy && echo ..",
 				disable_hyphenation, NULL, NULL);
-			pipeline_command (decomp, hcmd);
+			command_sequence_command (seq, hcmd);
 		}
 
 #ifdef TROFF_IS_GROFF
@@ -2331,17 +2303,23 @@ static int display (const char *dir, const char *man_file,
 				command *lcmd;
 
 				unpack_locale_bits (page_lang, &bits);
-				name = xasprintf (
-					"(echo .mso %s.tmac && "
-					"cat)", bits.language);
+				name = xasprintf ("echo .mso %s.tmac",
+						  bits.language);
 				lcmd = command_new_function (
 					name, locale_macros, free, page_lang);
-				pipeline_command (decomp, lcmd);
+				command_sequence_command (seq, lcmd);
 				free (name);
 				free_locale_bits (&bits);
 			}
 		}
 #endif /* TROFF_IS_GROFF */
+
+		if (seq->u.sequence.ncommands) {
+			assert (decomp->ncommands == 1);
+			command_sequence_command (seq, decomp->commands[0]);
+			decomp->commands[0] = seq;
+		} else
+			command_free (seq);
 	}
 
 	if (decomp) {