summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJunio C Hamano <gitster@pobox.com>2008-02-27 19:55:32 (GMT)
committerJunio C Hamano <gitster@pobox.com>2008-02-27 19:55:32 (GMT)
commitf79ff5c975499089b03904a6619923683262bf22 (patch)
tree06d71decd7e4198f37687b5f604de9149904ce8c
parent860cc3a4f9c3ac21fcd37bbe30f40ab073f01299 (diff)
parentc20181e3a3e39f2c8874567c219e9edddb84393a (diff)
downloadgit-f79ff5c975499089b03904a6619923683262bf22.zip
git-f79ff5c975499089b03904a6619923683262bf22.tar.gz
git-f79ff5c975499089b03904a6619923683262bf22.tar.bz2
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
-rw-r--r--builtin-fetch-pack.c4
-rw-r--r--builtin-send-pack.c15
-rw-r--r--builtin-tag.c3
-rw-r--r--builtin-verify-tag.c2
-rw-r--r--bundle.c6
-rw-r--r--receive-pack.c2
-rw-r--r--run-command.c28
-rw-r--r--run-command.h20
8 files changed, 58 insertions, 22 deletions
diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index f401352..5ea48ca 100644
--- 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 8afb1d0..b0cfae8 100644
--- 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 716b4ff..28c36fd 100644
--- 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 cc4c55d..f3ef11f 100644
--- 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 bd12ec8..0ba5df1 100644
--- 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 3267495..a971433 100644
--- 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 476d00c..743757c 100644
--- 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 1fc781d..debe307 100644
--- 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;