delta/git.git/run-command.c, branch cc/codespeed

Merge branch 'js/run-process-parallel-api-fix'

2017-08-11T20:27:02+00:00

API fix.

* js/run-process-parallel-api-fix:
  run_processes_parallel: change confusing task_cb convention

run_processes_parallel: change confusing task_cb convention

2017-07-21T18:58:46+00:00

By declaring the task_cb parameter of type `void **`, the signature of
the get_next_task method suggests that the "task-specific cookie" can be
defined in that method, and the signatures of the start_failure and of
the task_finished methods declare that parameter of type `void *`,
suggesting that those methods are mere users of said cookie.

That convention makes a total lot of sense, because the tasks are pretty
much dead when one of the latter two methods is called: there would be
little use to reset that cookie at that point because nobody would be
able to see the change afterwards.

However, this is not what the code actually does. For all three methods,
it passes the *address* of pp->children[i].data.

As reasoned above, this behavior makes no sense. So let's change the
implementation to adhere to the convention suggested by the signatures.

Signed-off-by: Johannes Schindelin 
Acked-by: Stefan Beller 
Signed-off-by: Junio C Hamano

run-command: restrict PATH search to executable files

2017-04-26T06:17:36+00:00

In some situations run-command will incorrectly try (and fail) to
execute a directory instead of an executable file.  This was observed by
having a directory called "ssh" in $PATH before the real ssh and trying
to use ssh protoccol, reslting in the following:

	$ git ls-remote ssh://url
	fatal: cannot exec 'ssh': Permission denied

It ends up being worse and run-command will even try to execute a
non-executable file if it preceeds the executable version of a file on
the PATH.  For example, if PATH=~/bin1:~/bin2:~/bin3 and there exists a
directory 'git-hello' in 'bin1', a non-executable file 'git-hello' in
bin2 and an executable file 'git-hello' (which prints "Hello World!") in
bin3 the following will occur:

	$ git hello
	fatal: cannot exec 'git-hello': Permission denied

This is due to only checking 'access()' when locating an executable in
PATH, which doesn't distinguish between files and directories.  Instead
use 'is_executable()' which check that the path is to a regular,
executable file.  Now run-command won't try to execute the directory or
non-executable file 'git-hello':

	$ git hello
	Hello World!

which matches what execvp(3) would have done when asked to execute
git-hello with such a $PATH.

Reported-by: Brian Hatfield 
Signed-off-by: Brandon Williams 
Reviewed-by: Jonathan Nieder 
Signed-off-by: Junio C Hamano

run-command: expose is_executable function

2017-04-26T01:45:29+00:00

Move the logic for 'is_executable()' from help.c to run_command.c and
expose it so that callers from outside help.c can access the function.
This is to enable run-command to be able to query if a file is
executable in a future patch.

Signed-off-by: Brandon Williams 
Reviewed-by: Jonathan Nieder 
Signed-off-by: Junio C Hamano

run-command: block signals between fork and execve

2017-04-21T00:55:32+00:00

Signal handlers of the parent firing in the forked child may
have unintended side effects.  Rather than auditing every signal
handler we have and will ever have, block signals while forking
and restore default signal handlers in the child before execve.

Restoring default signal handlers is required because
execve does not unblock signals, it only restores default
signal handlers.  So we must restore them with sigprocmask
before execve, leaving a window when signal handlers
we control can fire in the child.  Continue ignoring
ignored signals, but reset the rest to defaults.

Similarly, disable pthread cancellation to future-proof our code
in case we start using cancellation; as cancellation is
implemented with signals in glibc.

Signed-off-by: Eric Wong 
Signed-off-by: Brandon Williams 
Signed-off-by: Junio C Hamano

run-command: add note about forking and threading

2017-04-21T00:55:32+00:00

All non-Async-Signal-Safe functions (e.g. malloc and die) were removed
between 'fork' and 'exec' in start_command in order to avoid potential
deadlocking when forking while multiple threads are running.  This
deadlocking is possible when a thread (other than the one forking) has
acquired a lock and didn't get around to releasing it before the fork.
This leaves the lock in a locked state in the resulting process with no
hope of it ever being released.

Add a note describing this potential pitfall before the call to 'fork()'
so people working in this section of the code know to only use
Async-Signal-Safe functions in the child process.

Signed-off-by: Brandon Williams 
Signed-off-by: Junio C Hamano

run-command: handle dup2 and close errors in child

2017-04-21T00:55:32+00:00

Signed-off-by: Brandon Williams 
Signed-off-by: Junio C Hamano

run-command: eliminate calls to error handling functions in child

2017-04-21T00:55:32+00:00

All of our standard error handling paths have the potential to
call malloc or take stdio locks; so we must avoid them inside
the forked child.

Instead, the child only writes an 8 byte struct atomically to
the parent through the notification pipe to propagate an error.
All user-visible error reporting happens from the parent;
even avoiding functions like atexit(3) and exit(3).

Helped-by: Eric Wong 
Signed-off-by: Brandon Williams 
Signed-off-by: Junio C Hamano

run-command: don't die in child when duping /dev/null

2017-04-21T00:55:32+00:00

Signed-off-by: Brandon Williams 
Signed-off-by: Junio C Hamano

run-command: prepare child environment before forking

2017-04-21T00:55:32+00:00

In order to avoid allocation between 'fork()' and 'exec()' prepare the
environment to be used in the child process prior to forking.

Switch to using 'execve()' so that the construct child environment can
used in the exec'd process.

Signed-off-by: Brandon Williams 
Signed-off-by: Junio C Hamano