commit dcbecbb33c28408b50a477680cffd8c9629753fe
author Avery Pennarun <apenwarr@gmail.com> Wed Jul 13 01:27:29 2016 -0400
committer Avery Pennarun <apenwarr@gmail.com> Wed Jul 13 01:39:09 2016 -0400
tree ece41e20b54b46227a849ad007c4b85dddfaa14a
parent 48c86b2fd0f4ed5c18bc2a3ffceb9acefe60aaab

gstatic: don't abort just because one host has a connect() failure.

If IPv4 or IPv6 was down, we might abort early and return neither result,
which was wrong.

While we're here, fix up various other error handling mistakes.  In
particular, don't perror() when errno==0, or you might get the dreaded
"Error: success" message.  Add an overall alarm() to handle any blocking
syscalls that never return.  Don't use exceptfd in select(), since readfd
covers all the interesting types of exceptions, and removing it removes the
need to check readfd, which we hadn't been doing.

Change-Id: I75e839b0608d51bfee5e2a31c9a1ce2e4ece1e23

cmds/gstatic.c

diff --git a/cmds/gstatic.c b/cmds/gstatic.c
index 15b23fc..518871f 100644
--- a/cmds/gstatic.c
+++ b/cmds/gstatic.c
@@ -85,11 +85,13 @@
   total = 0;
   while (total < count) {
     rc = write(fd, buf + total, count - total);
-    if (rc < 0)
-      perror_die("write");
-    else if (rc == 0)
+    if (rc < 0) {
+      perror("write");
+      return -1;
+    } else if (rc == 0) {
+      fprintf(stderr, "write: EOF\n");
       return total;
-    else
+    } else
       total += rc;
   }
 
@@ -125,8 +127,10 @@
   socket_set_blocking(fd, false);
 
   rc = connect(fd, addr->ai_addr, addr->ai_addrlen);
-  if (rc < 0 && errno != EINPROGRESS)
-    perror_die("connect");
+  if (rc < 0 && errno != EINPROGRESS) {
+    perror("connect");
+    return -1;
+  }
 
   if (rc != 0) {
     FD_ZERO(&writeset);
@@ -136,7 +140,8 @@
 
     rc = select(fd + 1, NULL, &writeset, NULL, &timeout);
     if (rc < 0) {
-      perror_die("select");
+      perror("connect-select");
+      return -1;
     } else if (rc == 0) {
       /* timeout */
       return -1;
@@ -156,27 +161,24 @@
 
 ssize_t read_timeout(int fd, void *buf, size_t count, int timeout_ms)
 {
-  fd_set readset, exceptset;
+  fd_set readset;
   struct timeval timeout;
   int rc;
 
   FD_ZERO(&readset);
   FD_SET(fd, &readset);
-  exceptset = readset;
   timeout.tv_sec = timeout_ms / 1000;
   timeout.tv_usec = (timeout_ms % 1000) * 1000;
 
-  rc = select(fd + 1, &readset, NULL, &exceptset, &timeout);
+  rc = select(fd + 1, &readset, NULL, NULL, &timeout);
   if (rc < 0) {
-    perror_die("select");
+    perror("select");
+    return -1;
   } else if (rc == 0) {
-    /* timeout */
-    return 0;
+    fprintf(stderr, "select: timed out\n");
+    return -1;
   }
 
-  if (FD_ISSET(fd, &exceptset))
-    return 0;
-
   return read(fd, buf, count);
 }
 
@@ -198,7 +200,7 @@
 
   rc = xwrite(fd, HTTP_REQUEST, sizeof(HTTP_REQUEST));
   if (rc < (ssize_t)sizeof(HTTP_REQUEST))
-    perror_die("write");
+    goto err;
 
   rc = read_timeout(fd, http_response, sizeof(http_response), TIMEOUT_MS);
   if (rc < 0)
@@ -223,6 +225,9 @@
   int bad;
   int rc;
 
+  // In case we get stuck in one of the blocking syscalls (write, read, etc)
+  alarm(60);
+
   memset(&hints, 0, sizeof(hints));
   hints.ai_family = AF_UNSPEC;
   hints.ai_socktype = SOCK_STREAM;
@@ -241,8 +246,11 @@
     int fd;
 
     fd = socket(res->ai_family, res->ai_socktype, res->ai_protocol);
-    if (fd < 0)
-      perror_die("socket");
+    if (fd < 0) {
+      perror("socket");
+      bad = 1;
+      continue;
+    }
 
     rc = do_http_request(fd, res);
     if (rc != 0)