From 9b0b50936ec76ad8e582d18d5bf54bc81c685e9b Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Sat, 30 Dec 2006 21:55:15 -0500 Subject: Remove unnecessary argc parameter from run_command_v. The argc parameter is never used by the run_command_v family of functions. Instead they require that the passed argv[] be NULL terminated so they can rely on the operating system's execvp function to correctly pass the arguments to the new process. Making the caller pass the argc is just confusing, as the caller could be mislead into believing that the argc might take precendece over the argv, or that the argv does not need to be NULL terminated. So goodbye argc. Don't come back. Signed-off-by: Shawn O. Pearce Signed-off-by: Junio C Hamano --- run-command.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'run-command.h') diff --git a/run-command.h b/run-command.h index 70b477a748..82a0861f23 100644 --- a/run-command.h +++ b/run-command.h @@ -13,8 +13,8 @@ enum { #define RUN_COMMAND_NO_STDIO 1 #define RUN_GIT_CMD 2 /*If this is to be git sub-command */ -int run_command_v_opt(int argc, const char **argv, int opt); -int run_command_v(int argc, const char **argv); +int run_command_v_opt(const char **argv, int opt); +int run_command_v(const char **argv); int run_command(const char *cmd, ...); #endif -- cgit v1.3 From cd83c74cb3161a19b5efd33f40cfe378c2f64ddb Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Sat, 30 Dec 2006 21:55:19 -0500 Subject: Redirect update hook stdout to stderr. If an update hook outputs to stdout then that output will be sent back over the wire to the push client as though it were part of the git protocol. This tends to cause protocol errors on the client end of the connection, as the hook output is not expected in that context. Most hook developers work around this by making sure their hook outputs everything to stderr. But hooks shouldn't need to perform such special behavior. Instead we can just dup stderr to stdout prior to invoking the update hook. Signed-off-by: Shawn O. Pearce Signed-off-by: Junio C Hamano --- receive-pack.c | 3 ++- run-command.c | 32 ++++++++++++++++++++++++++------ run-command.h | 2 ++ 3 files changed, 30 insertions(+), 7 deletions(-) (limited to 'run-command.h') diff --git a/receive-pack.c b/receive-pack.c index 2b0ba638af..48e49465ba 100644 --- a/receive-pack.c +++ b/receive-pack.c @@ -73,7 +73,8 @@ static int run_update_hook(const char *refname, if (access(update_hook, X_OK) < 0) return 0; - code = run_command(update_hook, refname, old_hex, new_hex, NULL); + code = run_command_opt(RUN_COMMAND_STDOUT_TO_STDERR, + update_hook, refname, old_hex, new_hex, NULL); switch (code) { case 0: return 0; diff --git a/run-command.c b/run-command.c index d0330c3793..7e4ca43c62 100644 --- a/run-command.c +++ b/run-command.c @@ -14,7 +14,8 @@ int run_command_v_opt(const char **argv, int flags) dup2(fd, 0); dup2(fd, 1); close(fd); - } + } else if (flags & RUN_COMMAND_STDOUT_TO_STDERR) + dup2(2, 1); if (flags & RUN_GIT_CMD) { execv_git_cmd(argv); } else { @@ -51,14 +52,12 @@ int run_command_v(const char **argv) return run_command_v_opt(argv, 0); } -int run_command(const char *cmd, ...) +static int run_command_va_opt(int opt, const char *cmd, va_list param) { int argc; const char *argv[MAX_RUN_COMMAND_ARGS]; const char *arg; - va_list param; - va_start(param, cmd); argv[0] = (char*) cmd; argc = 1; while (argc < MAX_RUN_COMMAND_ARGS) { @@ -66,8 +65,29 @@ int run_command(const char *cmd, ...) if (!arg) break; } - va_end(param); if (MAX_RUN_COMMAND_ARGS <= argc) return error("too many args to run %s", cmd); - return run_command_v_opt(argv, 0); + return run_command_v_opt(argv, opt); +} + +int run_command_opt(int opt, const char *cmd, ...) +{ + va_list params; + int r; + + va_start(params, cmd); + r = run_command_va_opt(opt, cmd, params); + va_end(params); + return r; +} + +int run_command(const char *cmd, ...) +{ + va_list params; + int r; + + va_start(params, cmd); + r = run_command_va_opt(0, cmd, params); + va_end(params); + return r; } diff --git a/run-command.h b/run-command.h index 82a0861f23..8156eac31b 100644 --- a/run-command.h +++ b/run-command.h @@ -13,8 +13,10 @@ enum { #define RUN_COMMAND_NO_STDIO 1 #define RUN_GIT_CMD 2 /*If this is to be git sub-command */ +#define RUN_COMMAND_STDOUT_TO_STDERR 4 int run_command_v_opt(const char **argv, int opt); int run_command_v(const char **argv); +int run_command_opt(int opt, const char *cmd, ...); int run_command(const char *cmd, ...); #endif -- cgit v1.3 From 95d3c4f546c664c3571dd4a93f11ae2f54e55e6e Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Sat, 30 Dec 2006 21:55:22 -0500 Subject: Use /dev/null for update hook stdin. Currently the update hook invoked by receive-pack has its stdin connected to the pushing client. The hook shouldn't attempt to read from this stream, and doing so may consume data that was meant for receive-pack. Instead we should give the update hook /dev/null as its stdin, ensuring that it always receives EOF and doesn't disrupt the protocol if it attempts to read any data. The post-update hook is similar, as it gets invoked with /dev/null on stdin to prevent the hook from reading data from the client. Previously we had invoked it with stdout also connected to /dev/null, throwing away anything on stdout, to prevent client protocol errors. Instead we should redirect stdout to stderr, like we do with the update hook. Signed-off-by: Shawn O. Pearce Signed-off-by: Junio C Hamano --- receive-pack.c | 6 ++++-- run-command.c | 6 +++--- run-command.h | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) (limited to 'run-command.h') diff --git a/receive-pack.c b/receive-pack.c index 48e49465ba..c176d8fd00 100644 --- a/receive-pack.c +++ b/receive-pack.c @@ -73,7 +73,8 @@ static int run_update_hook(const char *refname, if (access(update_hook, X_OK) < 0) return 0; - code = run_command_opt(RUN_COMMAND_STDOUT_TO_STDERR, + code = run_command_opt(RUN_COMMAND_NO_STDIN + | RUN_COMMAND_STDOUT_TO_STDERR, update_hook, refname, old_hex, new_hex, NULL); switch (code) { case 0: @@ -188,7 +189,8 @@ static void run_update_post_hook(struct command *cmd) argc++; } argv[argc] = NULL; - run_command_v_opt(argv, RUN_COMMAND_NO_STDIO); + run_command_v_opt(argv, RUN_COMMAND_NO_STDIN + | RUN_COMMAND_STDOUT_TO_STDERR); } /* diff --git a/run-command.c b/run-command.c index 7e4ca43c62..cfbad74d14 100644 --- a/run-command.c +++ b/run-command.c @@ -9,12 +9,12 @@ int run_command_v_opt(const char **argv, int flags) if (pid < 0) return -ERR_RUN_COMMAND_FORK; if (!pid) { - if (flags & RUN_COMMAND_NO_STDIO) { + if (flags & RUN_COMMAND_NO_STDIN) { int fd = open("/dev/null", O_RDWR); dup2(fd, 0); - dup2(fd, 1); close(fd); - } else if (flags & RUN_COMMAND_STDOUT_TO_STDERR) + } + if (flags & RUN_COMMAND_STDOUT_TO_STDERR) dup2(2, 1); if (flags & RUN_GIT_CMD) { execv_git_cmd(argv); diff --git a/run-command.h b/run-command.h index 8156eac31b..59c4476ced 100644 --- a/run-command.h +++ b/run-command.h @@ -11,7 +11,7 @@ enum { ERR_RUN_COMMAND_WAITPID_NOEXIT, }; -#define RUN_COMMAND_NO_STDIO 1 +#define RUN_COMMAND_NO_STDIN 1 #define RUN_GIT_CMD 2 /*If this is to be git sub-command */ #define RUN_COMMAND_STDOUT_TO_STDERR 4 int run_command_v_opt(const char **argv, int opt); -- cgit v1.3