[PATCH] maint: refactor tail.c to use iopoll

[Pádraig Brady]

* src/tail.c (check_output_alive): Reuse iopoll()
rather than directly calling poll() or select().
* src/iopoll.c (iopoll): Refactor to support non blocking operation,
or ignoring descriptors by passing a negative value.
* src/iopoll.h (iopoll): Adjust to support a BLOCK parameter.
* src/tee.c (tee_files): Adjust iopoll() call to explicitly block.
* src/local.mk: Have tail depend on iopoll.c.
---
 src/iopoll.c | 50 +++++++++++++++++++++++++++++---------------------
 src/iopoll.h |  2 +-
 src/local.mk |  2 ++
 src/tail.c   | 40 +++-------------------------------------
 src/tee.c    |  3 ++-
 5 files changed, 37 insertions(+), 60 deletions(-)

diff --git a/src/iopoll.c b/src/iopoll.c
index ceb1b43ad..db3717b97 100644
--- a/src/iopoll.c
+++ b/src/iopoll.c
@@ -26,6 +26,12 @@
 #if defined _AIX || defined __sun || defined __APPLE__ || \
     defined __linux__ || defined __ANDROID__
 # define IOPOLL_USES_POLL 1
+  /* Check we've not enabled gnulib's poll module
+     as that will emulate poll() in a way not
+     currently compatible with our usage.  */
+# if defined HAVE_POLL
+#  error "gnulib's poll() replacement is currently incompatible"
+# endif
 #endif
 
 #if IOPOLL_USES_POLL
@@ -39,22 +45,24 @@
 #include "isapipe.h"
 
 
-/* Wait for fdin to become ready for reading or fdout to become a broken pipe.
-   Return 0 if fdin can be read() without blocking, or IOPOLL_BROKEN_OUTPUT if
-   fdout becomes a broken pipe, otherwise IOPOLL_ERROR if there is a poll()
+/* Wait for FDIN to become ready for reading or FDOUT to become a broken pipe.
+   If either of those are -1, then they're not checked.  Set BLOCK to true
+   to wait for an event, otherwise return the status immediately.
+   Return 0 if FDIN can be read() without blocking, or IOPOLL_BROKEN_OUTPUT if
+   FDOUT becomes a broken pipe, otherwise IOPOLL_ERROR if there is a poll()
    or select() error.  */
 
-#if IOPOLL_USES_POLL
-
 extern int
-iopoll (int fdin, int fdout)
+iopoll (int fdin, int fdout, bool block)
 {
+#if IOPOLL_USES_POLL
+
   struct pollfd pfds[2] = {  /* POLLRDBAND needed for illumos, macOS.  */
     { .fd = fdin,  .events = POLLIN | POLLRDBAND, .revents = 0 },
     { .fd = fdout, .events = POLLRDBAND, .revents = 0 },
   };
 
-  while (poll (pfds, 2, -1) > 0 || errno == EINTR)
+  while (poll (pfds, 2, block ? -1 : 0) > 0 || errno == EINTR)
     {
       if (errno == EINTR)
         continue;
@@ -63,14 +71,9 @@ iopoll (int fdin, int fdout)
       if (pfds[1].revents)            /* POLLERR, POLLHUP (or POLLNVAL) */
         return IOPOLL_BROKEN_OUTPUT;  /* output error or broken pipe    */
     }
-  return IOPOLL_ERROR;  /* poll error */
-}
 
 #else  /* fall back to select()-based implementation */
 
-extern int
-iopoll (int fdin, int fdout)
-{
   int nfds = (fdin > fdout ? fdin : fdout) + 1;
   int ret = 0;
 
@@ -80,21 +83,26 @@ iopoll (int fdin, int fdout)
     {
       fd_set rfds;
       FD_ZERO (&rfds);
-      FD_SET (fdin, &rfds);
-      FD_SET (fdout, &rfds);
-      ret = select (nfds, &rfds, NULL, NULL, NULL);
+      if (0 <= fdin)
+        FD_SET (fdin, &rfds);
+      if (0 <= fdout)
+        FD_SET (fdout, &rfds);
+
+      struct timeval delay = { .tv_sec = 0, .tv_usec = 0 };
+      ret = select (nfds, &rfds, NULL, NULL, block ? NULL : &delay);
 
       if (ret < 0)
         continue;
-      if (FD_ISSET (fdin, &rfds))  /* input available or EOF; should now */
-        return 0;                  /* be able to read() without blocking */
-      if (FD_ISSET (fdout, &rfds))     /* POLLERR, POLLHUP (or POLLIN)   */
-        return IOPOLL_BROKEN_OUTPUT;   /* output error or broken pipe    */
+      if (0 <= fdin && FD_ISSET (fdin, &rfds))   /* input available or EOF; */
+        return 0;          /* should now be able to read() without blocking */
+      if (0 <= fdout && FD_ISSET (fdout, &rfds)) /* equiv to POLLERR        */
+        return IOPOLL_BROKEN_OUTPUT;      /* output error or broken pipe    */
     }
-  return IOPOLL_ERROR;  /* select error */
-}
 
 #endif
