git/run-command.c, branch v2.4.9

Merge branch 'jk/run-command-capture'

2015-03-25T19:54:27Z

The run-command interface was easy to abuse and make a pipe for us to read from the process, wait for the process to finish and then attempt to read its output, which is a pattern that lead to a deadlock. Fix such uses by introducing a helper to do this correctly (i.e. we need to read first and then wait the process to finish) and also add code to prevent such abuse in the run-command helper. * jk/run-command-capture: run-command: forbid using run_command with piped output trailer: use capture_command submodule: use capture_command wt-status: use capture_command run-command: introduce capture_command helper wt_status: fix signedness mismatch in strbuf_read call wt-status: don't flush before running "submodule status"

run-command: forbid using run_command with piped output

2015-03-23T04:39:22Z

Because run_command both spawns and wait()s for the command before returning control to the caller, any reads from the pipes we open must necessarily happen after wait() returns. This can lead to deadlock, as the child process may block on writing to us while we are blocked waiting for it to exit. Worse, it only happens when the child fills the pipe buffer, which means that the problem may come and go depending on the platform and the size of the output produced by the child. Let's detect and flag this dangerous construct so that we can catch potential bugs early in the test suite rather than having them happen in the field. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

run-command: introduce capture_command helper

2015-03-23T04:38:31Z

Something as simple as reading the stdout from a command turns out to be rather hard to do right. Doing: cmd.out = -1; run_command(&cmd); strbuf_read(&buf, cmd.out, 0); can result in deadlock if the child process produces a large amount of output. What happens is: 1. The parent spawns the child with its stdout connected to a pipe, of which the parent is the sole reader. 2. The parent calls wait(), blocking until the child exits. 3. The child writes to stdout. If it writes more data than the OS pipe buffer can hold, the write() call will block. This is a deadlock; the parent is waiting for the child to exit, and the child is waiting for the parent to call read(). So we might try instead: start_command(&cmd); strbuf_read(&buf, cmd.out, 0); finish_command(&cmd); But that is not quite right either. We are examining cmd.out and running finish_command whether start_command succeeded or not, which is wrong. Moreover, these snippets do not do any error handling. If our read() fails, we must make sure to still call finish_command (to reap the child process). And both snippets failed to close the cmd.out descriptor, which they must do (provided start_command succeeded). Let's introduce a run-command helper that can make this a bit simpler for callers to get right. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

git-compat-util.h: move SHELL_PATH default into header

2015-03-10T22:11:24Z

If SHELL_PATH is not defined we use "/bin/sh". However, run-command.c is not the only file that needs to use the default value so move it into a common header. Signed-off-by: Kyle J. McKay Signed-off-by: Junio C Hamano

Merge branch 'jc/hook-cleanup'

2014-12-22T20:27:10Z

Remove unused code. * jc/hook-cleanup: run-command.c: retire unused run_hook_with_custom_index()

run-command.c: retire unused run_hook_with_custom_index()

2014-12-01T16:39:43Z

This was originally meant to be used to rewrite run_commit_hook() that only special cases the GIT_INDEX_FILE environment, but the run_hook_ve() refactoring done earlier made the implementation of run_commit_hook() thin and clean enough. Nobody uses this, so retire it as an unfinished clean-up made unnecessary. Signed-off-by: Junio C Hamano

run-command: use void to declare that functions take no parameters

2014-11-10T22:43:19Z

Explicitly declare that git_atexit_dispatch() and git_atexit_clear() take no parameters instead of leaving their parameter list empty and thus unspecified. Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano

Merge branch 'eb/no-pthreads'

2014-10-24T21:59:10Z

Allow us build with NO_PTHREADS=NoThanks compilation option. * eb/no-pthreads: Handle atexit list internaly for unthreaded builds pack-objects: set number of threads before checking and warning index-pack: fix compilation with NO_PTHREADS

Handle atexit list internaly for unthreaded builds

2014-10-19T22:38:30Z

Wrap atexit()s calls on unthreaded builds to handle callback list internally. This is needed because on unthreaded builds, asyncs inherits parent's atexit() list, that gets run as soon as the async exit()s (and again at the end of async's parent process). That led to remove temporary files too early. Also remove a by-atexit-callback guard against this kind of issue in clone.c, as this patch makes it redundant. Fixes test 5537 (temporary shallow file vanished before unpack-objects could open it) BTW remove an unused variable in shallow.c. Helped-by: Duy Nguyen Helped-by: Andreas Schwab Helped-by: Junio C Hamano Signed-off-by: Etienne Buira Signed-off-by: Junio C Hamano

run-command: add env_array, an optional argv_array for env

2014-10-19T22:26:31Z

Similar to args, add a struct argv_array member to struct child_process that simplifies specifying the environment for children. It is freed automatically by finish_command() or if start_command() encounters an error. Suggested-by: Jeff King Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano