[RFC/PATCH 0/6] Preparation for more filter types

[RFC/PATCH 0/6] Preparation for more filter types

[john]

This is the preliminary refactoring for supporting more types of filter
(for example Lua scripts or persistent filters).  The final patch adds a
table where more implementations can be added.

The first three (maybe four) patches are sensible cleanups even if we
don't want to take the whole plan any further.

John Keeping (6):
  html: remove redundant htmlfd variable
  ui-snapshot: set unused cgit_filter fields to zero
  filter: pass extra arguments via cgit_open_filter
  filter: add fprintf_filter function
  filter: add interface layer
  filter: introduce "filter type" prefix

 cgit.c        |   6 +--
 cgit.h        |  12 +++++-
 cgitrc.5.txt  |   9 +++++
 filter.c      | 119 ++++++++++++++++++++++++++++++++++++++++++++++++----------
 html.c        |   4 +-
 ui-repolist.c |  10 ++---
 ui-snapshot.c |   9 ++---
 ui-summary.c  |  13 +++----
 ui-tree.c     |   7 ++--
 9 files changed, 140 insertions(+), 49 deletions(-)

-- 
1.8.5.226.g0d60d77

[PATCH 1/6] html: remove redundant htmlfd variable

[john]

This is never changed from STDOUT_FILENO, so just use that value
directly.

Signed-off-by: John Keeping <john at keeping.me.uk>
---
 html.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/html.c b/html.c
index 903d4b7..f0ee2d6 100644
--- a/html.c
+++ b/html.c
@@ -41,8 +41,6 @@ static const char* url_escape_table[256] = {
 	"%fe", "%ff"
 };
 