+  return IOPOLL_ERROR;
+}
+
 
 
 /* Return true if fdin is relevant for iopoll().
diff --git a/src/iopoll.h b/src/iopoll.h
index 85935e960..00fc99836 100644
--- a/src/iopoll.h
+++ b/src/iopoll.h
@@ -1,6 +1,6 @@
 #define IOPOLL_BROKEN_OUTPUT -2
 #define IOPOLL_ERROR         -3
 
-int iopoll (int fdin, int fdout);
+int iopoll (int fdin, int fdout, bool block);
 bool iopoll_input_ok (int fdin);
 bool iopoll_output_ok (int fdout);
diff --git a/src/local.mk b/src/local.mk
index 8269a2f68..13eeea8e0 100644
--- a/src/local.mk
+++ b/src/local.mk
@@ -396,7 +396,9 @@ src_arch_SOURCES = src/uname.c src/uname-arch.c
 src_cut_SOURCES = src/cut.c src/set-fields.c
 src_numfmt_SOURCES = src/numfmt.c src/set-fields.c
 
+src_tail_SOURCES = src/tail.c src/iopoll.c
 src_tee_SOURCES = src/tee.c src/iopoll.c
+
 src_sum_SOURCES = src/sum.c src/sum.h src/digest.c
 src_sum_CPPFLAGS = -DHASH_ALGO_SUM=1 $(AM_CPPFLAGS)
 
diff --git a/src/tail.c b/src/tail.c
index 78022fc4b..92534a592 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -28,7 +28,6 @@
 #include <stdio.h>
 #include <assert.h>
 #include <getopt.h>
-#include <sys/select.h>
 #include <sys/types.h>
 #include <signal.h>
 
@@ -38,6 +37,7 @@
 #include "die.h"
 #include "error.h"
 #include "fcntl--.h"
+#include "iopoll.h"
 #include "isapipe.h"
 #include "posixver.h"
 #include "quote.h"
@@ -52,11 +52,8 @@
 
 #if HAVE_INOTIFY
 # include "hash.h"
-# include <sys/inotify.h>
-#endif
-
-#if defined _AIX || defined __sun || defined __APPLE__ || HAVE_INOTIFY
 # include <poll.h>
+# include <sys/inotify.h>
 #endif
 
 /* Linux can optimize the handling of local files.  */
@@ -352,39 +349,8 @@ check_output_alive (void)
   if (! monitor_output)
     return;
 
-  /* Check we've not enabled gnulib's poll module
-     as that will emulate poll() in a way not
-     currently compatible with tail's usage.  */
-#if defined HAVE_POLL
-# error "gnulib's poll() replacement is currently incompatible"
-#endif
-
-  /* poll(2) is needed on AIX (where 'select' gives a readable
-     event immediately) and Solaris (where 'select' never gave
-     a readable event).  Also use poll(2) on systems we know work
-     and/or are already using poll (inotify).  */
-#if defined _AIX || defined __sun || defined __APPLE__ || HAVE_INOTIFY
-  struct pollfd pfd;
-  pfd.fd = STDOUT_FILENO;
-  pfd.events = pfd.revents = 0;
-  pfd.events |= POLLRDBAND; /* Needed for illumos, macOS.  */
-
-  if (poll (&pfd, 1, 0) > 0 && (pfd.revents & (POLLERR | POLLHUP)))
+  if (iopoll (-1, STDOUT_FILENO, false) == IOPOLL_BROKEN_OUTPUT)
     die_pipe ();
-#else
-  struct timeval delay;
-  delay.tv_sec = delay.tv_usec = 0;
-
-  fd_set rfd;
-  FD_ZERO (&rfd);
-  FD_SET (STDOUT_FILENO, &rfd);
-
-  /* readable event on STDOUT is equivalent to POLLERR,
-     and implies an error condition on output like broken pipe.  */
-  if (select (STDOUT_FILENO + 1, &rfd, NULL, NULL, &delay) == 1)
-    die_pipe ();
-#endif
-
 }
 
 static bool
diff --git a/src/tee.c b/src/tee.c
index e328e6f04..dcf773e16 100644
--- a/src/tee.c
+++ b/src/tee.c
@@ -290,7 +290,8 @@ tee_files (int nfiles, char **files)
       if (pipe_check && out_pollable[first_out])
         {
           /* Monitor for input, or errors on first valid output.  */
-          int err = iopoll (STDIN_FILENO, fileno (descriptors[first_out]));
+          int err = iopoll (STDIN_FILENO, fileno (descriptors[first_out]),
+                            true);
 
           /* Close the output if it became a broken pipe.  */
           if (err == IOPOLL_BROKEN_OUTPUT)
-- 
2.26.2