start_command(), .in/.out/.err = -1: Callers must close the file descriptor

By setting .in, .out, or .err members of struct child_process to -1, the
callers of start_command() can request that a pipe is allocated that talks
to the child process and one end is returned by replacing -1 with the
file descriptor.

Previously, a flag was set (for .in and .out, but not .err) to signal
finish_command() to close the pipe end that start_command() had handed out,
so it was optional for callers to close the pipe, and many already do so.
Now we make it mandatory to close the pipe.

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

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]);
        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);
                *pack_lockfile = index_pack_lockfile(cmd.out);

+               close(cmd.out);
+       }

 
        if (finish_command(&cmd))
                die("%s failed", argv[0]);
 
        if (finish_command(&cmd))
                die("%s failed", argv[0]);

diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index 8afb1d0bca0635dc22f658455477d9fada231290..ba9bc91a5c513273ba837dd04aa568f8f67b7121 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;
        }
 
                refs = refs->next;
        }
 

+       close(po.in);

        if (finish_command(&po))
                return error("pack-objects died with strange error");
        return 0;
        if (finish_command(&po))
                return error("pack-objects died with strange error");
        return 0;

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);
 
        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);
                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);
        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");
 
        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..b3010f9827a9df6c3113fc0b90a8fbde4cf18bb7 100644 (file)
--- a/builtin-verify-tag.c
+++ b/builtin-verify-tag.c
@@ -52,7 +52,6 @@ static int run_gpg_verify(const char *buf, unsigned long size, int verbose)
 
        write_in_full(gpg.in, buf, len);
        close(gpg.in);
 
        write_in_full(gpg.in, buf, len);
        close(gpg.in);

-       gpg.close_in = 0;

        ret = finish_command(&gpg);
 
        unlink(path);
        ret = finish_command(&gpg);
 
        unlink(path);

diff --git a/bundle.c b/bundle.c
index bd12ec8537781c5c82e77637312ccabb708d5040..4352ce817f0488face752bd40d630255fe6ca186 100644 (file)
--- a/bundle.c
+++ b/bundle.c
@@ -333,6 +333,7 @@ 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);
        }
                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");
 
        if (finish_command(&rls))
                return error ("pack-objects died");
 

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;
                }
        }
                                break;
                }
        }

+       close(proc.in);

        return hook_status(finish_command(&proc), hook_name);
 }
 
        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);
                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();
                status = finish_command(&ip);
                if (!status) {
                        reprepare_packed_git();

diff --git a/run-command.c b/run-command.c
index 476d00c2182e3af82a0cfe495c61c9df1eb44d26..2919330366bbd52aca7fb860e4f82e7216652925 100644 (file)
--- a/run-command.c
+++ b/run-command.c
@@ -25,7 +25,6 @@ int start_command(struct child_process *cmd)
                if (pipe(fdin) < 0)
                        return -ERR_RUN_COMMAND_PIPE;
                cmd->in = fdin[1];
                if (pipe(fdin) < 0)
                        return -ERR_RUN_COMMAND_PIPE;
                cmd->in = fdin[1];

-               cmd->close_in = 1;

        }
 
        need_out = !cmd->no_stdout
        }
 
        need_out = !cmd->no_stdout


@@ -38,7 +37,6 @@ int start_command(struct child_process *cmd)
                        return -ERR_RUN_COMMAND_PIPE;
                }
                cmd->out = fdout[0];
                        return -ERR_RUN_COMMAND_PIPE;
                }
                cmd->out = fdout[0];

-               cmd->close_out = 1;

        }
 
        need_err = !cmd->no_stderr && cmd->err < 0;
        }
 
        need_err = !cmd->no_stderr && cmd->err < 0;


@@ -157,10 +155,6 @@ static int wait_or_whine(pid_t pid)
 
 int finish_command(struct child_process *cmd)
 {
 
 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);
 }
 
        return wait_or_whine(cmd->pid);
 }
 

diff --git a/run-command.h b/run-command.h
index 1fc781d7668468f9e74bd430b7569dc040440ba8..e9c84d03639a59178e16361ee70df95835b9beb3 100644 (file)
--- a/run-command.h
+++ b/run-command.h
@@ -19,8 +19,6 @@ struct child_process {
        int err;
        const char *dir;
        const char *const *env;
        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;
        unsigned no_stdin:1;
        unsigned no_stdout:1;
        unsigned no_stderr:1;