-static int htmlfd = STDOUT_FILENO;
-
 char *fmt(const char *format, ...)
 {
 	static char buf[8][1024];
@@ -77,7 +75,7 @@ char *fmtalloc(const char *format, ...)
 
 void html_raw(const char *data, size_t size)
 {
-	if (write(htmlfd, data, size) != size)
+	if (write(STDOUT_FILENO, data, size) != size)
 		die_errno("write error on html output");
 }
 
-- 
1.8.5.226.g0d60d77

[PATCH 2/6] ui-snapshot: set unused cgit_filter fields to zero

[john]

By switching the assignment of fields in the cgit_filter structure to
use designated initializers, the compiler will initialize all other
fields to their default value.  This will be needed when we add the
extra_args field in the next patch.

Signed-off-by: John Keeping <john at keeping.me.uk>
---
 ui-snapshot.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/ui-snapshot.c b/ui-snapshot.c
index 8f82119..5136c49 100644
--- a/ui-snapshot.c
+++ b/ui-snapshot.c
@@ -58,10 +58,10 @@ static int write_compressed_tar_archive(const char *hex,
 					char *filter_argv[])
 {
 	int rv;
-	struct cgit_filter f;
-
-	f.cmd = filter_argv[0];
-	f.argv = filter_argv;
+	struct cgit_filter f = {
+		.cmd = filter_argv[0],
+		.argv = filter_argv,
+	};
 	cgit_open_filter(&f);
 	rv = write_tar_archive(hex, prefix);
 	cgit_close_filter(&f);
-- 
1.8.5.226.g0d60d77

[PATCH 3/6] filter: pass extra arguments via cgit_open_filter

[john]

This avoids poking into the filter data structure at various points in
the code.  We rely on the fact that the number of arguments is fixed
based on the filter type (set in cgit_new_filter) and that the call
sites all know which filter type they're using.

Signed-off-by: John Keeping <john at keeping.me.uk>
---
 cgit.h        |  3 ++-
 filter.c      | 35 ++++++++++++++++++++++++-----------
 ui-repolist.c | 10 +++-------
 ui-summary.c  | 13 ++++++-------
 ui-tree.c     |  7 +++----
 5 files changed, 38 insertions(+), 30 deletions(-)

diff --git a/cgit.h b/cgit.h
index a72c503..e6e7715 100644
--- a/cgit.h
+++ b/cgit.h
@@ -59,6 +59,7 @@ typedef enum {
 struct cgit_filter {
 	char *cmd;
 	char **argv;
+	int extra_args;
 	int old_stdout;
 	int pipe_fh[2];
 	int pid;
@@ -342,7 +343,7 @@ extern const char *cgit_repobasename(const char *reponame);
 
 extern int cgit_parse_snapshots_mask(const char *str);
 
-extern int cgit_open_filter(struct cgit_filter *filter);
+extern int cgit_open_filter(struct cgit_filter *filter, ...);
 extern int cgit_close_filter(struct cgit_filter *filter);
 extern struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filtertype);
 
diff --git a/filter.c b/filter.c
index f4ee9ac..d8c0116 100644
--- a/filter.c
+++ b/filter.c
@@ -13,8 +13,16 @@
 #include <string.h>
 #include <stdlib.h>
 
-int cgit_open_filter(struct cgit_filter *filter)
+int cgit_open_filter(struct cgit_filter *filter, ...)
 {
+	int i;
+	va_list ap;
+
+	va_start(ap, filter);
+	for (i = 0; i < filter->extra_args; i++)
+		filter->argv[i+1] = va_arg(ap, char *);
+	va_end(ap);
+
 	filter->old_stdout = chk_positive(dup(STDOUT_FILENO),
 		"Unable to duplicate STDOUT");
 	chk_zero(pipe(filter->pipe_fh), "Unable to create pipe to subprocess");
@@ -36,45 +44,50 @@ int cgit_open_filter(struct cgit_filter *filter)
 
 int cgit_close_filter(struct cgit_filter *filter)
 {
-	int exit_status;
+	int i, exit_status;
 
 	chk_non_negative(dup2(filter->old_stdout, STDOUT_FILENO),
 		"Unable to restore STDOUT");
 	close(filter->old_stdout);
 	if (filter->pid < 0)
-		return 0;
+		goto done;
 	waitpid(filter->pid, &exit_status, 0);
 	if (WIFEXITED(exit_status) && !WEXITSTATUS(exit_status))
-		return 0;
+		goto done;
 	die("Subprocess %s exited abnormally", filter->cmd);
+
+done:
+	for (i = 0; i < filter->extra_args; i++)
+		filter->argv[i+1] = NULL;
+	return 0;
+
 }
 
 struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filtertype)
 {
 	struct cgit_filter *f;
 	int args_size = 0;
-	int extra_args;
 
 	if (!cmd || !cmd[0])
 		return NULL;
 
+	f = xmalloc(sizeof(struct cgit_filter));
+	memset(f, 0, sizeof(struct cgit_filter));
+
 	switch (filtertype) {
 		case SOURCE:
 		case ABOUT:
-			extra_args = 1;
+			f->extra_args = 1;
 			break;
 
 		case COMMIT:
 		default:
-			extra_args = 0;
+			f->extra_args = 0;
 			break;
 	}
-	
-	f = xmalloc(sizeof(struct cgit_filter));
-	memset(f, 0, sizeof(struct cgit_filter));
 
 	f->cmd = xstrdup(cmd);
-	args_size = (2 + extra_args) * sizeof(char *);
+	args_size = (2 + f->extra_args) * sizeof(char *);
 	f->argv = xmalloc(args_size);
 	memset(f->argv, 0, args_size);
 	f->argv[0] = f->cmd;
diff --git a/ui-repolist.c b/ui-repolist.c
index d4ee279..f622a01 100644
--- a/ui-repolist.c
+++ b/ui-repolist.c
@@ -331,13 +331,9 @@ void cgit_print_site_readme()
 {
 	if (!ctx.cfg.root_readme)
 		return;
-	if (ctx.cfg.about_filter) {
-		ctx.cfg.about_filter->argv[1] = ctx.cfg.root_readme;
-		cgit_open_filter(ctx.cfg.about_filter);
-	}
+	if (ctx.cfg.about_filter)
+		cgit_open_filter(ctx.cfg.about_filter, ctx.cfg.root_readme);
 	html_include(ctx.cfg.root_readme);
-	if (ctx.cfg.about_filter) {
+	if (ctx.cfg.about_filter)
 		cgit_close_filter(ctx.cfg.about_filter);
-		ctx.cfg.about_filter->argv[1] = NULL;
-	}
 }
diff --git a/ui-summary.c b/ui-summary.c
index 63a5a75..725f3ab 100644
--- a/ui-summary.c
+++ b/ui-summary.c
@@ -151,18 +151,17 @@ void cgit_print_repo_readme(char *path)
 	 * filesystem, while applying the about-filter.
 	 */
 	html("<div id='summary'>");
-	if (ctx.repo->about_filter) {
-		ctx.repo->about_filter->argv[1] = filename;
-		cgit_open_filter(ctx.repo->about_filter);
-	}
+	if (ctx.repo->about_filter)
+		cgit_open_filter(ctx.repo->about_filter, filename);
+
 	if (ref)
 		cgit_print_file(filename, ref, 1);
 	else
 		html_include(filename);
-	if (ctx.repo->about_filter) {
+
+	if (ctx.repo->about_filter)
 		cgit_close_filter(ctx.repo->about_filter);
-		ctx.repo->about_filter->argv[1] = NULL;
-	}
+
 	html("</div>");
 	if (free_filename)
 		free(filename);
diff --git a/ui-tree.c b/ui-tree.c
index 5ae3926..e4c3d22 100644
--- a/ui-tree.c
+++ b/ui-tree.c
@@ -45,13 +45,12 @@ static void print_text_buffer(const char *name, char *buf, unsigned long size)
 	}
 
 	if (ctx.repo->source_filter) {
+		char *filter_arg = xstrdup(name);
 		html("<td class='lines'><pre><code>");
-		ctx.repo->source_filter->argv[1] = xstrdup(name);
-		cgit_open_filter(ctx.repo->source_filter);
+		cgit_open_filter(ctx.repo->source_filter, filter_arg);
 		html_raw(buf, size);
 		cgit_close_filter(ctx.repo->source_filter);
-		free(ctx.repo->source_filter->argv[1]);
-		ctx.repo->source_filter->argv[1] = NULL;
+		free(filter_arg);
 		html("</code></pre></td></tr></table>\n");
 		return;
 	}
-- 
1.8.5.226.g0d60d77

[PATCH 4/6] filter: add fprintf_filter function

[john]

This stops the code in cgit.c::print_repo needing to inspect the
cgit_filter structure, meaning that we can abstract out different filter
types that will have different fields that need to be printed.

Signed-off-by: John Keeping <john at keeping.me.uk>
---
 cgit.c   | 6 +++---
 cgit.h   | 1 +
 filter.c | 5 +++++
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/cgit.c b/cgit.c
index 0be41b8..29b658e 100644
--- a/cgit.c
+++ b/cgit.c
@@ -706,11 +706,11 @@ static void print_repo(FILE *f, struct cgit_repo *repo)
 	fprintf(f, "repo.enable-log-linecount=%d\n",
 	        repo->enable_log_linecount);
 	if (repo->about_filter && repo->about_filter != ctx.cfg.about_filter)
-		fprintf(f, "repo.about-filter=%s\n", repo->about_filter->cmd);
+		cgit_fprintf_filter(repo->about_filter, f, "repo.about-filter=");
 	if (repo->commit_filter && repo->commit_filter != ctx.cfg.commit_filter)
-		fprintf(f, "repo.commit-filter=%s\n", repo->commit_filter->cmd);
+		cgit_fprintf_filter(repo->commit_filter, f, "repo.commit-filter=");
 	if (repo->source_filter && repo->source_filter != ctx.cfg.source_filter)
-		fprintf(f, "repo.source-filter=%s\n", repo->source_filter->cmd);
+		cgit_fprintf_filter(repo->source_filter, f, "repo.source-filter=");
 	if (repo->snapshots != ctx.cfg.snapshots) {
 		char *tmp = build_snapshot_setting(repo->snapshots);
 		fprintf(f, "repo.snapshots=%s\n", tmp ? tmp : "");
diff --git a/cgit.h b/cgit.h
index e6e7715..9b4be26 100644
--- a/cgit.h
+++ b/cgit.h
@@ -345,6 +345,7 @@ extern int cgit_parse_snapshots_mask(const char *str);
 
 extern int cgit_open_filter(struct cgit_filter *filter, ...);
 extern int cgit_close_filter(struct cgit_filter *filter);
+extern void cgit_fprintf_filter(struct cgit_filter *filter, FILE *f, const char *prefix);
 extern struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filtertype);
 
 extern void cgit_prepare_repo_env(struct cgit_repo * repo);
diff --git a/filter.c b/filter.c
index d8c0116..80cf689 100644
--- a/filter.c
+++ b/filter.c
@@ -63,6 +63,11 @@ done:
 
 }
 
+void cgit_fprintf_filter(struct cgit_filter *filter, FILE *f, const char *prefix)
+{
+	fprintf(f, "%s%s\n", prefix, filter->cmd);
+}
+
 struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filtertype)
 {
 	struct cgit_filter *f;
-- 
1.8.5.226.g0d60d77

[PATCH 5/6] filter: add interface layer

[john]

Change the existing cgit_{open,close,fprintf}_filter functions to
delegate to filter-specific implementations accessed via function
pointers on the cgit_filter object.

We treat the "exec" filter type slightly specially here by putting its
structure definition in the header file and providing an "init" function
to set up the function pointers.  This is required so that the
ui-snapshot.c code that applies a compression filter can continue to use
the filter interface to do so.

Signed-off-by: John Keeping <john at keeping.me.uk>
---
 cgit.h        |  8 ++++++++
 filter.c      | 66 ++++++++++++++++++++++++++++++++++++++++++++---------------
 ui-snapshot.c | 11 +++++-----
 3 files changed, 63 insertions(+), 22 deletions(-)

diff --git a/cgit.h b/cgit.h
index 9b4be26..92e8c55 100644
--- a/cgit.h
+++ b/cgit.h
@@ -57,6 +57,13 @@ typedef enum {
 } filter_type;
 
 struct cgit_filter {
+	int (*open)(struct cgit_filter *, va_list ap);
+	int (*close)(struct cgit_filter *);
+	void (*fprintf)(struct cgit_filter *, FILE *, const char *prefix);
+};
+
+struct cgit_exec_filter {
+	struct cgit_filter base;
 	char *cmd;
 	char **argv;
 	int extra_args;
@@ -346,6 +353,7 @@ extern int cgit_parse_snapshots_mask(const char *str);
 extern int cgit_open_filter(struct cgit_filter *filter, ...);
 extern int cgit_close_filter(struct cgit_filter *filter);
 extern void cgit_fprintf_filter(struct cgit_filter *filter, FILE *f, const char *prefix);
+extern void cgit_exec_filter_init(struct cgit_exec_filter *filter, char *cmd, char **argv);
 extern struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filtertype);
 
 extern void cgit_prepare_repo_env(struct cgit_repo * repo);
diff --git a/filter.c b/filter.c
index 80cf689..0f3edb0 100644
--- a/filter.c
+++ b/filter.c
@@ -13,15 +13,13 @@
 #include <string.h>
 #include <stdlib.h>
 
-int cgit_open_filter(struct cgit_filter *filter, ...)
+static int open_exec_filter(struct cgit_filter *base, va_list ap)
 {
+	struct cgit_exec_filter *filter = (struct cgit_exec_filter *) base;
 	int i;
-	va_list ap;
 
-	va_start(ap, filter);
 	for (i = 0; i < filter->extra_args; i++)
 		filter->argv[i+1] = va_arg(ap, char *);
-	va_end(ap);
 
 	filter->old_stdout = chk_positive(dup(STDOUT_FILENO),
 		"Unable to duplicate STDOUT");
@@ -41,9 +39,9 @@ int cgit_open_filter(struct cgit_filter *filter, ...)
 	return 0;
 }
 
-
-int cgit_close_filter(struct cgit_filter *filter)
+static int close_exec_filter(struct cgit_filter *base)
 {
+	struct cgit_exec_filter *filter = (struct cgit_exec_filter *) base;
 	int i, exit_status;
 
 	chk_non_negative(dup2(filter->old_stdout, STDOUT_FILENO),
@@ -63,21 +61,50 @@ done:
 
 }
 
-void cgit_fprintf_filter(struct cgit_filter *filter, FILE *f, const char *prefix)
+static void fprintf_exec_filter(struct cgit_filter *base, FILE *f, const char *prefix)
 {
+	struct cgit_exec_filter *filter = (struct cgit_exec_filter *) base;
 	fprintf(f, "%s%s\n", prefix, filter->cmd);
 }
 
-struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filtertype)
+int cgit_open_filter(struct cgit_filter *filter, ...)
 {
-	struct cgit_filter *f;
-	int args_size = 0;
+	int result;
+	va_list ap;
+	va_start(ap, filter);
+	result = filter->open(filter, ap);
+	va_end(ap);
+	return result;
+}
 
-	if (!cmd || !cmd[0])
-		return NULL;
+int cgit_close_filter(struct cgit_filter *filter)
+{
+	return filter->close(filter);
+}
+
+void cgit_fprintf_filter(struct cgit_filter *filter, FILE *f, const char *prefix)
+{
+	filter->fprintf(filter, f, prefix);
+}
+
+void cgit_exec_filter_init(struct cgit_exec_filter *filter, char *cmd, char **argv)
+{
+	memset(filter, 0, sizeof(*filter));
+	filter->base.open = open_exec_filter;
+	filter->base.close = close_exec_filter;
+	filter->base.fprintf = fprintf_exec_filter;
+	filter->cmd = cmd;
+	filter->argv = argv;
+}
+
+static struct cgit_filter *new_exec_filter(const char *cmd, filter_type filtertype)
+{
+	struct cgit_exec_filter *f;
+	int args_size = 0;
 
-	f = xmalloc(sizeof(struct cgit_filter));
-	memset(f, 0, sizeof(struct cgit_filter));
+	f = xmalloc(sizeof(*f));
+	/* We leave argv for now and assign it below. */
+	cgit_exec_filter_init(f, xstrdup(cmd), NULL);
 
 	switch (filtertype) {
 		case SOURCE:
@@ -91,10 +118,17 @@ struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filtertype)
 			break;
 	}
 
-	f->cmd = xstrdup(cmd);
 	args_size = (2 + f->extra_args) * sizeof(char *);
 	f->argv = xmalloc(args_size);
 	memset(f->argv, 0, args_size);
 	f->argv[0] = f->cmd;
-	return f;
+	return &f->base;
+}
+
+struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filtertype)
+{
+	if (!cmd || !cmd[0])
+		return NULL;
+
+	return new_exec_filter(cmd, filtertype);
 }
