Merge branch 'js/run-command'

* js/run-command:
start_command(), if .in/.out > 0, closes file descriptors, not the callers
start_command(), .in/.out/.err = -1: Callers must close the file descriptor

diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index f40135248a5a1e2012c4cb01667f037407aae800..5ea48ca7dbc22785f4e4f3c35f3dc48ee18f0d8a 100644 (file)
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -538,8 +538,10 @@ static int get_pack(int xd[2], char **pack_lockfile)
        cmd.git_cmd = 1;
        if (start_command(&cmd))
                die("fetch-pack: unable to fork off %s", argv[0]);
-       if (do_keep && pack_lockfile)
+       if (do_keep && pack_lockfile) {
                *pack_lockfile = index_pack_lockfile(cmd.out);
+               close(cmd.out);
+       }
 
        if (finish_command(&cmd))
                die("%s failed", argv[0]);

diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index 8afb1d0bca0635dc22f658455477d9fada231290..b0cfae83fc4dcdd8f58e29b87fd8aa4b6af0f6c0 100644 (file)
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -71,6 +71,7 @@ static int pack_objects(int fd, struct ref *refs)
                refs = refs->next;
        }
 
+       close(po.in);
        if (finish_command(&po))
                return error("pack-objects died with strange error");
        return 0;
@@ -403,12 +404,15 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest
        if (!remote_tail)
                remote_tail = &remote_refs;
        if (match_refs(local_refs, remote_refs, &remote_tail,
-                                              nr_refspec, refspec, flags))
+                      nr_refspec, refspec, flags)) {
+               close(out);
                return -1;
+       }
 
        if (!remote_refs) {
                fprintf(stderr, "No refs in common and none specified; doing nothing.\n"
                        "Perhaps you should specify a branch such as 'master'.\n");
+               close(out);
                return 0;
        }
 
@@ -495,12 +499,11 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest
 
        packet_flush(out);
        if (new_refs && !args.dry_run) {
-               if (pack_objects(out, remote_refs) < 0) {
-                       close(out);
+               if (pack_objects(out, remote_refs) < 0)
                        return -1;
-               }
        }
-       close(out);
+       else
+               close(out);
 
        if (expect_status_report)
                ret = receive_status(in, remote_refs);
@@ -648,7 +651,7 @@ int send_pack(struct send_pack_args *my_args,
        conn = git_connect(fd, dest, args.receivepack, args.verbose ? CONNECT_VERBOSE : 0);
        ret = do_send_pack(fd[0], fd[1], remote, dest, nr_heads, heads);
        close(fd[0]);
-       close(fd[1]);
+       /* do_send_pack always closes fd[1] */
        ret |= finish_connect(conn);
        return !!ret;
 }

diff --git a/builtin-tag.c b/builtin-tag.c
index 716b4fff323f7df638f8bed0940a1119e43c2590..28c36fdcd1658968ff7c3e2f1d6ba6f364f99592 100644 (file)
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -226,12 +226,13 @@ static int do_sign(struct strbuf *buffer)
 
        if (write_in_full(gpg.in, buffer->buf, buffer->len) != buffer->len) {
                close(gpg.in);
+               close(gpg.out);
                finish_command(&gpg);
                return error("gpg did not accept the tag data");
        }
        close(gpg.in);
-       gpg.close_in = 0;
        len = strbuf_read(buffer, gpg.out, 1024);
+       close(gpg.out);
 
        if (finish_command(&gpg) || !len || len < 0)
                return error("gpg failed to sign the tag");

diff --git a/builtin-verify-tag.c b/builtin-verify-tag.c
index cc4c55d7ee35ceeaf8c4a857d5dad857a7eb2664..f3ef11fa2d582682b428d6e165da65f05e2d7511 100644 (file)
--- a/builtin-verify-tag.c
+++ b/builtin-verify-tag.c
@@ -45,14 +45,12 @@ static int run_gpg_verify(const char *buf, unsigned long size, int verbose)
        memset(&gpg, 0, sizeof(gpg));
        gpg.argv = args_gpg;
        gpg.in = -1;
-       gpg.out = 1;
        args_gpg[2] = path;
        if (start_command(&gpg))
                return error("could not run gpg.");
 
        write_in_full(gpg.in, buf, len);
        close(gpg.in);
-       gpg.close_in = 0;
        ret = finish_command(&gpg);
 
        unlink(path);

diff --git a/bundle.c b/bundle.c
index bd12ec8537781c5c82e77637312ccabb708d5040..0ba5df17e15d679b03fe38af40260c118c9588fa 100644 (file)
--- a/bundle.c
+++ b/bundle.c
@@ -333,10 +333,12 @@ int create_bundle(struct bundle_header *header, const char *path,
                write_or_die(rls.in, sha1_to_hex(object->sha1), 40);
                write_or_die(rls.in, "\n", 1);
        }
+       close(rls.in);
        if (finish_command(&rls))
                return error ("pack-objects died");
-
-       return bundle_to_stdout ? close(bundle_fd) : commit_lock_file(&lock);
+       if (!bundle_to_stdout)
+               commit_lock_file(&lock);
+       return 0;
 }
 
 int unbundle(struct bundle_header *header, int bundle_fd)

