summaryrefslogtreecommitdiff
path: root/builtin-upload-archive.c
diff options
context:
space:
mode:
authorJunio C Hamano <junkio@cox.net>2006-09-12 00:26:57 -0700
committerJunio C Hamano <junkio@cox.net>2006-09-12 22:39:45 -0700
commitd3788e19e20cd14aeac99d1f294d9a368437284f (patch)
tree2c5f1aa4dcd80d48e027287d8c19c0b0d211e3b2 /builtin-upload-archive.c
parent23d6d112c004d4242f9dbd8161f79ccdeb47bde8 (diff)
downloadgit-d3788e19e20cd14aeac99d1f294d9a368437284f.tar.gz
upload-archive: monitor child communication more carefully.
Franck noticed that the code around polling and relaying messages from the child process was quite bogus. Here is an attempt to clean it up a bit, based on his patch: - When POLLHUP is set, it goes ahead and reads the file descriptor. Worse yet, it does not check the return value of read() for errors when it does. - When we processed one POLLIN, we should just go back and see if any more data is available. We can check if the child is still there when poll gave control back at us but without any actual input. [jc: with simplification suggested by Franck. ] Signed-off-by: Junio C Hamano <junkio@cox.net>
Diffstat (limited to 'builtin-upload-archive.c')
-rw-r--r--builtin-upload-archive.c60
1 files changed, 38 insertions, 22 deletions
diff --git a/builtin-upload-archive.c b/builtin-upload-archive.c
index 42cb9f8876..115a12dc13 100644
--- a/builtin-upload-archive.c
+++ b/builtin-upload-archive.c
@@ -16,6 +16,9 @@ static const char upload_archive_usage[] =
static const char deadchild[] =
"git-upload-archive: archiver died with error";
+static const char lostchild[] =
+"git-upload-archive: archiver process was lost";
+
static int run_upload_archive(int argc, const char **argv, const char *prefix)
{
@@ -73,6 +76,31 @@ static int run_upload_archive(int argc, const char **argv, const char *prefix)
return ar.write_archive(&ar.args);
}
+static void error_clnt(const char *fmt, ...)
+{
+ char buf[1024];
+ va_list params;
+ int len;
+
+ va_start(params, fmt);
+ len = vsprintf(buf, fmt, params);
+ va_end(params);
+ send_sideband(1, 3, buf, len, LARGE_PACKET_MAX);
+ die("sent error to the client: %s", buf);
+}
+
+static void process_input(int child_fd, int band)
+{
+ char buf[16384];
+ ssize_t sz = read(child_fd, buf, sizeof(buf));
+ if (sz < 0) {
+ if (errno != EINTR)
+ error_clnt("read error: %s\n", strerror(errno));
+ return;
+ }
+ send_sideband(1, band, buf, sz, LARGE_PACKET_MAX);
+}
+
int cmd_upload_archive(int argc, const char **argv, const char *prefix)
{
pid_t writer;
@@ -112,9 +140,6 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
while (1) {
struct pollfd pfd[2];
- char buf[16384];
- ssize_t sz;
- pid_t pid;
int status;
pfd[0].fd = fd1[0];
@@ -129,28 +154,19 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
}
continue;
}
- if (pfd[0].revents & (POLLIN|POLLHUP)) {
+ if (pfd[0].revents & POLLIN)
/* Data stream ready */
- sz = read(pfd[0].fd, buf, sizeof(buf));
- send_sideband(1, 1, buf, sz, LARGE_PACKET_MAX);
- }
- if (pfd[1].revents & (POLLIN|POLLHUP)) {
+ process_input(pfd[0].fd, 1);
+ if (pfd[1].revents & POLLIN)
/* Status stream ready */
- sz = read(pfd[1].fd, buf, sizeof(buf));
- send_sideband(1, 2, buf, sz, LARGE_PACKET_MAX);
- }
-
- if (((pfd[0].revents | pfd[1].revents) & POLLHUP) == 0)
- continue;
- /* did it die? */
- pid = waitpid(writer, &status, WNOHANG);
- if (!pid) {
- fprintf(stderr, "Hmph, HUP?\n");
+ process_input(pfd[1].fd, 2);
+ if ((pfd[0].revents | pfd[1].revents) == POLLIN)
continue;
- }
- if (!WIFEXITED(status) || WEXITSTATUS(status) > 0)
- send_sideband(1, 3, deadchild, strlen(deadchild),
- LARGE_PACKET_MAX);
+
+ if (waitpid(writer, &status, 0) < 0)
+ error_clnt("%s", lostchild);
+ else if (!WIFEXITED(status) || WEXITSTATUS(status) > 0)
+ error_clnt("%s", deadchild);
packet_flush(1);
break;
}