Johannes SixtIf git-daemon is run with --detach or --inetd, then stderr is explicitly redirected to /dev/null. But notice that the service programs were spawned via execl_git_cmd(), in particular, the stderr channel is inherited from the daemon. This means that errors that the programs wrote to stderr (for example, via die()), went to /dev/null.
This patch arranges that the daemon does not merely exec the service program, but forks it and monitors stderr of the child; it writes the errors that it produces to the daemons log via logerror().
A consequence is that the daemon process remains in memory for the full duration of the service program, but this cannot be avoided.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> --- I don't know whether service programs like upload-archive or upload-pack write progress report to stderr or not, for example, if a client does not support side-bands. In this case this patch is probably not enough since this would fill the log with unneeded progress information. Any hints are appreciated.
I intend to follow-up this patch with another one that integrates run_service_command() in execute() in order to streamline how the 'incoming' fd is inherited to the service programs. But I haven't even begun this change, yet, because I'd like to know first how this one is received.
-- Hannes
daemon.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++--------- 1 files changed, 48 insertions(+), 9 deletions(-)
Show changes to daemon.c +48 −9
diff --git a/daemon.c b/daemon.c index d7ceca4..3e1a354 100644 --- a/daemon.c +++ b/daemon.c @@ -1,6 +1,7 @@ #include "cache.h" #include "pkt-line.h" -#include "exec_cmd.h" +#include "run-command.h" +#include "strbuf.h" #include <syslog.h> @@ -343,28 +344,66 @@ static int run_service(char *dir, const struct daemon_service *service) return service->fn(); } +static void copy_to_log(int fd) +{ + struct strbuf line = STRBUF_INIT; + FILE *fp; + + fp = fdopen(fd, "r"); + if (fp == NULL) { + logerror("fdopen of error channel failed"); + close(fd); + return; + } + + while (strbuf_getline(&line, fp, '\n') != EOF) { + logerror("%s", line.buf); + strbuf_setlen(&line, 0); + } + + strbuf_release(&line); + fclose(fp); +} + +static int run_service_command(const char **argv) +{ + struct child_process cld; + + memset(&cld, 0, sizeof(cld)); + cld.argv = argv; + cld.git_cmd = 1; + cld.err = -1; + if (start_command(&cld)) + return -1; + + close(0); + close(1); + + copy_to_log(cld.err); + + return finish_command(&cld); +} + static int upload_pack(void) { /* Timeout as string */ char timeout_buf[64]; + const char *argv[] = { "upload-pack", "--strict", timeout_buf, ".", NULL }; snprintf(timeout_buf, sizeof timeout_buf, "--timeout=%u", timeout); - - /* git-upload-pack only ever reads stuff, so this is safe */ - execl_git_cmd("upload-pack", "--strict", timeout_buf, ".", NULL); - return -1; + return run_service_command(argv); } static int upload_archive(void) { - execl_git_cmd("upload-archive", ".", NULL); - return -1; + static const char *argv[] = { "upload-archive", ".", NULL }; + return run_service_command(argv); } static int receive_pack(void) { - execl_git_cmd("receive-pack", ".", NULL); - return -1; + static const char *argv[] = { "receive-pack", ".", NULL }; + return run_service_command(argv); } static struct daemon_service daemon_service[] = {
-- 1.6.3.17.g1665f
