List for cgit developers and users
 help / color / mirror / Atom feed
From: cgit at cryptocrack.de (Lukas Fleischer)
Subject: [PATCH 05/12] filter: basic write hooking infrastructure
Date: Mon, 13 Jan 2014 09:19:31 +0100	[thread overview]
Message-ID: <20140113081931.1499.15736@typhoon.lan> (raw)
In-Reply-To: <1389586279-23724-6-git-send-email-Jason@zx2c4.com>

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


  reply	other threads:[~2014-01-13  8:19 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-13  4:11 [PATCH 00/12] filter framework and lua integration: complete Jason
2014-01-13  4:11 ` [PATCH 01/12] filter: add fprintf_filter function Jason
2014-01-13  4:11 ` [PATCH 02/12] filter: add interface layer Jason
2014-01-13  4:11 ` [PATCH 03/12] filter: introduce "filter type" prefix Jason
2014-01-13  4:11 ` [PATCH 04/12] filter: allow for cleanup hook for filter types Jason
2014-01-13  4:11 ` [PATCH 05/12] filter: basic write hooking infrastructure Jason
2014-01-13  8:19   ` cgit [this message]
2014-01-13  4:11 ` [PATCH 06/12] filter: add preliminary lua support Jason
2014-01-13  8:31   ` cgit
2014-01-13  8:53     ` john
2014-01-13  8:39   ` cgit
2014-01-13  8:55   ` john
2014-01-13  9:41   ` bluewind
2014-01-13  4:11 ` [PATCH 07/12] filter: document lua filter type Jason
2014-01-13  4:11 ` [PATCH 08/12] filter: lua error reporting Jason
2014-01-13  4:11 ` [PATCH 09/12] filter: return on null filter from open and close Jason
2014-01-13  4:11 ` [PATCH 10/12] filter: add support for email filter Jason
2014-01-13  4:11 ` [PATCH 11/12] filter: add simple gravatar " Jason
2014-01-13  4:11 ` [PATCH 12/12] filter: add gravatar lua script Jason

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140113081931.1499.15736@typhoon.lan \
    --to=cgit@lists.zx2c4.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).