pty: Refactor forwarding

This should make the process slightly more efficient because we are
checking if there is anything in the buffers before entering the loop.
We also handle any return values from the read()/write() calls better so
that we won't continue to loop too often.

Signed-off-by: Michael Tremer <michael.tremer@ipfire.org>

diff --git a/src/libpakfire/pty.c b/src/libpakfire/pty.c
index a9cff21383825b8d956b68b9fbdc981362a45575..c4c190ecb6262b92f5e5b8bc9b637528a06dda1b 100644 (file)
--- a/src/libpakfire/pty.c
+++ b/src/libpakfire/pty.c
@@ -50,6 +50,7 @@ struct pakfire_pty_stdio {
        enum pakfire_pty_io {
                PAKFIRE_PTY_READY_TO_READ  = (1 << 0),
                PAKFIRE_PTY_READY_TO_WRITE = (1 << 1),
+               PAKFIRE_PTY_HANGUP         = (1 << 2),
        } io;
 
        // Event Source
@@ -267,6 +268,27 @@ static int pakfire_pty_done(struct pakfire_pty* pty, int code) {
        return sd_event_exit(loop, code < 0 ? EXIT_FAILURE : code);
 }
 
+static int pakfire_pty_buffer_is_full(struct pakfire_pty* pty, const struct pakfire_pty_stdio* stdio) {
+       if (stdio->buffered >= sizeof(stdio->buffer))
+               return 1;
+
+       return 0;
+}
+
+static int pakfire_pty_buffer_is_empty(struct pakfire_pty* pty, const struct pakfire_pty_stdio* stdio) {
+       if (stdio->buffered == 0)
+               return 1;
+
+       return 0;
+}
+
+static int pakfire_pty_buffer_has_data(struct pakfire_pty* pty, const struct pakfire_pty_stdio* stdio) {
+       if (stdio->buffered)
+               return 1;
+
+       return 0;
+}
+
 /*
        Reads as much data as possible into the buffer
 */
@@ -274,20 +296,13 @@ static int pakfire_pty_fill_buffer(struct pakfire_pty* pty, int fd, struct pakfi
        int r;
 
        // Skip this if there is not space left in the buffer
-       if (stdio->buffered >= sizeof(stdio->buffer))
-               return 0;
+       if (pakfire_pty_buffer_is_full(pty, stdio))
+               return -ENOBUFS;
 
        // Fill the buffer
        r = read(fd, stdio->buffer + stdio->buffered, sizeof(stdio->buffer) - stdio->buffered);
        if (r < 0) {
-               switch (errno) {
-                       case EAGAIN:
-                       case EIO:
-                               break;
-
-                       default:
-                               return -errno;
-               }
+               return -errno;
 
        // EOF
        } else if (r == 0) {
@@ -305,36 +320,22 @@ static int pakfire_pty_fill_buffer(struct pakfire_pty* pty, int fd, struct pakfi
        Writes as much data as possible from the buffer
 */
 static int pakfire_pty_drain_buffer(struct pakfire_pty* pty, int fd, struct pakfire_pty_stdio* stdio) {
-       int r;
-
-       // Do not try to write to an invalid file descriptor
-       if (fd < 0)
-               return 0;
+       ssize_t bytes_written;
 
        // Nothing to do if the buffer is empty
-       if (!stdio->buffered)
+       if (pakfire_pty_buffer_is_empty(pty, stdio))
                return 0;
 
        // Drain the buffer
-       r = write(fd, stdio->buffer, stdio->buffered);
-       if (r < 0) {
-               switch (errno) {
-                       case EAGAIN:
-                       case EIO:
-                               break;
-
-                       default:
-                               return -errno;
-               }
+       bytes_written = write(fd, stdio->buffer, stdio->buffered);
+       if (bytes_written < 0)
+               return -errno;
 
        // Successful write
-       } else {
-               memmove(stdio->buffer, stdio->buffer + r, stdio->buffered - r);
+       memmove(stdio->buffer, stdio->buffer + bytes_written, stdio->buffered - bytes_written);
+       stdio->buffered -= bytes_written;
 
-               stdio->buffered -= r;
-       }
-
-       return 0;
+       return bytes_written;
 }
 
 /*
@@ -353,64 +354,103 @@ static int pakfire_pty_forward(struct pakfire_pty* pty) {
                        return -EINVAL;
        }
 
-       while (pty->master.io || pty->stdin.io || pty->stdout.io) {
+       while (
+                       ((pty->stdin.io & PAKFIRE_PTY_READY_TO_READ) && !pakfire_pty_buffer_is_full(pty, &pty->stdin)) ||
+                       ((pty->master.io & PAKFIRE_PTY_READY_TO_WRITE) && pakfire_pty_buffer_has_data(pty, &pty->stdin)) ||
+                       ((pty->master.io & PAKFIRE_PTY_READY_TO_READ) && !pakfire_pty_buffer_is_full(pty, &pty->stdout)) ||
+                       ((pty->stdout.io & PAKFIRE_PTY_READY_TO_WRITE) && pakfire_pty_buffer_has_data(pty, &pty->stdout))
+               ) {
                // Read from standard input
                if (pty->stdin.io & PAKFIRE_PTY_READY_TO_READ) {
                        r = pakfire_pty_fill_buffer(pty, pty->stdin.fd, &pty->stdin);
                        if (r < 0) {
-                               CTX_ERROR(pty->ctx, "Failed reading from standard input: %s\n", strerror(-r));
-                               goto ERROR;
+                               switch (-r) {
+                                       case EAGAIN:
+                                               pty->stdin.io &= ~PAKFIRE_PTY_READY_TO_READ;
+                                               break;
+
+                                       case EIO:
+                                               pty->stdin.io |= PAKFIRE_PTY_HANGUP;
+                                               break;
+
+                                       default:
+                                               CTX_ERROR(pty->ctx, "Failed reading from standard input: %s\n",
+                                                       strerror(-r));
+                                               goto ERROR;
+                               }
+
+                       // EOF?
+                       } else if (r == 0) {
+                               pty->stdin.io |= PAKFIRE_PTY_HANGUP;
                        }
-
-                       // We are done reading for now
-                       pty->stdin.io &= ~PAKFIRE_PTY_READY_TO_READ;
-
-                       // But we may have data to write
-                       if (pty->stdin.buffered)
-                               pty->master.io |= PAKFIRE_PTY_READY_TO_WRITE;
                }
 
                // Write to the master
                if (pty->master.io & PAKFIRE_PTY_READY_TO_WRITE) {
                        r = pakfire_pty_drain_buffer(pty, pty->master.fd, &pty->stdin);
                        if (r < 0) {
-                               CTX_ERROR(pty->ctx, "Failed writing to the PTY: %s\n", strerror(-r));
-                               goto ERROR;
+                               switch (-r) {
+                                       case EAGAIN:
+                                               pty->master.io &= ~PAKFIRE_PTY_READY_TO_WRITE;
+                                               break;
+
+                                       case EIO:
+                                       case EPIPE:
+                                       case ECONNRESET:
+                                               pty->master.io |= PAKFIRE_PTY_HANGUP;
+                                               break;
+
+                                       default:
+                                               CTX_ERROR(pty->ctx, "Failed writing to the PTY: %s\n", strerror(-r));
+                                               goto ERROR;
+                               }
                        }
-
-                       // We are done writing for now
-                       if (!pty->stdin.buffered)
-                               pty->master.io &= ~PAKFIRE_PTY_READY_TO_WRITE;
                }
 
                // Read from the master
                if (pty->master.io & PAKFIRE_PTY_READY_TO_READ) {
                        r = pakfire_pty_fill_buffer(pty, pty->master.fd, &pty->stdout);
                        if (r < 0) {
-                               CTX_ERROR(pty->ctx, "Failed reading from the PTY: %s\n", strerror(-r));
-                               goto ERROR;
+                               switch (-r) {
+                                       case EAGAIN:
+                                               pty->master.io &= ~PAKFIRE_PTY_READY_TO_READ;
+                                               break;
+
+                                       case EIO:
+                                       case EPIPE:
+                                       case ECONNRESET:
+                                               pty->master.io |= PAKFIRE_PTY_HANGUP;
+                                               break;
+
+                                       default:
+                                               CTX_ERROR(pty->ctx, "Failed reading from the PTY: %s\n", strerror(-r));
+                                               goto ERROR;
+                               }
                        }
-
-                       // We are done reading for now
-                       pty->master.io &= ~PAKFIRE_PTY_READY_TO_READ;
-
-                       // But we may have data to write
-                       if (pty->stdout.buffered)
-                               pty->stdout.io |= PAKFIRE_PTY_READY_TO_WRITE;
                }
 
                // Write to standard output
                if (pty->stdout.io & PAKFIRE_PTY_READY_TO_WRITE) {
                        r = pakfire_pty_drain_buffer(pty, pty->stdout.fd, &pty->stdout);
                        if (r < 0) {
-                               CTX_ERROR(pty->ctx, "Failed writing to standard output: %s\n", strerror(-r));
-                               goto ERROR;
+                               switch (-r) {
+                                       case EAGAIN:
+                                               pty->stdout.io &= ~PAKFIRE_PTY_READY_TO_WRITE;
+                                               break;
+
+                                       case EIO:
+                                               pty->stdout.io |= PAKFIRE_PTY_HANGUP;
+                                               break;
+
+                                       default:
+                                               CTX_ERROR(pty->ctx, "Failed writing to standard output: %s\n", strerror(-r));
+                                               goto ERROR;
+                               }
                        }
-
-                       // We are done writing for now
-                       if (!pty->stdout.buffered)
-                               pty->stdout.io &= ~PAKFIRE_PTY_READY_TO_WRITE;
                }
+
+               if ((pty->master.io|pty->stdin.io|pty->stdout.io) & PAKFIRE_PTY_HANGUP)
+                       return pakfire_pty_done(pty, 0);
        }
 
        // If we have been requested to drain, and are fully drained, we are done
@@ -458,6 +498,10 @@ static int pakfire_pty_activity(struct pakfire_pty* pty, struct pakfire_pty_stdi
        if (events & (EPOLLOUT|EPOLLHUP))
                stdio->io |= PAKFIRE_PTY_READY_TO_WRITE;
 
+       // Did the file descriptor get closed?
+       if (events & EPOLLHUP)
+               stdio->io |= PAKFIRE_PTY_HANGUP;
+
        return pakfire_pty_forward(pty);
 }