From: Michael Tremer Date: Thu, 10 Oct 2024 09:47:36 +0000 (+0000) Subject: pty: Refactor forwarding X-Git-Tag: 0.9.30~1100 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=68698aab439507ac57365ebdabbf40242522287b;p=pakfire.git 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 --- diff --git a/src/libpakfire/pty.c b/src/libpakfire/pty.c index a9cff2138..c4c190ecb 100644 --- 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); }