1 file changed, 52 insertions, 68 deletions
diff --git a/source/filter.c b/source/filter.c
index f2dfe12..6cc5e80 100644
--- a/source/filter.c
+++ b/source/filter.c
@@ -25,6 +25,31 @@
 // The exec filter holding stdout, for cgit_abort_filters to take it back.
 static struct cgit_exec_filter *running_exec;
 
+/*
+ * Each of these returns its argument untouched when the syscall it wraps
+ * succeeded, and dies with the message and errno otherwise.
+ */
+static int die_unless_zero(int result, const char *msg)
+{
+	if (result != 0)
+		die_errno("%s", msg);
+	return result;
+}
+
+static int die_unless_positive(int result, const char *msg)
+{
+	if (result <= 0)
+		die_errno("%s", msg);
+	return result;
+}
+
+static int die_unless_non_negative(int result, const char *msg)
+{
+	if (result < 0)
+		die_errno("%s", msg);
+	return result;
+}
+
 static int open_exec_filter(struct cgit_filter *base, va_list ap)
 {
 	struct cgit_exec_filter *filter = (struct cgit_exec_filter *)base;
@@ -34,12 +59,12 @@ static int open_exec_filter(struct cgit_filter *base, va_list ap)
 	for (i = 0; i < filter->base.argument_count; i++)
 		filter->argv[i + 1] = va_arg(ap, char *);
 
-	filter->old_stdout = cgit_die_unless_positive(dup(STDOUT_FILENO), "Unable to duplicate stdout");
+	filter->old_stdout = die_unless_positive(dup(STDOUT_FILENO), "Unable to duplicate stdout");
 	// The child keeps the page's stdout, but not the descriptor the parent
 	// needs it back from.
 	fcntl(filter->old_stdout, F_SETFD, FD_CLOEXEC);
-	cgit_die_unless_zero(pipe(pipefd), "Unable to create pipe to subprocess");
-	filter->pid = cgit_die_unless_non_negative(fork(), "Unable to create subprocess");
+	die_unless_zero(pipe(pipefd), "Unable to create pipe to subprocess");
+	filter->pid = die_unless_non_negative(fork(), "Unable to create subprocess");
 	if (filter->pid == 0) {
 		close(pipefd[1]);
 		if (dup2(pipefd[0], STDIN_FILENO) < 0)
@@ -55,7 +80,7 @@ static int open_exec_filter(struct cgit_filter *base, va_list ap)
 		_exit(EXEC_FAILED);
 	}
 	close(pipefd[0]);
-	cgit_die_unless_non_negative(dup2(pipefd[1], STDOUT_FILENO), "Unable to use pipe as stdout");
+	die_unless_non_negative(dup2(pipefd[1], STDOUT_FILENO), "Unable to use pipe as stdout");
 	close(pipefd[1]);
 	running_exec = filter;
 	return 0;
@@ -64,22 +89,16 @@ static int open_exec_filter(struct cgit_filter *base, va_list ap)
 static int close_exec_filter(struct cgit_filter *base)
 {
 	struct cgit_exec_filter *filter = (struct cgit_exec_filter *)base;
-	int i, exit_status = 0;
+	int exit_status = 0;
 
 	running_exec = NULL;
-	cgit_die_unless_non_negative(dup2(filter->old_stdout, STDOUT_FILENO), "Unable to restore stdout");
+	die_unless_non_negative(dup2(filter->old_stdout, STDOUT_FILENO), "Unable to restore stdout");
 	close(filter->old_stdout);
-	if (filter->pid < 0)
-		goto done;
 	waitpid(filter->pid, &exit_status, 0);
 	if (!WIFEXITED(exit_status))
 		die("Subprocess %s exited abnormally", filter->cmd);
 	if (WEXITSTATUS(exit_status) == EXEC_FAILED)
 		die("Unable to run filter %s", filter->cmd);
-
-done:
-	for (i = 0; i < filter->base.argument_count; i++)
-		filter->argv[i + 1] = NULL;
 	return WEXITSTATUS(exit_status);
 }
 
@@ -103,16 +122,13 @@ static void cleanup_exec_filter(struct cgit_filter *base)
 static struct cgit_filter *new_exec_filter(const char *cmd, int argument_count)
 {
 	struct cgit_exec_filter *filter;
-	int argv_size;
 
 	filter = xmalloc(sizeof(*filter));
 	cgit_exec_filter_init(filter, cgit_strdup_first_line(cmd), NULL);
 	filter->base.argument_count = argument_count;
 	// argv is the command, then a slot per argument, then the NULL that
 	// execvp needs to find the end.
-	argv_size = (2 + argument_count) * sizeof(char *);
-	filter->argv = xmalloc(argv_size);
-	memset(filter->argv, 0, argv_size);
+	filter->argv = xcalloc(2 + argument_count, sizeof(char *));
 	filter->argv[0] = filter->cmd;
 	return &filter->base;
 }
@@ -126,15 +142,8 @@ void cgit_exec_filter_init(struct cgit_exec_filter *filter, const char *cmd, con
 	filter->base.cleanup = cleanup_exec_filter;
 	filter->cmd = cmd;
 	filter->argv = argv;
-	filter->base.argument_count = 0;
 }
 
-#ifdef NO_LUA
-void cgit_init_filters(void)
-{
-}
-#endif
-
 #ifndef NO_LUA
 struct lua_filter {
 	struct cgit_filter base;
@@ -279,19 +288,9 @@ static int script_html_url_arg(lua_State *lua_state)
 	return emit_unfiltered(lua_state, html_url_arg);
 }
 
-/*
- * html_include returns whether it found the file, and calling it through a
- * pointer that claims it returns nothing would be undefined, so the result is
- * dropped in a wrapper of the right shape.
- */
-static void include_and_discard_result(const char *filename)
-{
-	html_include(filename);
-}
-
 static int script_html_include(lua_State *lua_state)
 {
-	return emit_unfiltered(lua_state, include_and_discard_result);
+	return emit_unfiltered(lua_state, html_include);
 }
 
 static const struct {
@@ -383,8 +382,7 @@ static struct cgit_filter *new_lua_filter(const char *cmd, int argument_count)
 {
 	struct lua_filter *filter;
 
-	filter = xmalloc(sizeof(*filter));
-	memset(filter, 0, sizeof(*filter));
+	filter = xcalloc(1, sizeof(*filter));
 	filter->base.open = open_lua_filter;
 	filter->base.close = close_lua_filter;
 	filter->base.fprintfp = fprintf_lua_filter;
@@ -394,6 +392,10 @@ static struct cgit_filter *new_lua_filter(const char *cmd, int argument_count)
 
 	return &filter->base;
 }
+#else
+void cgit_init_filters(void)
+{
+}
 #endif
 
 int cgit_open_filter(struct cgit_filter *filter, ...)
@@ -477,35 +479,14 @@ int cgit_filter_holds_stdout(void)
 	return running_exec != NULL;
 }
 
-static const struct {
-	const char *prefix;
-	struct cgit_filter *(*create)(const char *cmd, int argument_count);
-} filter_specs[] = {
-	{ "exec", new_exec_filter },
-#ifndef NO_LUA
-	{ "lua", new_lua_filter },
-#endif
-};
-
 struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filtertype)
 {
-	const char *colon;
-	size_t i;
-	size_t len;
+	const char *colon, *arg;
 	int argument_count;
 
 	if (!cmd || !cmd[0])
 		return NULL;
 
-	colon = strchr(cmd, ':');
-	// Measured only once there is a colon to measure against, because
-	// subtracting from a null pointer is not something C defines.
-	len = colon ? (size_t)(colon - cmd) : 0;
-	// A single letter before the colon is a Windows drive letter, not a
-	// filter prefix.
-	if (len == 1)
-		colon = NULL;
-
 	switch (filtertype) {
 	case AUTH:
 		argument_count = 12;
@@ -526,17 +507,20 @@ struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filtertype)
 		break;
 	}
 
-	if (!colon)
-		return new_exec_filter(cmd, argument_count);
-
-	for (i = 0; i < ARRAY_SIZE(filter_specs); i++) {
-		if (len == strlen(filter_specs[i].prefix) && !strncmp(filter_specs[i].prefix, cmd, len))
-			return filter_specs[i].create(colon + 1, argument_count);
-	}
-
+	if (skip_prefix(cmd, "exec:", &arg))
+		return new_exec_filter(arg, argument_count);
+	if (skip_prefix(cmd, "lua:", &arg)) {
 #ifdef NO_LUA
-	if (len == 3 && !strncmp("lua", cmd, len))
 		die("Built without Lua, cannot run filter %s", cmd);
+#else
+		return new_lua_filter(arg, argument_count);
 #endif
-	die("Invalid filter type: %.*s", (int)len, cmd);
+	}
+
+	// A single letter before the colon is a Windows drive letter, not a
+	// filter prefix.
+	colon = strchr(cmd, ':');
+	if (!colon || colon - cmd == 1)
+		return new_exec_filter(cmd, argument_count);
+	die("Invalid filter type: %.*s", (int)(colon - cmd), cmd);
 }