{"thread":{"id":"2773","subject":"[PATCH] Clean up file descriptors when calling hooks.","startedAt":"2005-12-08T02:04:38Z","lastAt":"2005-12-08T05:40:28Z","messageCount":4,"participants":["Daniel Barkalow","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"13342","messageId":"Pine.LNX.4.64.0512072052560.25300@iabervon.org","threadId":"2773","inReplyTo":null,"subject":"[PATCH] Clean up file descriptors when calling hooks.","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2005-12-08T02:04:38Z","receivedAt":"2005-12-08T02:04:38Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"When calling post-update hook, don't leave stdin and stdout connected to \nthe pushing connection.\n\nSigned-off-by: Daniel Barkalow \n\n---\n\nSomeone who's got a better idea of what they should be instead should \nrevise this to make it not just use /dev/null. This does fix my particular \ntest, though; it doesn't cause a post-update hook to crash if it writes to \nstdout when pushing locally.\n\n receive-pack.c | 2 +-\n run-command.c | 15 +++++++++++++--\n run-command.h | 3 +++\n 3 files changed, 17 insertions(+), 3 deletions(-)\n\nca48629563058ba62b97b6d4cd3776ca7b7242d3\ndiff --git a/receive-pack.c b/receive-pack.c\nindex 1873506..cbe37e7 100644\n--- a/receive-pack.c\n+++ b/receive-pack.c\n@@ -173,7 +173,7 @@ static void run_update_post_hook(struct \n \t\targc++;\n \t}\n \targv[argc] = NULL;\n-\trun_command_v(argc, argv);\n+\trun_command_v_opt(argc, argv, RUN_COMMAND_NO_STDIO);\n }\n \n /*\ndiff --git a/run-command.c b/run-command.c\nindex 5787a50..8bf5922 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -2,13 +2,19 @@\n #include \"run-command.h\"\n #include \n \n-int run_command_v(int argc, char **argv)\n+int run_command_v_opt(int argc, char **argv, int flags)\n {\n \tpid_t pid = fork();\n \n \tif (pid < 0)\n \t\treturn -ERR_RUN_COMMAND_FORK;\n \tif (!pid) {\n+\t\tif (flags & RUN_COMMAND_NO_STDIO) {\n+\t\t\tint fd = open(\"/dev/null\", O_RDWR);\n+\t\t\tdup2(fd, 0);\n+\t\t\tdup2(fd, 1);\n+\t\t\tclose(fd);\t\t\t\n+\t\t}\n \t\texecvp(argv[0], (char *const*) argv);\n \t\tdie(\"exec %s failed.\", argv[0]);\n \t}\n@@ -36,6 +42,11 @@ int run_command_v(int argc, char **argv)\n \t}\n }\n \n+int run_command_v(int argc, char **argv)\n+{\n+\treturn run_command_v_opt(argc, argv, 0);\n+}\n+\n int run_command(const char *cmd, ...)\n {\n \tint argc;\n@@ -54,5 +65,5 @@ int run_command(const char *cmd, ...)\n \tva_end(param);\n \tif (MAX_RUN_COMMAND_ARGS <= argc)\n \t\treturn error(\"too many args to run %s\", cmd);\n-\treturn run_command_v(argc, argv);\n+\treturn run_command_v_opt(argc, argv, 0);\n }\ndiff --git a/run-command.h b/run-command.h\nindex 5ee0972..2469eea 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -11,6 +11,9 @@ enum {\n \tERR_RUN_COMMAND_WAITPID_NOEXIT,\n };\n \n+#define RUN_COMMAND_NO_STDIO 1\n+\n+int run_command_v_opt(int argc, char **argv, int opt);\n int run_command_v(int argc, char **argv);\n int run_command(const char *cmd, ...);\n \n-- \n0.99.9.GIT\n"},{"id":"13343","messageId":"7vvey0mik7.fsf@assigned-by-dhcp.cox.net","threadId":"2773","inReplyTo":"Pine.LNX.4.64.0512072052560.25300@iabervon.org","subject":"Re: [PATCH] Clean up file descriptors when calling hooks.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-12-08T02:25:12Z","receivedAt":"2005-12-08T02:25:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow writes:\n\n> When calling post-update hook, don't leave stdin and stdout connected to \n> the pushing connection.\n\nA quick question. I understand \"not connected to the pushing\nconnection\" is desirable, but is there a reason you chose to\nleave them open to /dev/null, not close()d?\n"},{"id":"13346","messageId":"Pine.LNX.4.64.0512080006460.25300@iabervon.org","threadId":"2773","inReplyTo":"7vvey0mik7.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Clean up file descriptors when calling hooks.","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2005-12-08T05:33:57Z","receivedAt":"2005-12-08T05:33:57Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Wed, 7 Dec 2005, Junio C Hamano wrote:\n\n> Daniel Barkalow writes:\n> \n> > When calling post-update hook, don't leave stdin and stdout connected to \n> > the pushing connection.\n> \n> A quick question. I understand \"not connected to the pushing\n> connection\" is desirable, but is there a reason you chose to\n> leave them open to /dev/null, not close()d?\n\nWouldn't that make programs that assume that stdout (or stdin) is still \nvalid get errors? I particularly want git-merge (which has a habit of \nsaying \"Already up-to-date. Yeeah!\") to work. Also, something that assumes \nthat fd 1 is somewhere to report stuff will get into trouble if it opens a \ncouple of files for writing and fd 0/1 weren't open before. I seem to \nrecall the standard thing for running with no I/O to be to use /dev/null \n(e.g., daemon(), if you don't keep your fds, replaces them with /dev/null).\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"13347","messageId":"7vwtigkuyb.fsf@assigned-by-dhcp.cox.net","threadId":"2773","inReplyTo":"Pine.LNX.4.64.0512080006460.25300@iabervon.org","subject":"Re: [PATCH] Clean up file descriptors when calling hooks.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-12-08T05:40:28Z","receivedAt":"2005-12-08T05:40:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow writes:\n\n> On Wed, 7 Dec 2005, Junio C Hamano wrote:\n>\n>> A quick question. I understand \"not connected to the pushing\n>> connection\" is desirable, but is there a reason you chose to\n>> leave them open to /dev/null, not close()d?\n>\n> Wouldn't that make programs that assume that stdout (or stdin) is still \n> valid get errors?\n\nright both of you are. applied and pushed out. thanks.\n"}]}