[Libguestfs] [PATCH] lib: command: switch from select() to poll()

[Pino Toscano]

select() has a maximum value for the FDs it can monitor, and since
the libguestfs library can be used in other applications, this limit
may be hit by users in case lots of FDs are opened.

As solution, switch to poll(): it has a slightly better interface to
check what changed and for which FD, and it does not have a limit in the
value of the FDs monitored.

poll() is supported on the platforms we support, so there is no need to
use the gnulib module for it.
---
 lib/command.c | 54 ++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 36 insertions(+), 18 deletions(-)

diff --git a/lib/command.c b/lib/command.c
index f2161de9a..13b084934 100644
--- a/lib/command.c
+++ b/lib/command.c
@@ -89,9 +89,8 @@
 #include <signal.h>
 #include <errno.h>
 #include <assert.h>
-#include <sys/types.h>
 #include <sys/stat.h>
-#include <sys/select.h>
+#include <poll.h>
 
 #ifdef HAVE_SYS_TIME_H
 #include <sys/time.h>
@@ -650,37 +649,41 @@ run_child (struct command *cmd)
 static int
 loop (struct command *cmd)
 {
-  fd_set rset, rset2;
-  int maxfd = -1, r;
+  struct pollfd fds[2];
+  int r;
   size_t nr_fds = 0;
   CLEANUP_FREE char *buf = safe_malloc (cmd->g, BUFSIZ);
   ssize_t n;
 
-  FD_ZERO (&rset);
+  memset (&fds, 0, sizeof fds);
+
+  fds[0].fd = cmd->errorfd;
+  fds[1].fd = cmd->outfd;
 
   if (cmd->errorfd >= 0) {
-    FD_SET (cmd->errorfd, &rset);
-    maxfd = MAX (cmd->errorfd, maxfd);
+    fds[0].events = POLLIN;
     nr_fds++;
   }
 
   if (cmd->outfd >= 0) {
-    FD_SET (cmd->outfd, &rset);
-    maxfd = MAX (cmd->outfd, maxfd);
+    fds[1].events = POLLIN;
     nr_fds++;
   }
 
   while (nr_fds > 0) {
-    rset2 = rset;
-    r = select (maxfd+1, &rset2, NULL, NULL, NULL);
+    r = poll (fds, 2, -1);
     if (r == -1) {
       if (errno == EINTR || errno == EAGAIN)
         continue;
-      perrorf (cmd->g, "select");
+      perrorf (cmd->g, "poll");
+      return -1;
+    }
+    if (fds[0].revents & POLLERR || fds[1].revents & POLLERR) {
+      perrorf (cmd->g, "poll");
       return -1;
     }
 
-    if (cmd->errorfd >= 0 && FD_ISSET (cmd->errorfd, &rset2)) {
+    if (fds[0].revents & POLLIN) {
       /* Read output and send it to the log. */
       n = read (cmd->errorfd, buf, BUFSIZ);
       if (n > 0)
@@ -689,20 +692,26 @@ loop (struct command *cmd)
       else if (n == 0) {
         if (close (cmd->errorfd) == -1)
           perrorf (cmd->g, "close: errorfd");
-        FD_CLR (cmd->errorfd, &rset);
+        fds[0].fd = -1;
         cmd->errorfd = -1;
         nr_fds--;
       }
       else if (n == -1) {
         perrorf (cmd->g, "read: errorfd");
         close (cmd->errorfd);
-        FD_CLR (cmd->errorfd, &rset);
+        fds[0].fd = -1;
         cmd->errorfd = -1;
         nr_fds--;
       }
     }
+    if (fds[0].revents & POLLHUP) {
+      close (cmd->errorfd);
+      fds[0].fd = -1;
+      cmd->errorfd = -1;
+      nr_fds--;
+    }
 
-    if (cmd->outfd >= 0 && FD_ISSET (cmd->outfd, &rset2)) {
+    if (fds[1].revents & POLLIN) {
       /* Read the output, buffer it up to the end of the line, then
        * pass it to the callback.
        */
@@ -716,18 +725,27 @@ loop (struct command *cmd)
           cmd->outbuf.close_data (cmd);
         if (close (cmd->outfd) == -1)
           perrorf (cmd->g, "close: outfd");
-        FD_CLR (cmd->outfd, &rset);
+        fds[1].fd = -1;
         cmd->outfd = -1;
         nr_fds--;
       }
       else if (n == -1) {
         perrorf (cmd->g, "read: outfd");
         close (cmd->outfd);
-        FD_CLR (cmd->outfd, &rset);
+        fds[1].fd = -1;
         cmd->outfd = -1;
         nr_fds--;
       }
     }
+    if (fds[1].revents & POLLHUP) {
+      if (cmd->outbuf.close_data)
+        cmd->outbuf.close_data (cmd);
+      if (close (cmd->outfd) == -1)
+        perrorf (cmd->g, "close: outfd");
+      fds[1].fd = -1;
+      cmd->outfd = -1;
+      nr_fds--;
+    }
   }
 
   return 0;
-- 
2.24.1