From 303e7c48eae7e140a9612ff1f9b5a95ca80b65c4 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Sat, 4 Jul 2009 21:26:38 +0200 Subject: MinGW: simplify waitpid() emulation macros Windows does not have signals. At least they cannot be diagnosed by the parent process; all that the parent process can observe is the exit code. This also adds a dummy definition of WTERMSIG. Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano diff --git a/compat/mingw.h b/compat/mingw.h index c1859c5..948de66 100644 --- a/compat/mingw.h +++ b/compat/mingw.h @@ -17,9 +17,10 @@ typedef int pid_t; #define S_IROTH 0 #define S_IXOTH 0 -#define WIFEXITED(x) ((unsigned)(x) < 259) /* STILL_ACTIVE */ +#define WIFEXITED(x) 1 +#define WIFSIGNALED(x) 0 #define WEXITSTATUS(x) ((x) & 0xff) -#define WIFSIGNALED(x) ((unsigned)(x) > 259) +#define WTERMSIG(x) SIGTERM #define SIGHUP 1 #define SIGQUIT 3 -- cgit v0.10.2-6-g49f6 From 5709e0363a891b72eb9e9756d7fb121d9bf6a7c7 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Sat, 4 Jul 2009 21:26:39 +0200 Subject: run_command: return exit code as positive value As a general guideline, functions in git's code return zero to indicate success and negative values to indicate failure. The run_command family of functions followed this guideline. But there are actually two different kinds of failure: - failures of system calls; - non-zero exit code of the program that was run. Usually, a non-zero exit code of the program is a failure and means a failure to the caller. Except that sometimes it does not. For example, the exit code of merge programs (e.g. external merge drivers) conveys information about how the merge failed, and not all exit calls are actually failures. Furthermore, the return value of run_command is sometimes used as exit code by the caller. This change arranges that the exit code of the program is returned as a positive value, which can now be regarded as the "result" of the function. System call failures continue to be reported as negative values. Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano diff --git a/builtin-merge.c b/builtin-merge.c index af9adab..96ecaf4 100644 --- a/builtin-merge.c +++ b/builtin-merge.c @@ -594,7 +594,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common, discard_cache(); if (read_cache() < 0) die("failed to read the cache"); - return -ret; + return ret; } } diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c index 6ec1d05..6235903 100644 --- a/builtin-receive-pack.c +++ b/builtin-receive-pack.c @@ -143,8 +143,8 @@ static int run_status(int code, const char *cmd_name) case -ERR_RUN_COMMAND_WAITPID_NOEXIT: return error("%s died strangely", cmd_name); default: - error("%s exited with error code %d", cmd_name, -code); - return -code; + error("%s exited with error code %d", cmd_name, code); + return code; } } diff --git a/convert.c b/convert.c index 1816e97..491e714 100644 --- a/convert.c +++ b/convert.c @@ -267,7 +267,7 @@ static int filter_buffer(int fd, void *data) status = finish_command(&child_process); if (status) - error("external filter %s failed %d", params->cmd, -status); + error("external filter %s failed %d", params->cmd, status); return (write_err || status); } diff --git a/git.c b/git.c index 65ed733..d223eab 100644 --- a/git.c +++ b/git.c @@ -418,9 +418,9 @@ static void execv_dashed_external(const char **argv) */ status = run_command_v_opt(argv, 0); if (status != -ERR_RUN_COMMAND_EXEC) { - if (IS_RUN_COMMAND_ERR(status)) + if (status < 0) die("unable to run '%s'", argv[0]); - exit(-status); + exit(status); } errno = ENOENT; /* as if we called execvp */ diff --git a/ll-merge.c b/ll-merge.c index a2c13c4..31c7457 100644 --- a/ll-merge.c +++ b/ll-merge.c @@ -192,10 +192,6 @@ static int ll_ext_merge(const struct ll_merge_driver *fn, args[2] = cmd.buf; status = run_command_v_opt(args, 0); - if (status < -ERR_RUN_COMMAND_FORK) - ; /* failure in run-command */ - else - status = -status; fd = open(temp[1], O_RDONLY); if (fd < 0) goto bad; diff --git a/run-command.c b/run-command.c index eb2efc3..a4e309e 100644 --- a/run-command.c +++ b/run-command.c @@ -241,14 +241,7 @@ static int wait_or_whine(pid_t pid) if (!WIFEXITED(status)) return -ERR_RUN_COMMAND_WAITPID_NOEXIT; code = WEXITSTATUS(status); - switch (code) { - case 127: - return -ERR_RUN_COMMAND_EXEC; - case 0: - return 0; - default: - return -code; - } + return code == 127 ? -ERR_RUN_COMMAND_EXEC : code; } } diff --git a/run-command.h b/run-command.h index e345502..0211e1d 100644 --- a/run-command.h +++ b/run-command.h @@ -10,7 +10,6 @@ enum { ERR_RUN_COMMAND_WAITPID_SIGNAL, ERR_RUN_COMMAND_WAITPID_NOEXIT, }; -#define IS_RUN_COMMAND_ERR(x) (-(x) >= ERR_RUN_COMMAND_FORK) struct child_process { const char **argv; -- cgit v0.10.2-6-g49f6 From 0ac77ec3150f43a5c2a6b1e47e9db5aafe53fb72 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Sat, 4 Jul 2009 21:26:40 +0200 Subject: run_command: report system call errors instead of returning error codes The motivation for this change is that system call failures are serious errors that should be reported to the user, but only few callers took the burden to decode the error codes that the functions returned into error messages. If at all, then only an unspecific error message was given. A prominent example is this: $ git upload-pack . | : fatal: unable to run 'git-upload-pack' In this example, git-upload-pack, the external command invoked through the git wrapper, dies due to SIGPIPE, but the git wrapper does not bother to report the real cause. In fact, this very error message is copied to the syslog if git-daemon's client aborts the connection early. With this change, system call failures are reported immediately after the failure and only a generic failure code is returned to the caller. In the above example the error is now to the point: $ git upload-pack . | : error: git-upload-pack died of signal Note that there is no error report if the invoked program terminated with a non-zero exit code, because it is reasonable to expect that the invoked program has already reported an error. (But many run_command call sites nevertheless write a generic error message.) There was one special return code that was used to identify the case where run_command failed because the requested program could not be exec'd. This special case is now treated like a system call failure with errno set to ENOENT. No error is reported in this case, because the call site in git.c expects this as a normal result. Therefore, the callers that carefully decoded the return value still check for this condition. Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c index 6235903..1dcdb1a 100644 --- a/builtin-receive-pack.c +++ b/builtin-receive-pack.c @@ -125,27 +125,11 @@ static const char post_receive_hook[] = "hooks/post-receive"; static int run_status(int code, const char *cmd_name) { - switch (code) { - case 0: - return 0; - case -ERR_RUN_COMMAND_FORK: - return error("fork of %s failed", cmd_name); - case -ERR_RUN_COMMAND_EXEC: + if (code < 0 && errno == ENOENT) return error("execute of %s failed", cmd_name); - case -ERR_RUN_COMMAND_PIPE: - return error("pipe failed"); - case -ERR_RUN_COMMAND_WAITPID: - return error("waitpid failed"); - case -ERR_RUN_COMMAND_WAITPID_WRONG_PID: - return error("waitpid is confused"); - case -ERR_RUN_COMMAND_WAITPID_SIGNAL: - return error("%s died of signal", cmd_name); - case -ERR_RUN_COMMAND_WAITPID_NOEXIT: - return error("%s died strangely", cmd_name); - default: + else if (code > 0) error("%s exited with error code %d", cmd_name, code); - return code; - } + return code; } static int run_receive_hook(const char *hook_name) diff --git a/git.c b/git.c index d223eab..03726ee 100644 --- a/git.c +++ b/git.c @@ -417,12 +417,8 @@ static void execv_dashed_external(const char **argv) * OK to return. Otherwise, we just pass along the status code. */ status = run_command_v_opt(argv, 0); - if (status != -ERR_RUN_COMMAND_EXEC) { - if (status < 0) - die("unable to run '%s'", argv[0]); + if (status >= 0 || errno != ENOENT) exit(status); - } - errno = ENOENT; /* as if we called execvp */ argv[0] = tmp; diff --git a/run-command.c b/run-command.c index a4e309e..e273c6c 100644 --- a/run-command.c +++ b/run-command.c @@ -19,6 +19,7 @@ int start_command(struct child_process *cmd) { int need_in, need_out, need_err; int fdin[2], fdout[2], fderr[2]; + int failed_errno; /* * In case of errors we must keep the promise to close FDs @@ -28,9 +29,10 @@ int start_command(struct child_process *cmd) need_in = !cmd->no_stdin && cmd->in < 0; if (need_in) { if (pipe(fdin) < 0) { + failed_errno = errno; if (cmd->out > 0) close(cmd->out); - return -ERR_RUN_COMMAND_PIPE; + goto fail_pipe; } cmd->in = fdin[1]; } @@ -40,11 +42,12 @@ int start_command(struct child_process *cmd) && cmd->out < 0; if (need_out) { if (pipe(fdout) < 0) { + failed_errno = errno; if (need_in) close_pair(fdin); else if (cmd->in) close(cmd->in); - return -ERR_RUN_COMMAND_PIPE; + goto fail_pipe; } cmd->out = fdout[0]; } @@ -52,6 +55,7 @@ int start_command(struct child_process *cmd) need_err = !cmd->no_stderr && cmd->err < 0; if (need_err) { if (pipe(fderr) < 0) { + failed_errno = errno; if (need_in) close_pair(fdin); else if (cmd->in) @@ -60,7 +64,11 @@ int start_command(struct child_process *cmd) close_pair(fdout); else if (cmd->out) close(cmd->out); - return -ERR_RUN_COMMAND_PIPE; +fail_pipe: + error("cannot create pipe for %s: %s", + cmd->argv[0], strerror(failed_errno)); + errno = failed_errno; + return -1; } cmd->err = fderr[0]; } @@ -122,6 +130,9 @@ int start_command(struct child_process *cmd) strerror(errno)); exit(127); } + if (cmd->pid < 0) + error("cannot fork() for %s: %s", cmd->argv[0], + strerror(failed_errno = errno)); #else int s0 = -1, s1 = -1, s2 = -1; /* backups of stdin, stdout, stderr */ const char **sargv = cmd->argv; @@ -173,6 +184,9 @@ int start_command(struct child_process *cmd) } cmd->pid = mingw_spawnvpe(cmd->argv[0], cmd->argv, env); + failed_errno = errno; + if (cmd->pid < 0 && errno != ENOENT) + error("cannot spawn %s: %s", cmd->argv[0], strerror(errno)); if (cmd->env) free_environ(env); @@ -189,7 +203,6 @@ int start_command(struct child_process *cmd) #endif if (cmd->pid < 0) { - int err = errno; if (need_in) close_pair(fdin); else if (cmd->in) @@ -200,9 +213,8 @@ int start_command(struct child_process *cmd) close(cmd->out); if (need_err) close_pair(fderr); - return err == ENOENT ? - -ERR_RUN_COMMAND_EXEC : - -ERR_RUN_COMMAND_FORK; + errno = failed_errno; + return -1; } if (need_in) @@ -221,33 +233,41 @@ int start_command(struct child_process *cmd) return 0; } -static int wait_or_whine(pid_t pid) +static int wait_or_whine(pid_t pid, const char *argv0) { - for (;;) { - int status, code; - pid_t waiting = waitpid(pid, &status, 0); - - if (waiting < 0) { - if (errno == EINTR) - continue; - error("waitpid failed (%s)", strerror(errno)); - return -ERR_RUN_COMMAND_WAITPID; - } - if (waiting != pid) - return -ERR_RUN_COMMAND_WAITPID_WRONG_PID; - if (WIFSIGNALED(status)) - return -ERR_RUN_COMMAND_WAITPID_SIGNAL; - - if (!WIFEXITED(status)) - return -ERR_RUN_COMMAND_WAITPID_NOEXIT; + int status, code = -1; + pid_t waiting; + int failed_errno = 0; + + while ((waiting = waitpid(pid, &status, 0)) < 0 && errno == EINTR) + ; /* nothing */ + + if (waiting < 0) { + failed_errno = errno; + error("waitpid for %s failed: %s", argv0, strerror(errno)); + } else if (waiting != pid) { + error("waitpid is confused (%s)", argv0); + } else if (WIFSIGNALED(status)) { + error("%s died of signal", argv0); + } else if (WIFEXITED(status)) { code = WEXITSTATUS(status); - return code == 127 ? -ERR_RUN_COMMAND_EXEC : code; + /* + * Convert special exit code when execvp failed. + */ + if (code == 127) { + code = -1; + failed_errno = ENOENT; + } + } else { + error("waitpid is confused (%s)", argv0); } + errno = failed_errno; + return code; } int finish_command(struct child_process *cmd) { - return wait_or_whine(cmd->pid); + return wait_or_whine(cmd->pid, cmd->argv[0]); } int run_command(struct child_process *cmd) @@ -331,10 +351,7 @@ int start_async(struct async *async) int finish_async(struct async *async) { #ifndef __MINGW32__ - int ret = 0; - - if (wait_or_whine(async->pid)) - ret = error("waitpid (async) failed"); + int ret = wait_or_whine(async->pid, "child process"); #else DWORD ret = 0; if (WaitForSingleObject(async->tid, INFINITE) != WAIT_OBJECT_0) @@ -378,15 +395,7 @@ int run_hook(const char *index_file, const char *name, ...) hook.env = env; } - ret = start_command(&hook); + ret = run_command(&hook); free(argv); - if (ret) { - warning("Could not spawn %s", argv[0]); - return ret; - } - ret = finish_command(&hook); - if (ret == -ERR_RUN_COMMAND_WAITPID_SIGNAL) - warning("%s exited due to uncaught signal", argv[0]); - return ret; } diff --git a/run-command.h b/run-command.h index 0211e1d..ac6c09c 100644 --- a/run-command.h +++ b/run-command.h @@ -1,16 +1,6 @@ #ifndef RUN_COMMAND_H #define RUN_COMMAND_H -enum { - ERR_RUN_COMMAND_FORK = 10000, - ERR_RUN_COMMAND_EXEC, - ERR_RUN_COMMAND_PIPE, - ERR_RUN_COMMAND_WAITPID, - ERR_RUN_COMMAND_WAITPID_WRONG_PID, - ERR_RUN_COMMAND_WAITPID_SIGNAL, - ERR_RUN_COMMAND_WAITPID_NOEXIT, -}; - struct child_process { const char **argv; pid_t pid; diff --git a/t/t5530-upload-pack-error.sh b/t/t5530-upload-pack-error.sh index f5102b9..82ca300 100755 --- a/t/t5530-upload-pack-error.sh +++ b/t/t5530-upload-pack-error.sh @@ -53,7 +53,10 @@ test_expect_success 'upload-pack fails due to error in rev-list' ' ! echo "0032want $(git rev-parse HEAD) 00000009done 0000" | git upload-pack . > /dev/null 2> output.err && - grep "waitpid (async) failed" output.err + # pack-objects survived + grep "Total.*, reused" output.err && + # but there was an error, which must have been in rev-list + grep "bad tree object" output.err ' test_expect_success 'create empty repository' ' diff --git a/transport.c b/transport.c index 501a77b..0885801 100644 --- a/transport.c +++ b/transport.c @@ -417,18 +417,8 @@ static int curl_transport_push(struct transport *transport, int refspec_nr, cons argv[argc++] = *refspec++; argv[argc] = NULL; err = run_command_v_opt(argv, RUN_GIT_CMD); - switch (err) { - case -ERR_RUN_COMMAND_FORK: - error("unable to fork for %s", argv[0]); - case -ERR_RUN_COMMAND_EXEC: + if (err < 0 && errno == ENOENT) error("unable to exec %s", argv[0]); - break; - case -ERR_RUN_COMMAND_WAITPID: - case -ERR_RUN_COMMAND_WAITPID_WRONG_PID: - case -ERR_RUN_COMMAND_WAITPID_SIGNAL: - case -ERR_RUN_COMMAND_WAITPID_NOEXIT: - error("%s died with strange error", argv[0]); - } return !!err; } -- cgit v0.10.2-6-g49f6 From b99d5f40d6a5cba7d7cd7599063b3cd78aa4d219 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Sat, 4 Jul 2009 21:26:41 +0200 Subject: run_command: encode deadly signal number in the return value We now write the signal number in the error message if the program terminated by a signal. The negative return value is constructed such that after truncation to 8 bits it looks like a POSIX shell's $?: $ echo 0000 | { git upload-pack .; echo $? >&2; } | : error: git-upload-pack died of signal 13 141 Previously, the exit code was 255 instead of 141. Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano diff --git a/run-command.c b/run-command.c index e273c6c..30c2b3d 100644 --- a/run-command.c +++ b/run-command.c @@ -248,7 +248,14 @@ static int wait_or_whine(pid_t pid, const char *argv0) } else if (waiting != pid) { error("waitpid is confused (%s)", argv0); } else if (WIFSIGNALED(status)) { - error("%s died of signal", argv0); + code = WTERMSIG(status); + error("%s died of signal %d", argv0, code); + /* + * This return value is chosen so that code & 0xff + * mimics the exit code that a POSIX shell would report for + * a program that died from this signal. + */ + code -= 128; } else if (WIFEXITED(status)) { code = WEXITSTATUS(status); /* -- cgit v0.10.2-6-g49f6 From c024beb56da679839d61f352d088b9a86823233a Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Sat, 4 Jul 2009 21:26:42 +0200 Subject: run_command: report failure to execute the program, but optionally don't In the case where a program was not found, it was still the task of the caller to report an error to the user. Usually, this is an interesting case but only few callers actually reported a specific error (though many call sites report a generic error message regardless of the cause). With this change the error is reported by run_command, but since there is one call site in git.c that does not want that, an option is added to struct child_process, which is used to turn the error off. Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c index 1dcdb1a..c85507b 100644 --- a/builtin-receive-pack.c +++ b/builtin-receive-pack.c @@ -125,9 +125,7 @@ static const char post_receive_hook[] = "hooks/post-receive"; static int run_status(int code, const char *cmd_name) { - if (code < 0 && errno == ENOENT) - return error("execute of %s failed", cmd_name); - else if (code > 0) + if (code > 0) error("%s exited with error code %d", cmd_name, code); return code; } diff --git a/git.c b/git.c index 03726ee..1824028 100644 --- a/git.c +++ b/git.c @@ -416,7 +416,7 @@ static void execv_dashed_external(const char **argv) * if we fail because the command is not found, it is * OK to return. Otherwise, we just pass along the status code. */ - status = run_command_v_opt(argv, 0); + status = run_command_v_opt(argv, RUN_SILENT_EXEC_FAILURE); if (status >= 0 || errno != ENOENT) exit(status); diff --git a/run-command.c b/run-command.c index 30c2b3d..b613bdd 100644 --- a/run-command.c +++ b/run-command.c @@ -185,7 +185,7 @@ fail_pipe: cmd->pid = mingw_spawnvpe(cmd->argv[0], cmd->argv, env); failed_errno = errno; - if (cmd->pid < 0 && errno != ENOENT) + if (cmd->pid < 0 && (!cmd->silent_exec_failure || errno != ENOENT)) error("cannot spawn %s: %s", cmd->argv[0], strerror(errno)); if (cmd->env) @@ -233,7 +233,7 @@ fail_pipe: return 0; } -static int wait_or_whine(pid_t pid, const char *argv0) +static int wait_or_whine(pid_t pid, const char *argv0, int silent_exec_failure) { int status, code = -1; pid_t waiting; @@ -264,6 +264,9 @@ static int wait_or_whine(pid_t pid, const char *argv0) if (code == 127) { code = -1; failed_errno = ENOENT; + if (!silent_exec_failure) + error("cannot run %s: %s", argv0, + strerror(ENOENT)); } } else { error("waitpid is confused (%s)", argv0); @@ -274,7 +277,7 @@ static int wait_or_whine(pid_t pid, const char *argv0) int finish_command(struct child_process *cmd) { - return wait_or_whine(cmd->pid, cmd->argv[0]); + return wait_or_whine(cmd->pid, cmd->argv[0], cmd->silent_exec_failure); } int run_command(struct child_process *cmd) @@ -294,6 +297,7 @@ static void prepare_run_command_v_opt(struct child_process *cmd, cmd->no_stdin = opt & RUN_COMMAND_NO_STDIN ? 1 : 0; cmd->git_cmd = opt & RUN_GIT_CMD ? 1 : 0; cmd->stdout_to_stderr = opt & RUN_COMMAND_STDOUT_TO_STDERR ? 1 : 0; + cmd->silent_exec_failure = opt & RUN_SILENT_EXEC_FAILURE ? 1 : 0; } int run_command_v_opt(const char **argv, int opt) @@ -358,7 +362,7 @@ int start_async(struct async *async) int finish_async(struct async *async) { #ifndef __MINGW32__ - int ret = wait_or_whine(async->pid, "child process"); + int ret = wait_or_whine(async->pid, "child process", 0); #else DWORD ret = 0; if (WaitForSingleObject(async->tid, INFINITE) != WAIT_OBJECT_0) diff --git a/run-command.h b/run-command.h index ac6c09c..0c00b25 100644 --- a/run-command.h +++ b/run-command.h @@ -31,6 +31,7 @@ struct child_process { unsigned no_stdout:1; unsigned no_stderr:1; unsigned git_cmd:1; /* if this is to be git sub-command */ + unsigned silent_exec_failure:1; unsigned stdout_to_stderr:1; void (*preexec_cb)(void); }; @@ -44,6 +45,7 @@ extern int run_hook(const char *index_file, const char *name, ...); #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 +#define RUN_SILENT_EXEC_FAILURE 8 int run_command_v_opt(const char **argv, int opt); /* diff --git a/transport.c b/transport.c index 0885801..802ce7f 100644 --- a/transport.c +++ b/transport.c @@ -396,7 +396,6 @@ static int curl_transport_push(struct transport *transport, int refspec_nr, cons { const char **argv; int argc; - int err; if (flags & TRANSPORT_PUSH_MIRROR) return error("http transport does not support mirror mode"); @@ -416,10 +415,7 @@ static int curl_transport_push(struct transport *transport, int refspec_nr, cons while (refspec_nr--) argv[argc++] = *refspec++; argv[argc] = NULL; - err = run_command_v_opt(argv, RUN_GIT_CMD); - if (err < 0 && errno == ENOENT) - error("unable to exec %s", argv[0]); - return !!err; + return !!run_command_v_opt(argv, RUN_GIT_CMD); } static struct ref *get_refs_via_curl(struct transport *transport, int for_push) -- cgit v0.10.2-6-g49f6 From 90e41a89caa464a84e13cbc9378b0348a61f713b Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Sat, 4 Jul 2009 21:26:43 +0200 Subject: receive-pack: remove unnecessary run_status report The function run_status was used to report failures after a hook was run. By now, the only thing that the function itself reported was the exit code of the hook (if it was non-zero). But this is redundant because it can be expected that the hook itself will have reported a suitable error. Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c index c85507b..b771fe9 100644 --- a/builtin-receive-pack.c +++ b/builtin-receive-pack.c @@ -123,13 +123,6 @@ static struct command *commands; static const char pre_receive_hook[] = "hooks/pre-receive"; static const char post_receive_hook[] = "hooks/post-receive"; -static int run_status(int code, const char *cmd_name) -{ - if (code > 0) - error("%s exited with error code %d", cmd_name, code); - return code; -} - static int run_receive_hook(const char *hook_name) { static char buf[sizeof(commands->old_sha1) * 2 + PATH_MAX + 4]; @@ -156,7 +149,7 @@ static int run_receive_hook(const char *hook_name) code = start_command(&proc); if (code) - return run_status(code, hook_name); + return code; for (cmd = commands; cmd; cmd = cmd->next) { if (!cmd->error_string) { size_t n = snprintf(buf, sizeof(buf), "%s %s %s\n", @@ -168,7 +161,7 @@ static int run_receive_hook(const char *hook_name) } } close(proc.in); - return run_status(finish_command(&proc), hook_name); + return finish_command(&proc); } static int run_update_hook(struct command *cmd) @@ -185,9 +178,8 @@ static int run_update_hook(struct command *cmd) argv[3] = sha1_to_hex(cmd->new_sha1); argv[4] = NULL; - return run_status(run_command_v_opt(argv, RUN_COMMAND_NO_STDIN | - RUN_COMMAND_STDOUT_TO_STDERR), - update_hook); + return run_command_v_opt(argv, RUN_COMMAND_NO_STDIN | + RUN_COMMAND_STDOUT_TO_STDERR); } static int is_ref_checked_out(const char *ref) @@ -401,7 +393,6 @@ static void run_update_post_hook(struct command *cmd) argv[argc] = NULL; status = run_command_v_opt(argv, RUN_COMMAND_NO_STDIN | RUN_COMMAND_STDOUT_TO_STDERR); - run_status(status, update_post_hook); } static void execute_commands(const char *unpacker_error) @@ -519,7 +510,6 @@ static const char *unpack(void) code = run_command_v_opt(unpacker, RUN_GIT_CMD); if (!code) return NULL; - run_status(code, unpacker[0]); return "unpack-objects abnormal exit"; } else { const char *keeper[7]; @@ -545,7 +535,6 @@ static const char *unpack(void) ip.git_cmd = 1; status = start_command(&ip); if (status) { - run_status(status, keeper[0]); return "index-pack fork failed"; } pack_lockfile = index_pack_lockfile(ip.out); @@ -555,7 +544,6 @@ static const char *unpack(void) reprepare_packed_git(); return NULL; } - run_status(status, keeper[0]); return "index-pack abnormal exit"; } } -- cgit v0.10.2-6-g49f6 From 5a7a3671b74c043216549b94a718da04cc3ffcd6 Mon Sep 17 00:00:00 2001 From: David Soria Parra Date: Tue, 4 Aug 2009 11:28:40 +0200 Subject: run-command.c: squelch a "use before assignment" warning i686-apple-darwin9-gcc-4.0.1 (GCC) 4.0.1 (Apple Inc. build 5490) compiler (and probably others) mistakenly thinks variable failed_errno is used before assigned. Work it around by giving it a fake initialization. Signed-off-by: David Soria Parra Signed-off-by: Junio C Hamano diff --git a/run-command.c b/run-command.c index b613bdd..71f8336 100644 --- a/run-command.c +++ b/run-command.c @@ -19,7 +19,7 @@ int start_command(struct child_process *cmd) { int need_in, need_out, need_err; int fdin[2], fdout[2], fderr[2]; - int failed_errno; + int failed_errno = failed_errno; /* * In case of errors we must keep the promise to close FDs -- cgit v0.10.2-6-g49f6 From 0b91322311b649a4b5f9581fec2dca9e1c2da716 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Sat, 8 Aug 2009 22:44:20 +0200 Subject: api-run-command.txt: describe error behavior of run_command functions Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano diff --git a/Documentation/technical/api-run-command.txt b/Documentation/technical/api-run-command.txt index 2efe7a4..b26c281 100644 --- a/Documentation/technical/api-run-command.txt +++ b/Documentation/technical/api-run-command.txt @@ -35,12 +35,32 @@ Functions Convenience functions that encapsulate a sequence of start_command() followed by finish_command(). The argument argv specifies the program and its arguments. The argument opt is zero - or more of the flags `RUN_COMMAND_NO_STDIN`, `RUN_GIT_CMD`, or - `RUN_COMMAND_STDOUT_TO_STDERR` that correspond to the members - .no_stdin, .git_cmd, .stdout_to_stderr of `struct child_process`. + or more of the flags `RUN_COMMAND_NO_STDIN`, `RUN_GIT_CMD`, + `RUN_COMMAND_STDOUT_TO_STDERR`, or `RUN_SILENT_EXEC_FAILURE` + that correspond to the members .no_stdin, .git_cmd, + .stdout_to_stderr, .silent_exec_failure of `struct child_process`. The argument dir corresponds the member .dir. The argument env corresponds to the member .env. +The functions above do the following: + +. If a system call failed, errno is set and -1 is returned. A diagnostic + is printed. + +. If the program was not found, then -1 is returned and errno is set to + ENOENT; a diagnostic is printed only if .silent_exec_failure is 0. + +. Otherwise, the program is run. If it terminates regularly, its exit + code is returned. No diagnistic is printed, even if the exit code is + non-zero. + +. If the program terminated due to a signal, then the return value is the + signal number - 128, ie. it is negative and so indicates an unusual + condition; a diagnostic is printed. This return value can be passed to + exit(2), which will report the same code to the parent process that a + POSIX shell's $? would report for a program that died from the signal. + + `start_async`:: Run a function asynchronously. Takes a pointer to a `struct @@ -143,6 +163,11 @@ string pointers (NULL terminated) in .env: To specify a new initial working directory for the sub-process, specify it in the .dir member. +If the program cannot be found, the functions return -1 and set +errno to ENOENT. Normally, an error message is printed, but if +.silent_exec_failure is set to 1, no message is printed for this +special error condition. + * `struct async` -- cgit v0.10.2-6-g49f6