rework run_command error reporting
cover rework run_command error reporting
I got tired of thinking about which return values of run_command functions are exit codes and which are error codes of system call failures.
Furthermore, it is a pitty that almost no run_command callers correctly report system call failures.
But on top of all I find it unacceptable that recent git-daemon logs "unable to run 'git-pack-objects'" if a client terminates the connection early.
This series addresses these issues, and also fixes an error in git-bisect on Windows.
1/7 Truncate result of run_command that is used in exit() to lowest 8 bits
This one fixes a breakage of git-bisect on Windows and should be applied at any rate.
2/7 MinGW: simplify waitpid() emulation macros
3/7 run_command: return exit code as positive value
4/7 run_command: report system call errors instead of returning error codes
These address the mentioned issues; 4/7 is the important change.
5/7 run_command: encode deadly signal number in the return value 6/7 run_command: report failure to execute the program, but optionally don't
These two are nice to have.
7/7 receive-pack: remove unnecessary run_status report
This one is actually RFC because it removes a status report that users might be used to see, or even depend on.
bisect.c | 4 +- builtin-add.c | 2 +- builtin-merge.c | 2 +- builtin-receive-pack.c | 38 ++-------------- compat/mingw.h | 5 +- convert.c | 2 +- git.c | 10 +--- ll-merge.c | 4 -- run-command.c | 105 +++++++++++++++++++++++------------------ run-command.h | 13 +---- t/t5530-upload-pack-error.sh | 5 ++- transport.c | 16 +------ 12 files changed, 81 insertions(+), 125 deletions(-)
PATCH 1/7 Truncate result of run_command that is used in exit() to lowest 8 bits
On Windows, negative return values are not detected reliably by the parent process as failure. This fixes it.
--- bisect.c | 4 ++-- builtin-add.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
bisect.c +2 -2
@@ -760,7 +760,7 @@ static int bisect_checkout(char *bisect_rev_hex) argv_checkout[2 ] = bisect_rev_hex; res =run_command_v_opt (argv_checkout,RUN_GIT_CMD );if (res) -exit (res); +exit (res &0xff ); argv_show_branch[1 ] = bisect_rev_hex;return run_command_v_opt (argv_show_branch,RUN_GIT_CMD );
@@ -850,7 +850,7 @@ static void check_merge_bases(void)handle_skipped_merge_base (mb); }else {printf ("Bisecting: a merge base must be tested\n" ); -exit (bisect_checkout (sha1_to_hex (mb))); +exit (bisect_checkout (sha1_to_hex (mb)) &0xff ); } }
builtin-add.c +1 -1
@@ -305,7 +305,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)if (patch_interactive) add_interactive =1 ;if (add_interactive) -exit (interactive_add (argc -1 , argv +1 , prefix)); +exit (interactive_add (argc -1 , argv +1 , prefix) &0xff );if (edit_interactive)return (edit_patch (argc, argv, prefix));
Signature · 1 line
1.6.3.17.g1665f
PATCH 1/7 MinGW: truncate exit()'s argument to lowest 8 bits
For some reason, MinGW's bash cannot reliably detect failure of the child process if a negative value is passed to exit(). This fixes it by truncating the exit code in all calls of exit().
This issue was worked around in run_builtin() of git.c (2488df84 builtinrun_command: do not exit with -1, 2007-11-15). This workaround is no longer necessary and is reverted.
--- compat/mingw.h | 2 ++ git.c | 2 +- 2 files changed, 3 insertions(+), 1 deletions(-)
compat/mingw.h +2 -0
@@ -92,6 +92,8 @@ static inline int fcntl(int fd, int cmd, long arg) errno =EINVAL ;return -1 ; } +/* bash cannot reliably detect negative return codes as failure */ +#define exit (code) exit((code) & 0xff)/* * simple adaptors
git.c +1 -1
@@ -245,7 +245,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv) status = p->fn (argc, argv, prefix);if (status) -return status &0xff ; +return status;/* Somebody closed stdout? */ if (fstat (fileno (stdout), &st))
Signature · 1 line
1.6.3.3.393.g6b649
PATCH 2/7 MinGW: simplify waitpid() emulation macros
In reply to Johannes SixtWindows 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.
---
The dummy definition WTERMSIG will be used in a subsequent patch.
compat/mingw.h | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)
compat/mingw.h +3 -2
@@ -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
Signature · 1 line
1.6.3.17.g1665f
PATCH 3/7 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.
--- builtin-merge.c | 2 +- builtin-receive-pack.c | 4 ++-- convert.c | 2 +- git.c | 4 ++-- ll-merge.c | 4 ---- run-command.c | 9 +-------- run-command.h | 1 - 7 files changed, 7 insertions(+), 19 deletions(-)
builtin-merge.c +1 -1
@@ -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; } }
builtin-receive-pack.c +2 -2
@@ -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; } }
convert.c +1 -1
@@ -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); }
git.c +2 -2
@@ -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 */
ll-merge.c +0 -4
@@ -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;
run-command.c +1 -8
@@ -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; } }
run-command.h +0 -1
@@ -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;
Signature · 1 line
1.6.3.17.g1665f
PATCH 4/7 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.
--- builtin-receive-pack.c | 22 +--------- git.c | 8 +--- run-command.c | 89 +++++++++++++++++++++++------------------- run-command.h | 10 ----- t/t5530-upload-pack-error.sh | 5 ++- transport.c | 12 +----- 6 files changed, 59 insertions(+), 87 deletions(-)
builtin-receive-pack.c +3 -19
@@ -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)
git.c +2 -6
@@ -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 ]); -exit (status); - } - errno =ENOENT ;/* as if we called execvp */ +if (status >=0 || errno !=ENOENT ) +exit (status &0xff ); argv[0 ] = tmp;
run-command.c +49 -40
174 lines not on this page · raw message
run-command.h +0 -10
@@ -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;
t/t5530-upload-pack-error.sh +4 -1
@@ -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' '
transport.c +1 -11
@@ -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; }
Signature · 1 line
1.6.3.17.g1665f
PATCH 5/7 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
141Previously, the exit code was 255 instead of 141.
--- run-command.c | 9 ++++++++- 1 files changed, 8 insertions(+), 1 deletions(-)
run-command.c +8 -1
@@ -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);/*
Signature · 1 line
1.6.3.17.g1665f
PATCH 6/7 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.
--- builtin-receive-pack.c | 4 +--- git.c | 2 +- run-command.c | 12 ++++++++---- run-command.h | 2 ++ transport.c | 6 +----- 5 files changed, 13 insertions(+), 13 deletions(-)
builtin-receive-pack.c +1 -3
@@ -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; }
git.c +1 -1
@@ -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 &0xff );
run-command.c +8 -4
@@ -184,7 +184,7 @@ fail_pipe: } cmd->pid =mingw_spawnvpe (cmd->argv[0 ], cmd->argv, env); -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 (failed_errno = errno));
@@ -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 )
run-command.h +2 -0
@@ -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 8int run_command_v_opt (const char **argv,int opt);/*
transport.c +1 -5
@@ -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, conswhile (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)
Signature · 1 line
1.6.3.17.g1665f
PATCH 7/7 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.
--- builtin-receive-pack.c | 20 ++++---------------- 1 files changed, 4 insertions(+), 16 deletions(-)
builtin-receive-pack.c +4 -16
@@ -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" ; } }
Signature · 1 line
1.6.3.17.g1665f
Re: [PATCH 4/7] run_command: report system call errors instead of returning error codes
In reply to Johannes SixtReview
Quoted 2 lines · Johannes Sixt
(quoted diff) +2
-0+ if (status >= 0 || errno != ENOENT) + exit(status & 0xff);
With my replacement patch 1/7 this '& 0xff' is no longer necessary. Would you kindly squash this in?
-- Hannes
git.c +1 -1
@@ -418,7 +418,7 @@ static void execv_dashed_external(const char **argv)*/ status =run_command_v_opt (argv,RUN_SILENT_EXEC_FAILURE );if (status >=0 || errno !=ENOENT ) -exit (status &0xff ); +exit (status); argv[0 ] = tmp;
Re: [PATCH 4/7] run_command: report system call errors instead of returning error codes
Quoted 6 lines · Johannes Sixt
Review
Quoted 2 lines · Johannes Sixt
(quoted diff) +2
-0+ if (status >= 0 || errno != ENOENT) + exit(status & 0xff);With my replacement patch 1/7 this '&
0xff' is no longer necessary. Would you kindly squash this in?
Sounds sensible. Thanks.
Re: [PATCH 4/7] run_command: report system call errors instead of returning error codes
In reply to Johannes SixtQuoted 16 lines · Johannes Sixt
(quoted diff) +3
-1cmd->pid = mingw_spawnvpe(cmd->argv[0], cmd->argv, env); + if (cmd->pid < 0 && errno != ENOENT) + error("cannot spawn %s: %s", cmd->argv[0], + strerror(failed_errno = 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)
Review int start_command(struct child_process *cmd) · line 218
Quoted 9 lines
(quoted diff) +2
-3@@ -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; }
Sorry to bother you once again. 'gcc -O2' correctly points out that failed_errno could be used uninitialized (MinGW code path only). Would you please squash this in?
-- Hannes
run-command.c +2 -2
@@ -184,9 +184,9 @@ fail_pipe: } cmd->pid =mingw_spawnvpe (cmd->argv[0 ], cmd->argv, env); + failed_errno = errno;if (cmd->pid <0 && (!cmd->silent_exec_failure || errno !=ENOENT )) -error ("cannot spawn %s: %s" , cmd->argv[0 ], -strerror (failed_errno = errno)); +error ("cannot spawn %s: %s" , cmd->argv[0 ],strerror (errno));if (cmd->env)free_environ (env);