At least for me, this improves the readability of xread and
xwrite; hopefully allowing missing "continue" statements to
be spotted more easily.

Signed-off-by: Eric Wong <e...@80x24.org>
Signed-off-by: Jeff King <p...@peff.net>
---
  Jeff King <p...@peff.net> wrote:
  > On Sun, Jul 10, 2016 at 03:47:36AM +0000, Eric Wong wrote:
  > > Yes, I'm fine with either, but I'm slightly thrown off by
  > > a function relying on errno being set by the caller, even if it
  > > is errno.  So maybe localizing it is better (see below)
  > 
  > Yeah, I had a similar reservation, but didn't want to clutter the
  > interface. However, just passing errno isn't too bad (as you showed
  > below), and is much less magical.
  > 
  > Do you want to squash that and re-send the whole patch to make Junio's
  > life easier?

  Done, also updated the subject to match the new function name.

  I wasn't attached to the "io_wait" name, either, as it could
  be confused with "I/O wait" commonly associated with disk I/O
  and not often with socket/pipe I/O that poll gets used for.

 wrapper.c | 48 ++++++++++++++++++++----------------------------
 1 file changed, 20 insertions(+), 28 deletions(-)

diff --git a/wrapper.c b/wrapper.c
index 0b920f1..78f6431 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -227,6 +227,24 @@ int xopen(const char *path, int oflag, ...)
        }
 }
 
+static int handle_nonblock(int fd, short poll_events, int err)
+{
+       struct pollfd pfd;
+
+       if (err != EAGAIN && err != EWOULDBLOCK)
+               return 0;
+
+       pfd.fd = fd;
+       pfd.events = poll_events;
+
+       /*
+        * no need to check for errors, here;
+        * a subsequent read/write will detect unrecoverable errors
+        */
+       poll(&pfd, 1, -1);
+       return 1;
+}
+
 /*
  * xread() is the same a read(), but it automatically restarts read()
  * operations with a recoverable error (EAGAIN and EINTR). xread()
@@ -242,21 +260,8 @@ ssize_t xread(int fd, void *buf, size_t len)
                if (nr < 0) {
                        if (errno == EINTR)
                                continue;
-                       if (errno == EAGAIN || errno == EWOULDBLOCK) {
-                               struct pollfd pfd;
-                               pfd.events = POLLIN;
-                               pfd.fd = fd;
-                               /*
-                                * it is OK if this poll() failed; we
-                                * want to leave this infinite loop
-                                * only when read() returns with
-                                * success, or an expected failure,
-                                * which would be checked by the next
-                                * call to read(2).
-                                */
-                               poll(&pfd, 1, -1);
+                       if (handle_nonblock(fd, POLLIN, errno))
                                continue;
-                       }
                }
                return nr;
        }
@@ -277,21 +282,8 @@ ssize_t xwrite(int fd, const void *buf, size_t len)
                if (nr < 0) {
                        if (errno == EINTR)
                                continue;
-                       if (errno == EAGAIN || errno == EWOULDBLOCK) {
-                               struct pollfd pfd;
-                               pfd.events = POLLOUT;
-                               pfd.fd = fd;
-                               /*
-                                * it is OK if this poll() failed; we
-                                * want to leave this infinite loop
-                                * only when write() returns with
-                                * success, or an expected failure,
-                                * which would be checked by the next
-                                * call to write(2).
-                                */
-                               poll(&pfd, 1, -1);
+                       if (handle_nonblock(fd, POLLOUT, errno))
                                continue;
-                       }
                }
 
                return nr;
-- 
EW
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to