diff --git a/receive-pack.c b/receive-pack.c
index 326749583221d0f5e81f58608a23ab1dc1601177..a971433db155bbac162cece038b73dc64e682ac0 100644 (file)
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -132,6 +132,7 @@ static int run_hook(const char *hook_name)
                                break;
                }
        }
+       close(proc.in);
        return hook_status(finish_command(&proc), hook_name);
 }
 
@@ -414,6 +415,7 @@ static const char *unpack(void)
                if (start_command(&ip))
                        return "index-pack fork failed";
                pack_lockfile = index_pack_lockfile(ip.out);
+               close(ip.out);
                status = finish_command(&ip);
                if (!status) {
                        reprepare_packed_git();

diff --git a/run-command.c b/run-command.c
index 476d00c2182e3af82a0cfe495c61c9df1eb44d26..743757c36ec0e5667fd8af28800ac095f1581adb 100644 (file)
--- a/run-command.c
+++ b/run-command.c
@@ -20,12 +20,19 @@ int start_command(struct child_process *cmd)
        int need_in, need_out, need_err;
        int fdin[2], fdout[2], fderr[2];
 
+       /*
+        * In case of errors we must keep the promise to close FDs
+        * that have been passed in via ->in and ->out.
+        */
+
        need_in = !cmd->no_stdin && cmd->in < 0;
        if (need_in) {
-               if (pipe(fdin) < 0)
+               if (pipe(fdin) < 0) {
+                       if (cmd->out > 0)
+                               close(cmd->out);
                        return -ERR_RUN_COMMAND_PIPE;
+               }
                cmd->in = fdin[1];
-               cmd->close_in = 1;
        }
 
        need_out = !cmd->no_stdout
@@ -35,10 +42,11 @@ int start_command(struct child_process *cmd)
                if (pipe(fdout) < 0) {
                        if (need_in)
                                close_pair(fdin);
+                       else if (cmd->in)
+                               close(cmd->in);
                        return -ERR_RUN_COMMAND_PIPE;
                }
                cmd->out = fdout[0];
-               cmd->close_out = 1;
        }
 
        need_err = !cmd->no_stderr && cmd->err < 0;
@@ -46,8 +54,12 @@ int start_command(struct child_process *cmd)
                if (pipe(fderr) < 0) {
                        if (need_in)
                                close_pair(fdin);
+                       else if (cmd->in)
+                               close(cmd->in);
                        if (need_out)
                                close_pair(fdout);
+                       else if (cmd->out)
+                               close(cmd->out);
                        return -ERR_RUN_COMMAND_PIPE;
                }
                cmd->err = fderr[0];
@@ -57,8 +69,12 @@ int start_command(struct child_process *cmd)
        if (cmd->pid < 0) {
                if (need_in)
                        close_pair(fdin);
+               else if (cmd->in)
+                       close(cmd->in);
                if (need_out)
                        close_pair(fdout);
+               else if (cmd->out)
+                       close(cmd->out);
                if (need_err)
                        close_pair(fderr);
                return -ERR_RUN_COMMAND_FORK;
@@ -120,7 +136,7 @@ int start_command(struct child_process *cmd)
 
        if (need_out)
                close(fdout[1]);
-       else if (cmd->out > 1)
+       else if (cmd->out)
                close(cmd->out);
 
        if (need_err)
@@ -157,10 +173,6 @@ static int wait_or_whine(pid_t pid)
 
 int finish_command(struct child_process *cmd)
 {
-       if (cmd->close_in)
-               close(cmd->in);
-       if (cmd->close_out)
-               close(cmd->out);
        return wait_or_whine(cmd->pid);
 }
 

diff --git a/run-command.h b/run-command.h
index 1fc781d7668468f9e74bd430b7569dc040440ba8..debe3074b5a01fb5a19e61f07ff66c250cdc4f82 100644 (file)
--- a/run-command.h
+++ b/run-command.h
@@ -14,13 +14,29 @@ enum {
 struct child_process {
        const char **argv;
        pid_t pid;
+       /*
+        * Using .in, .out, .err:
+        * - Specify 0 for no redirections (child inherits stdin, stdout,
+        *   stderr from parent).
+        * - Specify -1 to have a pipe allocated as follows:
+        *     .in: returns the writable pipe end; parent writes to it,
+        *          the readable pipe end becomes child's stdin
+        *     .out, .err: returns the readable pipe end; parent reads from
+        *          it, the writable pipe end becomes child's stdout/stderr
+        *   The caller of start_command() must close the returned FDs
+        *   after it has completed reading from/writing to it!
+        * - Specify > 0 to set a channel to a particular FD as follows:
+        *     .in: a readable FD, becomes child's stdin
+        *     .out: a writable FD, becomes child's stdout/stderr
+        *     .err > 0 not supported
+        *   The specified FD is closed by start_command(), even in case
+        *   of errors!
+        */
        int in;
        int out;
        int err;
        const char *dir;
        const char *const *env;
-       unsigned close_in:1;
-       unsigned close_out:1;
        unsigned no_stdin:1;
        unsigned no_stdout:1;
        unsigned no_stderr:1;