start_command(), if .in/.out > 0, closes file descriptors, not the callers
Callers of start_command() can set the members .in and .out of struct child_process to a value > 0 to specify that this descriptor is used as the stdin or stdout of the child process. Previously, if start_command() was successful, this descriptor was closed upon return. Here we now make sure that the descriptor is also closed in case of failures. All callers are updated not to close the file descriptor themselves after start_command() was called. Note that earlier run_gpg_verify() of git-verify-tag set .out = 1, which worked because start_command() treated this as a special case, but now this is incorrect because it closes the descriptor. The intent here is to inherit stdout to the child, which is achieved by .out = 0. Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Johannes Sixt committed
Feb 21, 2008 at 23:42 UTC
c20181e3a3e39f2c8874567c219e9edddb84393a
5 files changed
+49
-11
builtin-send-pack.c
+8
-6
@@ -404,12 +404,15 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest
404
if (!remote_tail)
405
remote_tail = &remote_refs;
406
if (match_refs(local_refs, remote_refs, &remote_tail,
407
- nr_refspec, refspec, flags))
407
+ nr_refspec, refspec, flags)) {
408
+ close(out);
409
return -1;
410
+ }
411
412
if (!remote_refs) {
413
fprintf(stderr, "No refs in common and none specified; doing nothing.\n"
414
"Perhaps you should specify a branch such as 'master'.\n");
415
+ close(out);
416
return 0;
417
}
418
@@ -496,12 +499,11 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest
499
500
packet_flush(out);
501
if (new_refs && !args.dry_run) {
499
- if (pack_objects(out, remote_refs) < 0) {
500
- close(out);
502
+ if (pack_objects(out, remote_refs) < 0)
503
return -1;
502
- }
504
}
504
- close(out);
505
+ else
506
+ close(out);
507
508
if (expect_status_report)
509
ret = receive_status(in, remote_refs);
@@ -649,7 +651,7 @@ int send_pack(struct send_pack_args *my_args,
651
conn = git_connect(fd, dest, args.receivepack, args.verbose ? CONNECT_VERBOSE : 0);
652
ret = do_send_pack(fd[0], fd[1], remote, dest, nr_heads, heads);
653
close(fd[0]);
652
- close(fd[1]);
654
+ /* do_send_pack always closes fd[1] */
655
ret |= finish_connect(conn);
656
return !!ret;
657
}
builtin-verify-tag.c
-1
@@ -45,7 +45,6 @@ static int run_gpg_verify(const char *buf, unsigned long size, int verbose)
45
memset(&gpg, 0, sizeof(gpg));
46
gpg.argv = args_gpg;
47
gpg.in = -1;
48
- gpg.out = 1;
48
args_gpg[2] = path;
49
if (start_command(&gpg))
50
return error("could not run gpg.");
bundle.c
+3
-2
@@ -336,8 +336,9 @@ int create_bundle(struct bundle_header *header, const char *path,
336
close(rls.in);
337
if (finish_command(&rls))
338
return error ("pack-objects died");
339
-
340
- return bundle_to_stdout ? close(bundle_fd) : commit_lock_file(&lock);
339
+ if (!bundle_to_stdout)
340
+ commit_lock_file(&lock);
341
+ return 0;
342
}
343
344
int unbundle(struct bundle_header *header, int bundle_fd)
run-command.c
+20
-2
@@ -20,10 +20,18 @@ int start_command(struct child_process *cmd)
20
int need_in, need_out, need_err;
21
int fdin[2], fdout[2], fderr[2];
22
23
+ /*
24
+ * In case of errors we must keep the promise to close FDs
25
+ * that have been passed in via ->in and ->out.
26
+ */
27
+
28
need_in = !cmd->no_stdin && cmd->in < 0;
29
if (need_in) {
25
- if (pipe(fdin) < 0)
30
+ if (pipe(fdin) < 0) {
31
+ if (cmd->out > 0)
32
+ close(cmd->out);
33
return -ERR_RUN_COMMAND_PIPE;
34
+ }
35
cmd->in = fdin[1];
36
}
37
@@ -34,6 +42,8 @@ int start_command(struct child_process *cmd)
42
if (pipe(fdout) < 0) {
43
if (need_in)
44
close_pair(fdin);
45
+ else if (cmd->in)
46
+ close(cmd->in);
47
return -ERR_RUN_COMMAND_PIPE;
48
}
49
cmd->out = fdout[0];
@@ -44,8 +54,12 @@ int start_command(struct child_process *cmd)
54
if (pipe(fderr) < 0) {
55
if (need_in)
56
close_pair(fdin);
57
+ else if (cmd->in)
58
+ close(cmd->in);
59
if (need_out)
60
close_pair(fdout);
61
+ else if (cmd->out)
62
+ close(cmd->out);
63
return -ERR_RUN_COMMAND_PIPE;
64
}
65
cmd->err = fderr[0];
@@ -55,8 +69,12 @@ int start_command(struct child_process *cmd)
69
if (cmd->pid < 0) {
70
if (need_in)
71
close_pair(fdin);
72
+ else if (cmd->in)
73
+ close(cmd->in);
74
if (need_out)
75
close_pair(fdout);
76
+ else if (cmd->out)
77
+ close(cmd->out);
78
if (need_err)
79
close_pair(fderr);
80
return -ERR_RUN_COMMAND_FORK;
@@ -118,7 +136,7 @@ int start_command(struct child_process *cmd)
136
137
if (need_out)
138
close(fdout[1]);
121
- else if (cmd->out > 1)
139
+ else if (cmd->out)
140
close(cmd->out);
141
142
if (need_err)
run-command.h
+18
@@ -14,6 +14,24 @@ enum {
14
struct child_process {
15
const char **argv;
16
pid_t pid;
17
+ /*
18
+ * Using .in, .out, .err:
19
+ * - Specify 0 for no redirections (child inherits stdin, stdout,
20
+ * stderr from parent).
21
+ * - Specify -1 to have a pipe allocated as follows:
22
+ * .in: returns the writable pipe end; parent writes to it,
23
+ * the readable pipe end becomes child's stdin
24
+ * .out, .err: returns the readable pipe end; parent reads from
25
+ * it, the writable pipe end becomes child's stdout/stderr
26
+ * The caller of start_command() must close the returned FDs
27
+ * after it has completed reading from/writing to it!
28
+ * - Specify > 0 to set a channel to a particular FD as follows:
29
+ * .in: a readable FD, becomes child's stdin
30
+ * .out: a writable FD, becomes child's stdout/stderr
31
+ * .err > 0 not supported
32
+ * The specified FD is closed by start_command(), even in case
33
+ * of errors!
34
+ */
35
int in;
36
int out;
37
int err;