# [PATCH] Clean up file descriptors when calling hooks. 4 messages from 2005-12-08 to 2005-12-08. Participants: Daniel Barkalow, Junio C Hamano. Thread: https://gitlist.dev/t/2773 ## Daniel Barkalow, 2005-12-08 02:04 Subject: [PATCH] Clean up file descriptors when calling hooks. Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0512072052560.25300%40iabervon.org ``` When calling post-update hook, don't leave stdin and stdout connected to the pushing connection. Signed-off-by: Daniel Barkalow --- Someone who's got a better idea of what they should be instead should revise this to make it not just use /dev/null. This does fix my particular test, though; it doesn't cause a post-update hook to crash if it writes to stdout when pushing locally. receive-pack.c | 2 +- run-command.c | 15 +++++++++++++-- run-command.h | 3 +++ 3 files changed, 17 insertions(+), 3 deletions(-) ca48629563058ba62b97b6d4cd3776ca7b7242d3 diff --git a/receive-pack.c b/receive-pack.c index 1873506..cbe37e7 100644 --- a/receive-pack.c +++ b/receive-pack.c @@ -173,7 +173,7 @@ static void run_update_post_hook(struct argc++; } argv[argc] = NULL; - run_command_v(argc, argv); + run_command_v_opt(argc, argv, RUN_COMMAND_NO_STDIO); } /* diff --git a/run-command.c b/run-command.c index 5787a50..8bf5922 100644 --- a/run-command.c +++ b/run-command.c @@ -2,13 +2,19 @@ #include "run-command.h" #include -int run_command_v(int argc, char **argv) +int run_command_v_opt(int argc, char **argv, int flags) { pid_t pid = fork(); if (pid < 0) return -ERR_RUN_COMMAND_FORK; if (!pid) { + if (flags & RUN_COMMAND_NO_STDIO) { + int fd = open("/dev/null", O_RDWR); + dup2(fd, 0); + dup2(fd, 1); + close(fd); + } execvp(argv[0], (char *const*) argv); die("exec %s failed.", argv[0]); } @@ -36,6 +42,11 @@ int run_command_v(int argc, char **argv) } } +int run_command_v(int argc, char **argv) +{ + return run_command_v_opt(argc, argv, 0); +} + int run_command(const char *cmd, ...) { int argc; @@ -54,5 +65,5 @@ int run_command(const char *cmd, ...) va_end(param); if (MAX_RUN_COMMAND_ARGS <= argc) return error("too many args to run %s", cmd); - return run_command_v(argc, argv); + return run_command_v_opt(argc, argv, 0); } diff --git a/run-command.h b/run-command.h index 5ee0972..2469eea 100644 --- a/run-command.h +++ b/run-command.h @@ -11,6 +11,9 @@ enum { ERR_RUN_COMMAND_WAITPID_NOEXIT, }; +#define RUN_COMMAND_NO_STDIO 1 + +int run_command_v_opt(int argc, char **argv, int opt); int run_command_v(int argc, char **argv); int run_command(const char *cmd, ...); -- 0.99.9.GIT ``` ## Junio C Hamano, 2005-12-08 02:25 Subject: Re: [PATCH] Clean up file descriptors when calling hooks. Message-ID: <7vvey0mik7.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vvey0mik7.fsf%40assigned-by-dhcp.cox.net In-Reply-To: ``` Daniel Barkalow writes: > When calling post-update hook, don't leave stdin and stdout connected to > the pushing connection. A quick question. I understand "not connected to the pushing connection" is desirable, but is there a reason you chose to leave them open to /dev/null, not close()d? ``` ## Daniel Barkalow, 2005-12-08 05:33 Subject: Re: [PATCH] Clean up file descriptors when calling hooks. Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0512080006460.25300%40iabervon.org In-Reply-To: <7vvey0mik7.fsf@assigned-by-dhcp.cox.net> ``` On Wed, 7 Dec 2005, Junio C Hamano wrote: > Daniel Barkalow writes: > > > When calling post-update hook, don't leave stdin and stdout connected to > > the pushing connection. > > A quick question. I understand "not connected to the pushing > connection" is desirable, but is there a reason you chose to > leave them open to /dev/null, not close()d? Wouldn't that make programs that assume that stdout (or stdin) is still valid get errors? I particularly want git-merge (which has a habit of saying "Already up-to-date. Yeeah!") to work. Also, something that assumes that fd 1 is somewhere to report stuff will get into trouble if it opens a couple of files for writing and fd 0/1 weren't open before. I seem to recall the standard thing for running with no I/O to be to use /dev/null (e.g., daemon(), if you don't keep your fds, replaces them with /dev/null). -Daniel *This .sig left intentionally blank* ``` ## Junio C Hamano, 2005-12-08 05:40 Subject: Re: [PATCH] Clean up file descriptors when calling hooks. Message-ID: <7vwtigkuyb.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vwtigkuyb.fsf%40assigned-by-dhcp.cox.net In-Reply-To: ``` Daniel Barkalow writes: > On Wed, 7 Dec 2005, Junio C Hamano wrote: > >> A quick question. I understand "not connected to the pushing >> connection" is desirable, but is there a reason you chose to >> leave them open to /dev/null, not close()d? > > Wouldn't that make programs that assume that stdout (or stdin) is still > valid get errors? right both of you are. applied and pushed out. thanks. ```