undef/git-commit-vandalism
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

start_command: report child process setup errors to the parent's stderr

Browse Source 
When the child process's environment is set up in start_command(), error
messages were written to wherever the parent redirected the child's stderr
channel. However, even if the parent redirected the child's stderr, errors
during this setup process, including the exec itself, are usually an
indication of a problem in the parent's environment. Therefore, the error
messages should go to the parent's stderr.

Redirection of the child's error messages is usually only used to redirect
hook error messages during client-server exchanges. In these cases, hook
setup errors could be regarded as information leak.

This patch makes a copy of stderr if necessary and uses a special
die routine that is used for all die() calls in the child that sends the
errors messages to the parent's stderr.

The trace call that reported a failed execvp is removed (because it writes
to stderr) and replaced by die_errno() with special treatment of ENOENT.
The improvement in the error message can be seen with this sequence:

   mkdir .git/hooks/pre-commit
   git commit

Previously, the error message was

   error: cannot run .git/hooks/pre-commit: No such file or directory

and now it is

   fatal: cannot exec '.git/hooks/pre-commit': Permission denied

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Johannes Sixt 2010-01-10 14:07:52 +01:00 committed by Junio C Hamano
parent 27a557a9ff
commit a5487ddf0f
1 changed files with 43 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
run-command.c
View File

@ -15,6 +15,30 @@ static inline void dup_devnull(int to)
close(fd);
}
#ifndef WIN32
static int child_err = 2;
static NORETURN void die_child(const char *err, va_list params)
{
char msg[4096];
int len = vsnprintf(msg, sizeof(msg), err, params);
if (len > sizeof(msg))
len = sizeof(msg);
write(child_err, "fatal: ", 7);
write(child_err, msg, len);
write(child_err, "\n", 1);
exit(128);
}
static inline void set_cloexec(int fd)
{
int flags = fcntl(fd, F_GETFD);
if (flags >= 0)
fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
}
#endif
int start_command(struct child_process *cmd)
{
int need_in, need_out, need_err;
@ -79,6 +103,17 @@ fail_pipe:
fflush(NULL);
cmd->pid = fork();
if (!cmd->pid) {
/*
* Redirect the channel to write syscall error messages to
* before redirecting the process's stderr so that all die()
* in subsequent call paths use the parent's stderr.
*/
if (cmd->no_stderr || need_err) {
child_err = dup(2);
set_cloexec(child_err);
}
set_die_routine(die_child);
if (cmd->no_stdin)
dup_devnull(0);
else if (need_in) {
@ -126,9 +161,14 @@ fail_pipe:
} else {
execvp(cmd->argv[0], (char *const*) cmd->argv);
}
trace_printf("trace: exec '%s' failed: %s\n", cmd->argv[0],
strerror(errno));
exit(127);
/*
* Do not check for cmd->silent_exec_failure; the parent
* process will check it when it sees this exit code.
*/
if (errno == ENOENT)
exit(127);
else
die_errno("cannot exec '%s'", cmd->argv[0]);
}
if (cmd->pid < 0)
error("cannot fork() for %s: %s", cmd->argv[0],