diff --git a/ui-snapshot.c b/ui-snapshot.c
index 5136c49..7115ec4 100644
--- a/ui-snapshot.c
+++ b/ui-snapshot.c
@@ -58,13 +58,12 @@ static int write_compressed_tar_archive(const char *hex,
 					char *filter_argv[])
 {
 	int rv;
-	struct cgit_filter f = {
-		.cmd = filter_argv[0],
-		.argv = filter_argv,
-	};
-	cgit_open_filter(&f);
+	struct cgit_exec_filter f;
+	cgit_exec_filter_init(&f, filter_argv[0], filter_argv);
+
+	cgit_open_filter(&f.base);
 	rv = write_tar_archive(hex, prefix);
-	cgit_close_filter(&f);
+	cgit_close_filter(&f.base);
 	return rv;
 }
 
-- 
1.8.5.226.g0d60d77

[PATCH 6/6] filter: introduce "filter type" prefix

[john]

This allows different filter implementations to be specified in the
configuration file.  Currently only "exec" is supported, but it may now
be specified either with or without the "exec:" prefix.

Signed-off-by: John Keeping <john at keeping.me.uk>
---
 cgitrc.5.txt |  9 +++++++++
 filter.c     | 33 +++++++++++++++++++++++++++++++--
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/cgitrc.5.txt b/cgitrc.5.txt
index 52caed0..60159f6 100644
--- a/cgitrc.5.txt
+++ b/cgitrc.5.txt
@@ -557,6 +557,15 @@ config files, e.g. "repo.desc" becomes "desc".
 
 FILTER API
 ----------
