Break up io_callback() into read_callback() and write_callback()

to make it clear that we can't get an event with both read and write
set.

diff --git a/src/exec_pty.c b/src/exec_pty.c
index 602eac303d41d95e70f10e00733c11aedbe43749..369f2b5435d9143a9d67a93953187ffb31bd89bd 100644 (file)
--- a/src/exec_pty.c
+++ b/src/exec_pty.c
@@ -537,130 +537,137 @@ terminate_command(pid_t pid, bool use_pgrp)
 }
 
 /*
- * Read/write an iobuf that is ready.
+ * Read an iobuf that is ready.
  */
 static void
-io_callback(int fd, int what, void *v)
+read_callback(int fd, int what, void *v)
 {
     struct io_buffer *iob = v;
     struct sudo_event_base *evbase;
     int n;
-    debug_decl(io_callback, SUDO_DEBUG_EXEC);
+    debug_decl(read_callback, SUDO_DEBUG_EXEC);
 
-    if (ISSET(what, SUDO_EV_WRITE)) {
-       evbase = sudo_ev_get_base(iob->wevent);
-       do {
-           n = write(fd, iob->buf + iob->off, iob->len - iob->off);
-       } while (n == -1 && errno == EINTR);
-       if (n == -1) {
-           switch (errno) {
-           case EPIPE:
-           case ENXIO:
-           case EIO:
-           case EBADF:
-               /* other end of pipe closed or pty revoked */
-               sudo_debug_printf(SUDO_DEBUG_INFO,
-                   "unable to write %d bytes to fd %d",
-                   iob->len - iob->off, fd);
-               /* Close reader if there is one. */
-               if (iob->revent != NULL) {
-                   safe_close(sudo_ev_get_fd(iob->revent));
-                   ev_free_by_fd(evbase, sudo_ev_get_fd(iob->revent));
-               }
-               CLR(what, SUDO_EV_READ);
-               safe_close(fd);
-               ev_free_by_fd(evbase, fd);
-               break;
-           case EAGAIN:
-               /* not an error */
-               break;
-           default:
-#if 0 /* XXX -- how to set cstat? stash in iobufs instead? */
-               if (cstat != NULL) {
-                   cstat->type = CMD_ERRNO;
-                   cstat->val = errno;
-               }
-#endif /* XXX */
-               sudo_debug_printf(SUDO_DEBUG_ERROR,
-                   "error writing fd %d: %s", fd, strerror(errno));
-               sudo_ev_loopbreak(evbase);
+    evbase = sudo_ev_get_base(iob->revent);
+    do {
+       n = read(fd, iob->buf + iob->len, sizeof(iob->buf) - iob->len);
+    } while (n == -1 && errno == EINTR);
+    switch (n) {
+       case -1:
+           if (errno == EAGAIN)
                break;
+           /* treat read error as fatal and close the fd */
+           sudo_debug_printf(SUDO_DEBUG_ERROR,
+               "error reading fd %d: %s", fd, strerror(errno));
+           /* FALLTHROUGH */
+       case 0:
+           /* got EOF or pty has gone away */
+           if (n == 0) {
+               sudo_debug_printf(SUDO_DEBUG_INFO,
+                   "read EOF from fd %d", fd);
            }
-       } else {
-           sudo_debug_printf(SUDO_DEBUG_INFO,
-               "wrote %d bytes to fd %d", n, fd);
-           iob->off += n;
-           /* Reset buffer if fully consumed. */
-           if (iob->off == iob->len) {
+           safe_close(fd);
+           ev_free_by_fd(evbase, fd);
+           /* If writer already consumed the buffer, close it too. */
+           if (iob->wevent != NULL && iob->off == iob->len) {
+               safe_close(sudo_ev_get_fd(iob->wevent));
+               ev_free_by_fd(evbase, sudo_ev_get_fd(iob->wevent));
                iob->off = iob->len = 0;
-               /* Forward the EOF from reader to writer. */
-               if (iob->revent == NULL) {
-                   CLR(what, SUDO_EV_READ);
-                   safe_close(fd);
-                   ev_free_by_fd(evbase, fd);
-               }
            }
-           /* Re-enable writer if buffer is not empty. */
-           if (iob->len > iob->off) {
+           break;
+       default:
+           sudo_debug_printf(SUDO_DEBUG_INFO,
+               "read %d bytes from fd %d", n, fd);
+           if (!iob->action(iob->buf + iob->len, n, iob))
+               terminate_command(cmnd_pid, true);
+           iob->len += n;
+           /* Enable writer if not /dev/tty or we are foreground pgrp. */
+           if (iob->wevent != NULL &&
+               (foreground || !USERTTY_EVENT(iob->wevent))) {
                if (sudo_ev_add(evbase, iob->wevent, NULL, false) == -1)
                    sudo_fatal(U_("unable to add event to queue"));
            }
-           /* Enable reader if buffer is not full. */
-           if (iob->revent != NULL &&
-               (ttymode == TERM_RAW || !USERTTY_EVENT(iob->revent))) {
-               if (iob->len != sizeof(iob->buf)) {
-                   if (sudo_ev_add(evbase, iob->revent, NULL, false) == -1)
-                       sudo_fatal(U_("unable to add event to queue"));
-               }
+           /* Re-enable reader if buffer is not full. */
+           if (iob->len != sizeof(iob->buf)) {
+               if (sudo_ev_add(evbase, iob->revent, NULL, false) == -1)
+                   sudo_fatal(U_("unable to add event to queue"));
            }
-       }
+           break;
     }
-    if (ISSET(what, SUDO_EV_READ)) {
-       evbase = sudo_ev_get_base(iob->revent);
-       do {
-           n = read(fd, iob->buf + iob->len, sizeof(iob->buf) - iob->len);
-       } while (n == -1 && errno == EINTR);
-       switch (n) {
-           case -1:
-               if (errno == EAGAIN)
-                   break;
-               /* treat read error as fatal and close the fd */
-               sudo_debug_printf(SUDO_DEBUG_ERROR,
-                   "error reading fd %d: %s", fd, strerror(errno));
-               /* FALLTHROUGH */
-           case 0:
-               /* got EOF or pty has gone away */
-               if (n == 0) {
-                   sudo_debug_printf(SUDO_DEBUG_INFO,
-                       "read EOF from fd %d", fd);
-               }
+}
+
+/*
+ * Write an iobuf that is ready.
+ */
+static void
+write_callback(int fd, int what, void *v)
+{
+    struct io_buffer *iob = v;
+    struct sudo_event_base *evbase;
+    int n;
+    debug_decl(write_callback, SUDO_DEBUG_EXEC);
+
+    evbase = sudo_ev_get_base(iob->wevent);
+    do {
+       n = write(fd, iob->buf + iob->off, iob->len - iob->off);
+    } while (n == -1 && errno == EINTR);
+    if (n == -1) {
+       switch (errno) {
+       case EPIPE:
+       case ENXIO:
+       case EIO:
+       case EBADF:
+           /* other end of pipe closed or pty revoked */
+           sudo_debug_printf(SUDO_DEBUG_INFO,
+               "unable to write %d bytes to fd %d",
+               iob->len - iob->off, fd);
+           /* Close reader if there is one. */
+           if (iob->revent != NULL) {
+               safe_close(sudo_ev_get_fd(iob->revent));
+               ev_free_by_fd(evbase, sudo_ev_get_fd(iob->revent));
+           }
+           safe_close(fd);
+           ev_free_by_fd(evbase, fd);
+           break;
+       case EAGAIN:
+           /* not an error */
+           break;
+       default:
+#if 0 /* XXX -- how to set cstat? stash in iobufs instead? */
+           if (cstat != NULL) {
+               cstat->type = CMD_ERRNO;
+               cstat->val = errno;
+           }
+#endif /* XXX */
+           sudo_debug_printf(SUDO_DEBUG_ERROR,
+               "error writing fd %d: %s", fd, strerror(errno));
+           sudo_ev_loopbreak(evbase);
+           break;
+       }
+    } else {
+       sudo_debug_printf(SUDO_DEBUG_INFO,
+           "wrote %d bytes to fd %d", n, fd);
+       iob->off += n;
+       /* Reset buffer if fully consumed. */
+       if (iob->off == iob->len) {
+           iob->off = iob->len = 0;
+           /* Forward the EOF from reader to writer. */
+           if (iob->revent == NULL) {
                safe_close(fd);
                ev_free_by_fd(evbase, fd);
-               /* If writer already consumed the buffer, close it too. */
-               if (iob->wevent != NULL && iob->off == iob->len) {
-                   safe_close(sudo_ev_get_fd(iob->wevent));
-                   ev_free_by_fd(evbase, sudo_ev_get_fd(iob->wevent));
-                   iob->off = iob->len = 0;
-               }
-               break;
-           default:
-               sudo_debug_printf(SUDO_DEBUG_INFO,
-                   "read %d bytes from fd %d", n, fd);
-               if (!iob->action(iob->buf + iob->len, n, iob))
-                   terminate_command(cmnd_pid, true);
-               iob->len += n;
-               /* Enable writer if not /dev/tty or we are foreground pgrp. */
-               if (iob->wevent != NULL &&
-                   (foreground || !USERTTY_EVENT(iob->wevent))) {
-                   if (sudo_ev_add(evbase, iob->wevent, NULL, false) == -1)
-                       sudo_fatal(U_("unable to add event to queue"));
-               }
-               /* Re-enable reader if buffer is not full. */
-               if (iob->len != sizeof(iob->buf)) {
-                   if (sudo_ev_add(evbase, iob->revent, NULL, false) == -1)
-                       sudo_fatal(U_("unable to add event to queue"));
-               }
-               break;
+           }
+       }
+       /* Re-enable writer if buffer is not empty. */
+       if (iob->len > iob->off) {
+           if (sudo_ev_add(evbase, iob->wevent, NULL, false) == -1)
+               sudo_fatal(U_("unable to add event to queue"));
+       }
+       /* Enable reader if buffer is not full. */
+       if (iob->revent != NULL &&
+           (ttymode == TERM_RAW || !USERTTY_EVENT(iob->revent))) {
+           if (iob->len != sizeof(iob->buf)) {
+               if (sudo_ev_add(evbase, iob->revent, NULL, false) == -1)
+                   sudo_fatal(U_("unable to add event to queue"));
+           }
        }
     }
 }
@@ -684,8 +691,8 @@ io_buf_new(int rfd, int wfd, bool (*action)(const char *, unsigned int, struct i
     /* Allocate and add to head of list. */
     if ((iob = malloc(sizeof(*iob))) == NULL)
        sudo_fatalx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
-    iob->revent = sudo_ev_alloc(rfd, SUDO_EV_READ, io_callback, iob);
-    iob->wevent = sudo_ev_alloc(wfd, SUDO_EV_WRITE, io_callback, iob);
+    iob->revent = sudo_ev_alloc(rfd, SUDO_EV_READ, read_callback, iob);
+    iob->wevent = sudo_ev_alloc(wfd, SUDO_EV_WRITE, write_callback, iob);
     iob->len = 0;
     iob->off = 0;
     iob->action = action;