From: Johannes Sixt Date: Sun, 10 Jan 2010 13:11:22 +0000 (+0100) Subject: start_command: detect execvp failures early X-Git-Tag: v1.7.0-rc0~62^2~1 X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=2b541bf8be2bbd6cc8daf8e3d5d4a8ee30b2ce4e;p=git.git start_command: detect execvp failures early Previously, failures during execvp could be detected only by finish_command. However, in some situations it is beneficial for the parent process to know earlier that the child process will not run. The idea to use a pipe to signal failures to the parent process and the test case were lifted from patches by Ilari Liusvaara. Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano --- diff --git a/Makefile b/Makefile index 87fc7ff47..22c1546e2 100644 --- a/Makefile +++ b/Makefile @@ -1785,6 +1785,7 @@ TEST_PROGRAMS += test-genrandom$X TEST_PROGRAMS += test-match-trees$X TEST_PROGRAMS += test-parse-options$X TEST_PROGRAMS += test-path-utils$X +TEST_PROGRAMS += test-run-command$X TEST_PROGRAMS += test-sha1$X TEST_PROGRAMS += test-sigchain$X diff --git a/run-command.c b/run-command.c index dccac37c4..efe9fe413 100644 --- a/run-command.c +++ b/run-command.c @@ -17,6 +17,12 @@ static inline void dup_devnull(int to) #ifndef WIN32 static int child_err = 2; +static int child_notifier = -1; + +static void notify_parent(void) +{ + write(child_notifier, "", 1); +} static NORETURN void die_child(const char *err, va_list params) { @@ -142,6 +148,11 @@ fail_pipe: trace_argv_printf(cmd->argv, "trace: run_command:"); #ifndef WIN32 +{ + int notify_pipe[2]; + if (pipe(notify_pipe)) + notify_pipe[0] = notify_pipe[1] = -1; + fflush(NULL); cmd->pid = fork(); if (!cmd->pid) { @@ -156,6 +167,11 @@ fail_pipe: } set_die_routine(die_child); + close(notify_pipe[0]); + set_cloexec(notify_pipe[1]); + child_notifier = notify_pipe[1]; + atexit(notify_parent); + if (cmd->no_stdin) dup_devnull(0); else if (need_in) { @@ -196,8 +212,16 @@ fail_pipe: unsetenv(*cmd->env); } } - if (cmd->preexec_cb) + if (cmd->preexec_cb) { + /* + * We cannot predict what the pre-exec callback does. + * Forgo parent notification. + */ + close(child_notifier); + child_notifier = -1; + cmd->preexec_cb(); + } if (cmd->git_cmd) { execv_git_cmd(cmd->argv); } else { @@ -215,6 +239,27 @@ fail_pipe: if (cmd->pid < 0) error("cannot fork() for %s: %s", cmd->argv[0], strerror(failed_errno = errno)); + + /* + * Wait for child's execvp. If the execvp succeeds (or if fork() + * failed), EOF is seen immediately by the parent. Otherwise, the + * child process sends a single byte. + * Note that use of this infrastructure is completely advisory, + * therefore, we keep error checks minimal. + */ + close(notify_pipe[1]); + if (read(notify_pipe[0], ¬ify_pipe[1], 1) == 1) { + /* + * At this point we know that fork() succeeded, but execvp() + * failed. Errors have been reported to our stderr. + */ + wait_or_whine(cmd->pid, cmd->argv[0], + cmd->silent_exec_failure); + failed_errno = errno; + cmd->pid = -1; + } + close(notify_pipe[0]); +} #else { int s0 = -1, s1 = -1, s2 = -1; /* backups of stdin, stdout, stderr */ diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh new file mode 100755 index 000000000..10b26e4d8 --- /dev/null +++ b/t/t0061-run-command.sh @@ -0,0 +1,14 @@ +#!/bin/sh +# +# Copyright (c) 2009 Ilari Liusvaara +# + +test_description='Test run command' + +. ./test-lib.sh + +test_expect_success 'start_command reports ENOENT' ' + test-run-command start-command-ENOENT ./does-not-exist +' + +test_done diff --git a/test-run-command.c b/test-run-command.c new file mode 100644 index 000000000..0612bfa7c --- /dev/null +++ b/test-run-command.c @@ -0,0 +1,35 @@ +/* + * test-run-command.c: test run command API. + * + * (C) 2009 Ilari Liusvaara + * + * This code is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include "git-compat-util.h" +#include "run-command.h" +#include +#include + +int main(int argc, char **argv) +{ + struct child_process proc; + + memset(&proc, 0, sizeof(proc)); + + if (argc < 3) + return 1; + proc.argv = (const char **)argv+2; + + if (!strcmp(argv[1], "start-command-ENOENT")) { + if (start_command(&proc) < 0 && errno == ENOENT) + return 0; + fprintf(stderr, "FAIL %s\n", argv[1]); + return 1; + } + + fprintf(stderr, "check usage\n"); + return 1; +}