+By default, filters are separate processes that are executed each time they
+are needed.  Alternative technologies may be used by prefixing the filter
+specification with the relevant string; available values are:
+
+'exec:'::
+	The default "one process per filter" mode.
+
+Parameters are provided to filters as follows.
+
 about filter::
 	This filter is given a single parameter: the filename of the source
 	file to filter. The filter can use the filename to determine (for
diff --git a/filter.c b/filter.c
index 0f3edb0..ba66e46 100644
--- a/filter.c
+++ b/filter.c
@@ -64,7 +64,7 @@ done:
 static void fprintf_exec_filter(struct cgit_filter *base, FILE *f, const char *prefix)
 {
 	struct cgit_exec_filter *filter = (struct cgit_exec_filter *) base;
-	fprintf(f, "%s%s\n", prefix, filter->cmd);
+	fprintf(f, "%sexec:%s\n", prefix, filter->cmd);
 }
 
 int cgit_open_filter(struct cgit_filter *filter, ...)
@@ -125,10 +125,39 @@ static struct cgit_filter *new_exec_filter(const char *cmd, filter_type filterty
 	return &f->base;
 }
 
+static const struct {
+	const char *prefix;
+	struct cgit_filter *(*ctor)(const char *cmd, filter_type filtertype);
+} filter_specs[] = {
+	{ "exec", new_exec_filter },
+};
+
 struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filtertype)
 {
+	char *colon;
+	int i;
+	size_t len;
 	if (!cmd || !cmd[0])
 		return NULL;
 
-	return new_exec_filter(cmd, filtertype);
+	colon = strchr(cmd, ':');
+	len = colon - cmd;
+	/*
+	 * In case we're running on Windows, don't allow a single letter before
+	 * the colon.
+	 */
+	if (len == 1)
+		colon = NULL;
+
+	/* If no prefix is given, exec filter is the default. */
+	if (!colon)
+		return new_exec_filter(cmd, filtertype);
+
+	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].ctor(colon + 1, filtertype);
+	}
+
+	die("Invalid filter type: %.*s", (int) len, cmd);
 }
-- 
1.8.5.226.g0d60d77

[PATCH 1/6] html: remove redundant htmlfd variable

[Jason]

I'm merging this, but, it strikes me the initial intent of this was a
bit neat -- instead of dup2ing over stdout and restoring it with a
dup'd original stdout, the htmlfd just had to be modified. I may end
up reverting this change later, but for now I'll merge it.

[PATCH 3/6] filter: pass extra arguments via cgit_open_filter

[Jason]

This is indeed very nice. Thanks. Merged.

[PATCH 4/6] filter: add fprintf_filter function

[Jason]

What's the purpose of this? Why not just keep the original string that
was passed to about-filter=... in the cmd variable as we have now? The
thing that's variable from filter to filter is argv, the type (commit,
about, etc), and the mechanism (lua, stdout, etc). But the variable
aspects don't require changing ->cmd do they?

[PATCH 4/6] filter: add fprintf_filter function

[john]

On Sun, Jan 12, 2014 at 08:23:02PM +0100, Jason A. Donenfeld wrote:
> What's the purpose of this? Why not just keep the original string that
> was passed to about-filter=... in the cmd variable as we have now? The
> thing that's variable from filter to filter is argv, the type (commit,
> about, etc), and the mechanism (lua, stdout, etc). But the variable
> aspects don't require changing ->cmd do they?

I'm looking at splitting up the data so there is a filter object that
contains function pointers to implementation functions and then some
data that is specific to to given filter type.  With that change, cmd
moves to the "exec filter" structure and is no longer accessible.  I'm
expecting some filter types to have a structured value in the config
file that will be parsed to multiple fields which will be glued back
together in the fprintf function for that filter type.

[PATCH 4/6] filter: add fprintf_filter function

[Jason]

On Sun, Jan 12, 2014 at 8:35 PM, John Keeping <john at keeping.me.uk> wrote:
> I'm looking at splitting up the data so there is a filter object that
> contains function pointers to implementation functions and then some
> data that is specific to to given filter type.  With that change, cmd
> moves to the "exec filter" structure and is no longer accessible.  I'm
> expecting some filter types to have a structured value in the config
> file that will be parsed to multiple fields which will be glued back
> together in the fprintf function for that filter type.

Compelling enough. Merging this and the subsequent ones into
jd-jk/filterinfra. I'm doing a few extra patches on top of these as we
speak. Expect an email shortly when I'm finished.