[PATCH 05/12] filter: basic write hooking infrastructure

[Lukas Fleischer]

On Mon, 13 Jan 2014 at 05:11:12, 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.

Clever. Not sure whether I like it, though. I think it would be much
better to have a more transparent hooking mechanism that does use any
"tricks". Could you elaborate on where Git directly prints for us?

> 
> Signed-off-by: Jason A. Donenfeld <Jason at zx2c4.com>
> ---
>  cgit.c   |  2 ++
>  cgit.h   |  1 +
>  cgit.mk  |  4 +++-
>  filter.c | 30 ++++++++++++++++++++++++++++++
>  4 files changed, 36 insertions(+), 1 deletion(-)
> 
> [...]
> 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

This breaks compilation at least on FreeBSD and OpenBSD which do no have
libdl (and have dlsym() built-in as a part of libc). I am not sure about
other platforms. So if we go this way, we should check how to make this
cross-platform compatible.

> +

Stray newline?

>  
>  # 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..8990575 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)(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(buf, count);
> +}
> +
> +static inline void hook_write(ssize_t (*new_write)(const void *buf, size_t count))
> +{
> +       /* We want to avoid buggy nested patterns. */
> +       assert(filter_write == NULL);
> +       filter_write = new_write;
> +}
> +static inline void unhook_write()
> +{
> +       assert(filter_write != NULL);
> +       filter_write = NULL;
> +}
> -- 
> 1.8.5.2
> 
> _______________________________________________
> CGit mailing list
> CGit at lists.zx2c4.com
> http://lists.zx2c4.com/mailman/listinfo/cgit