From mboxrd@z Thu Jan  1 00:00:00 1970
From: john at keeping.me.uk (John Keeping)
Date: Sun, 12 Jan 2014 22:52:23 +0000
Subject: [PATCH] filter: basic write hooking infrastructure
In-Reply-To: <1389563881-10400-1-git-send-email-Jason@zx2c4.com>
References: <1389563881-10400-1-git-send-email-Jason@zx2c4.com>
Message-ID: <20140112225223.GW7608@serenity.lan>

On Sun, Jan 12, 2014 at 10:58:01PM +0100, Jason A. Donenfeld wrote:
> Filters can now call hook_write and unhook_write if they want to
> redirect writing to stdout to a different function. This saves us from
> potential file descriptor pipes and other less efficient mechanisms.
> 
> We do this instead of replacing the call in html_raw because some places
> stdlib's printf functions are used (ui-patch or within git itself),
> which has its own internal buffering, which makes it difficult to
> interlace our function calls. So, we dlsym libc's write and then
> override it in the link stage.
> 
> Signed-off-by: Jason A. Donenfeld <Jason at zx2c4.com>
> ---

Clever.  I've been looking at hooking in at a higher level before html.c
escapes the output.  But I think the approach taken with source_filter
may be a better way of getting the raw output to the filter.

I can't help thinking it would be easier to just replace html_raw here
though.  All our writes go through there anyway and we don't need to
worry about writing to other file descriptors.

>  cgit.c   |  2 ++
>  cgit.h   |  1 +
>  cgit.mk  |  4 +++-
>  filter.c | 30 ++++++++++++++++++++++++++++++
>  4 files changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/cgit.c b/cgit.c
> index 4f31e58..725fd65 100644
> --- a/cgit.c
> +++ b/cgit.c
> @@ -904,6 +904,8 @@ int main(int argc, const char **argv)
>  	const char *path;
>  	int err, ttl;
>  
> +	cgit_init_filters();
> +
>  	prepare_context(&ctx);
>  	cgit_repolist.length = 0;
>  	cgit_repolist.count = 0;
> diff --git a/cgit.h b/cgit.h
> index 893c38f..db34493 100644
> --- a/cgit.h
> +++ b/cgit.h
> @@ -357,6 +357,7 @@ extern void cgit_fprintf_filter(struct cgit_filter *filter, FILE *f, const char
>  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_cleanup_filters(void);
> +extern void cgit_init_filters(void);
>  
>  extern void cgit_prepare_repo_env(struct cgit_repo * repo);
>  
> diff --git a/cgit.mk b/cgit.mk
> index 19a76e7..9d6dea8 100644
> --- a/cgit.mk
> +++ b/cgit.mk
> @@ -61,6 +61,8 @@ $(CGIT_VERSION_OBJS): $(CGIT_PREFIX)VERSION
>  $(CGIT_VERSION_OBJS): EXTRA_CPPFLAGS = \
>  	-DCGIT_VERSION='"$(CGIT_VERSION)"'
>  
> +CGIT_LIBS += -ldl
> +
>  
>  # Git handles dependencies using ":=" so dependencies in CGIT_OBJ are not
>  # handled by that and we must handle them ourselves.
> @@ -88,4 +90,4 @@ $(CGIT_OBJS): %.o: %.c GIT-CFLAGS $(CGIT_PREFIX)CGIT-CFLAGS $(missing_dep_dirs)
>  	$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $(CGIT_CFLAGS) $<
>  
>  $(CGIT_PREFIX)cgit: $(CGIT_OBJS) GIT-LDFLAGS $(GITLIBS)
> -	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
> +	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS) $(CGIT_LIBS)
> diff --git a/filter.c b/filter.c
> index 86c1d5d..d85b1bb 100644
> --- a/filter.c
> +++ b/filter.c
> @@ -12,6 +12,10 @@
>  #include <unistd.h>
>  #include <string.h>
>  #include <stdlib.h>
> +#include <dlfcn.h>
> +
> +static ssize_t (*libc_write)(int fd, const void *buf, size_t count);
> +static ssize_t (*filter_write)(int fd, const void *buf, size_t count) = NULL;
>  
>  static int open_exec_filter(struct cgit_filter *base, va_list ap)
>  {
> @@ -192,3 +196,29 @@ void cgit_cleanup_filters(void)
>  		reap_filter(cgit_repolist.repos[i].source_filter);
>  	}
>  }
> +
> +void cgit_init_filters(void)
> +{
> +	libc_write = dlsym(RTLD_NEXT, "write");
> +	if (!libc_write)
> +		die("Could not locate libc's write function");
> +}
> +
> +ssize_t write(int fd, const void *buf, size_t count)
> +{
> +	if (fd != STDOUT_FILENO || !filter_write)
> +		return libc_write(fd, buf, count);
> +	return filter_write(fd, buf, count);
> +}
> +
> +static inline void hook_write(ssize_t (*new_write)(int fd, const void *buf, size_t count))
> +{
> +	/* We want to avoid buggy nested patterns. */
> +	assert(filter_write == NULL);
> +	filter_write = new_write;
> +}

Missing blank line here.

> +static inline void unhook_write()
> +{
> +	assert(filter_write != NULL);
> +	filter_write = NULL;
> +}