Merge branch 'jk/run-command-capture'

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"

diff --combined run-command.c
index 3afb124c79dcb234344e17795205153d4a2e0643,4184e8d9f3a61459df1d4fa4cdec8a59386f1f17..aad03ab705f301268980079282fcf370b9275c5a
--- 1/run-command.c
--- 2/run-command.c
+++ b/run-command.c
@@@ -4,6 -4,10 +4,6 @@@
  #include "sigchain.h"
  #include "argv-array.h"
  
 -#ifndef SHELL_PATH
 -# define SHELL_PATH "/bin/sh"
 -#endif
 -
  void child_process_init(struct child_process *child)
  {
        memset(child, 0, sizeof(*child));
@@@ -557,7 -561,12 +557,12 @@@ int finish_command(struct child_proces
  
  int run_command(struct child_process *cmd)
  {
-       int code = start_command(cmd);
+       int code;
+ 
+       if (cmd->out < 0 || cmd->err < 0)
+               die("BUG: run_command with a pipe can cause deadlock");
+ 
+       code = start_command(cmd);
        if (code)
                return code;
        return finish_command(cmd);
@@@ -829,3 -838,19 +834,19 @@@ int run_hook_le(const char *const *env
  
        return ret;
  }
+ 
+ int capture_command(struct child_process *cmd, struct strbuf *buf, size_t hint)
+ {
+       cmd->out = -1;
+       if (start_command(cmd) < 0)
+               return -1;
+ 
+       if (strbuf_read(buf, cmd->out, hint) < 0) {
+               close(cmd->out);
+               finish_command(cmd); /* throw away exit code */
+               return -1;
+       }
+ 
+       close(cmd->out);
+       return finish_command(cmd);
+ }

diff --combined wt-status.c
index 7036fa28dc39f72ed2064ca0ad15f2e381808020,ef232a74beb7121e176da692a893a7cd66dbdd85..853419f05f232045cf4ad3f8a8a2af04be9329a0
--- 1/wt-status.c
--- 2/wt-status.c
+++ b/wt-status.c
@@@ -729,7 -729,6 +729,6 @@@ static void wt_status_print_submodule_s
        struct strbuf cmd_stdout = STRBUF_INIT;
        struct strbuf summary = STRBUF_INIT;
        char *summary_content;
-       size_t len;
  
        argv_array_pushf(&sm_summary.env_array, "GIT_INDEX_FILE=%s",
                         s->index_file);
@@@ -745,15 -744,11 +744,11 @@@
  
        sm_summary.git_cmd = 1;
        sm_summary.no_stdin = 1;
-       fflush(s->fp);
-       sm_summary.out = -1;
  
-       run_command(&sm_summary);
- 
-       len = strbuf_read(&cmd_stdout, sm_summary.out, 1024);
+       capture_command(&sm_summary, &cmd_stdout, 1024);
  
        /* prepend header, only if there's an actual output */
-       if (len) {
+       if (cmd_stdout.len) {
                if (uncommitted)
                        strbuf_addstr(&summary, _("Submodules changed but not updated:"));
                else
@@@ -764,6 -759,7 +759,7 @@@
        strbuf_release(&cmd_stdout);
  
        if (s->display_comment_prefix) {
+               size_t len;
                summary_content = strbuf_detach(&summary, &len);
                strbuf_add_commented_lines(&summary, summary_content, len);
                free(summary_content);
@@@ -849,8 -845,6 +845,8 @@@ static void wt_status_print_verbose(str
  {
        struct rev_info rev;
        struct setup_revision_opt opt;
 +      int dirty_submodules;
 +      const char *c = color(WT_STATUS_HEADER, s);
  
        init_revisions(&rev, NULL);
        DIFF_OPT_SET(&rev.diffopt, ALLOW_TEXTCONV);
@@@ -875,25 -869,7 +871,25 @@@
                rev.diffopt.use_color = 0;
                wt_status_add_cut_line(s->fp);
        }
 +      if (s->verbose > 1 && s->commitable) {
 +              /* print_updated() printed a header, so do we */
 +              if (s->fp != stdout)
 +                      wt_status_print_trailer(s);
 +              status_printf_ln(s, c, _("Changes to be committed:"));
 +              rev.diffopt.a_prefix = "c/";
 +              rev.diffopt.b_prefix = "i/";
 +      } /* else use prefix as per user config */
        run_diff_index(&rev, 1);
 +      if (s->verbose > 1 &&
 +          wt_status_check_worktree_changes(s, &dirty_submodules)) {
 +              status_printf_ln(s, c,
 +                      "--------------------------------------------------");
 +              status_printf_ln(s, c, _("Changes not staged for commit:"));
 +              setup_work_tree();
 +              rev.diffopt.a_prefix = "i/";
 +              rev.diffopt.b_prefix = "w/";
 +              run_diff_files(&rev, 0);
 +      }
  }
  
  static void wt_status_print_tracking(struct wt_status *s)
@@@ -1242,8 -1218,6 +1238,8 @@@ static void wt_status_get_detached_from
                state->detached_from =
                        xstrdup(find_unique_abbrev(cb.nsha1, DEFAULT_ABBREV));
        hashcpy(state->detached_sha1, cb.nsha1);
 +      state->detached_at = !get_sha1("HEAD", sha1) &&
 +                           !hashcmp(sha1, state->detached_sha1);
  
        free(ref);
        strbuf_release(&cb.buf);
@@@ -1332,8 -1306,10 +1328,8 @@@ void wt_status_print(struct wt_status *
                                on_what = _("rebase in progress; onto ");
                                branch_name = state.onto;
                        } else if (state.detached_from) {
 -                              unsigned char sha1[20];
                                branch_name = state.detached_from;
 -                              if (!get_sha1("HEAD", sha1) &&
 -                                  !hashcmp(sha1, state.detached_sha1))
 +                              if (state.detached_at)
                                        on_what = _("HEAD detached at ");
                                else
                                        on_what = _("HEAD detached from ");