* [PATCH] filter: basic write hooking infrastructure
@ 2014-01-12 21:58 Jason
2014-01-12 22:31 ` [PATCH v2] " Jason
2014-01-12 22:52 ` [PATCH] " john
0 siblings, 2 replies; 4+ messages in thread
From: Jason @ 2014-01-12 21:58 UTC (permalink / raw)
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>
---
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;
+}
+static inline void unhook_write()
+{
+ assert(filter_write != NULL);
+ filter_write = NULL;
+}
--
1.8.5.2
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] filter: basic write hooking infrastructure
2014-01-12 21:58 [PATCH] filter: basic write hooking infrastructure Jason
@ 2014-01-12 22:31 ` Jason
2014-01-12 22:52 ` [PATCH] " john
1 sibling, 0 replies; 4+ messages in thread
From: Jason @ 2014-01-12 22:31 UTC (permalink / raw)
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>
---
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..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
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] filter: basic write hooking infrastructure
2014-01-12 21:58 [PATCH] filter: basic write hooking infrastructure Jason
2014-01-12 22:31 ` [PATCH v2] " Jason
@ 2014-01-12 22:52 ` john
2014-01-12 23:10 ` Jason
1 sibling, 1 reply; 4+ messages in thread
From: john @ 2014-01-12 22:52 UTC (permalink / raw)
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;
> +}
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] filter: basic write hooking infrastructure
2014-01-12 22:52 ` [PATCH] " john
@ 2014-01-12 23:10 ` Jason
0 siblings, 0 replies; 4+ messages in thread
From: Jason @ 2014-01-12 23:10 UTC (permalink / raw)
On Sun, Jan 12, 2014 at 11:52 PM, John Keeping <john at keeping.me.uk> wrote:
> 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.
That was my original idea too. But then I saw that often times we
_don't_ actually use html_raw. Sometimes we printf directly, and other
times, git itself writes data. I don't want to maintain a patchset
against git, so it seemed like the lower level was the only way to go.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-01-12 23:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-12 21:58 [PATCH] filter: basic write hooking infrastructure Jason
2014-01-12 22:31 ` [PATCH v2] " Jason
2014-01-12 22:52 ` [PATCH] " john
2014-01-12 23:10 ` Jason
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).