Date: Mon, 29 Jun 2026 15:52:13 +0000 From: Dag-Erling=?utf-8?Q? Sm=C3=B8rg?=rav <des@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 298f37a71ae6 - main - libfetch: Overhaul socket read / write Message-ID: <6a4294ad.3c153.42c0ca4e@gitrepo.freebsd.org>
index | next in thread | raw e-mail
The branch main has been updated by des: URL: https://cgit.FreeBSD.org/src/commit/?id=298f37a71ae6d9f2cb0c2abd4ff2887e81e0942c commit 298f37a71ae6d9f2cb0c2abd4ff2887e81e0942c Author: Dag-Erling Smørgrav <des@FreeBSD.org> AuthorDate: 2026-06-27 12:16:31 +0000 Commit: Dag-Erling Smørgrav <des@FreeBSD.org> CommitDate: 2026-06-29 13:49:19 +0000 libfetch: Overhaul socket read / write * Make fetch_ssl_read() and fetch_ssl_write() behave more like read(2) and write(2), and drop fetch_socket_read() in favor of read(2). * Don't request POLLERR, it's implied. * Don't needlessly set errno, it's relatively costly. * Always check for EAGAIN from writev(2), otherwise we will abort on a short write instead of proceeding to poll(2). * Always check for EAGAIN from poll(2) even though it can't happen on FreeBSD; POSIX says it can, and it might in the future. * Rewrite fetch_read() and fetch_writev() to be more similar to each other. The main difference is that a partial read is treated as success while a partial write is treated as failure. PR: 296316 MFC after: 1 week --- lib/libfetch/common.c | 206 ++++++++++++++++++++++++++++++-------------------- lib/libfetch/common.h | 4 +- 2 files changed, 127 insertions(+), 83 deletions(-) diff --git a/lib/libfetch/common.c b/lib/libfetch/common.c index 88b5e5a2f096..c5fb45f70da2 100644 --- a/lib/libfetch/common.c +++ b/lib/libfetch/common.c @@ -1253,13 +1253,9 @@ fetch_ssl(conn_t *conn, const struct url *URL, int verbose) #endif } -#define FETCH_READ_WAIT -2 -#define FETCH_READ_ERROR -1 -#define FETCH_READ_DONE 0 - #ifdef WITH_SSL static ssize_t -fetch_ssl_read(SSL *ssl, char *buf, size_t len) +fetch_ssl_read(SSL *ssl, void *buf, size_t len) { ssize_t rlen; int ssl_err; @@ -1267,43 +1263,57 @@ fetch_ssl_read(SSL *ssl, char *buf, size_t len) rlen = SSL_read(ssl, buf, len); if (rlen < 0) { ssl_err = SSL_get_error(ssl, rlen); - if (ssl_err == SSL_ERROR_WANT_READ || - ssl_err == SSL_ERROR_WANT_WRITE) { - return (FETCH_READ_WAIT); - } else { - ERR_print_errors_fp(stderr); - return (FETCH_READ_ERROR); + switch (ssl_err) { + case SSL_ERROR_ZERO_RETURN: + return (0); + case SSL_ERROR_WANT_READ: + errno = EAGAIN; + return (-1); + case SSL_ERROR_SYSCALL: + return (-1); + default: + errno = EPROTO; + return (-1); } } return (rlen); } -#endif static ssize_t -fetch_socket_read(int sd, char *buf, size_t len) +fetch_ssl_write(SSL *ssl, void *buf, size_t len) { - ssize_t rlen; + ssize_t wlen; + int ssl_err; - rlen = read(sd, buf, len); - if (rlen < 0) { - if (errno == EAGAIN || (errno == EINTR && fetchRestartCalls)) { - return (FETCH_READ_WAIT); - } else { - return (FETCH_READ_ERROR); + wlen = SSL_write(ssl, buf, len); + if (wlen < 0) { + ssl_err = SSL_get_error(ssl, wlen); + switch (ssl_err) { + case SSL_ERROR_ZERO_RETURN: + return (0); + case SSL_ERROR_WANT_WRITE: + errno = EAGAIN; + return (-1); + case SSL_ERROR_SYSCALL: + return (-1); + default: + errno = EPROTO; + return (-1); } } - return (rlen); + return (wlen); } +#endif /* - * Read a character from a connection w/ timeout + * Read from a connection w/ timeout */ ssize_t -fetch_read(conn_t *conn, char *buf, size_t len) +fetch_read(conn_t *conn, void *buf, size_t len) { struct timeval now, timeout, delta; struct pollfd pfd; - ssize_t rlen; + ssize_t rlen, total; int deltams; if (fetchTimeout > 0) { @@ -1312,10 +1322,10 @@ fetch_read(conn_t *conn, char *buf, size_t len) } deltams = INFTIM; - memset(&pfd, 0, sizeof pfd); pfd.fd = conn->sd; - pfd.events = POLLIN | POLLERR; + pfd.events = POLLIN; + total = 0; for (;;) { /* * The socket is non-blocking. Instead of the canonical @@ -1323,8 +1333,8 @@ fetch_read(conn_t *conn, char *buf, size_t len) * * 1) call read() or SSL_read(). * 2) if we received some data, return it. - * 3) if an error occurred, return -1. - * 4) if read() or SSL_read() signaled EOF, return. + * 3) if read() or SSL_read() signaled EOF, return. + * 4) if an error occurred, return -1. * 5) if we did not receive any data but we're not at EOF, * call poll(). * @@ -1341,14 +1351,26 @@ fetch_read(conn_t *conn, char *buf, size_t len) rlen = fetch_ssl_read(conn->ssl, buf, len); else #endif - rlen = fetch_socket_read(conn->sd, buf, len); - if (rlen >= 0) { + rlen = read(conn->sd, buf, len); + if (rlen > 0) { + /* something was read */ + total += rlen; + len -= rlen; + if (len == 0) + break; + /* a partial read is success */ + break; + } else if (rlen == 0) { + /* connection closed */ break; - } else if (rlen == FETCH_READ_ERROR) { + } else if (errno != EAGAIN) { + /* error */ + if (errno == EINTR && fetchRestartCalls) + continue; fetch_syserr(); - return (-1); + break; } - // assert(rlen == FETCH_READ_WAIT); + /* check what's left of our timeout */ if (fetchTimeout > 0) { gettimeofday(&now, NULL); if (!timercmp(&timeout, &now, >)) { @@ -1360,16 +1382,19 @@ fetch_read(conn_t *conn, char *buf, size_t len) deltams = delta.tv_sec * 1000 + delta.tv_usec / 1000; } - errno = 0; - pfd.revents = 0; + /* wait for the socket to become readable */ if (poll(&pfd, 1, deltams) < 0) { + if (errno == EAGAIN) + continue; if (errno == EINTR && fetchRestartCalls) continue; - fetch_syserr(); - return (-1); + break; } } - return (rlen); + /* a partial read is success */ + if (rlen < 0 && total == 0) + return (-1); + return (total); } @@ -1426,7 +1451,7 @@ fetch_getln(conn_t *conn) * Write to a connection w/ timeout */ ssize_t -fetch_write(conn_t *conn, const char *buf, size_t len) +fetch_write(conn_t *conn, const void *buf, size_t len) { struct iovec iov; @@ -1447,17 +1472,65 @@ fetch_writev(conn_t *conn, struct iovec *iov, int iovcnt) ssize_t wlen, total; int deltams; - memset(&pfd, 0, sizeof pfd); - if (fetchTimeout) { - pfd.fd = conn->sd; - pfd.events = POLLOUT | POLLERR; + if (fetchTimeout > 0) { gettimeofday(&timeout, NULL); timeout.tv_sec += fetchTimeout; } + deltams = INFTIM; + pfd.fd = conn->sd; + pfd.events = POLLOUT; + total = 0; - while (iovcnt > 0) { - while (fetchTimeout && pfd.revents == 0) { + for (;;) { + /* + * The socket is non-blocking. Instead of the canonical + * poll() -> write(), we do the following: + * + * 1) call write() or SSL_write(). + * 2) if we wrote everything, return success. + * 3) if write() or SSL_write() signaled EOF before we + * wrote everything, return -1. + * 4) if an error occurred, return -1. + * 5) if we did not write everything but we're not at EOF, + * call poll(). + */ +#ifdef WITH_SSL + if (conn->ssl != NULL) { + wlen = fetch_ssl_write(conn->ssl, + iov->iov_base, iov->iov_len); + } else +#endif + wlen = writev(conn->sd, iov, iovcnt); + if (wlen > 0) { + /* something was written */ + total += wlen; + /* skip iovs which were completely written */ + while (iovcnt > 0 && wlen >= (ssize_t)iov->iov_len) { + wlen -= iov->iov_len; + iov++; + iovcnt--; + } + /* are we done? */ + if (iovcnt == 0) + break; + /* skip written portion of current iov */ + iov->iov_len -= wlen; + iov->iov_base = __DECONST(char *, iov->iov_base) + wlen; + /* a partial write is incomplete */ + continue; + } else if (wlen == 0) { + /* connection closed */ + break; + } else if (errno != EAGAIN) { + /* error */ + if (errno == EINTR && fetchRestartCalls) + continue; + fetch_syserr(); + break; + } + /* check what's left of our timeout */ + if (fetchTimeout > 0) { gettimeofday(&now, NULL); if (!timercmp(&timeout, &now, >)) { errno = ETIMEDOUT; @@ -1467,48 +1540,19 @@ fetch_writev(conn_t *conn, struct iovec *iov, int iovcnt) timersub(&timeout, &now, &delta); deltams = delta.tv_sec * 1000 + delta.tv_usec / 1000; - errno = 0; - pfd.revents = 0; - if (poll(&pfd, 1, deltams) < 0) { - /* POSIX compliance */ - if (errno == EAGAIN) - continue; - if (errno == EINTR && fetchRestartCalls) - continue; - return (-1); - } } - errno = 0; -#ifdef WITH_SSL - if (conn->ssl != NULL) - wlen = SSL_write(conn->ssl, - iov->iov_base, iov->iov_len); - else -#endif - wlen = writev(conn->sd, iov, iovcnt); - if (wlen == 0) { - /* we consider a short write a failure */ - /* XXX perhaps we shouldn't in the SSL case */ - errno = EPIPE; - fetch_syserr(); - return (-1); - } - if (wlen < 0) { + /* wait for the socket to become writeable */ + if (poll(&pfd, 1, deltams) < 0) { + if (errno == EAGAIN) + continue; if (errno == EINTR && fetchRestartCalls) continue; return (-1); } - total += wlen; - while (iovcnt > 0 && wlen >= (ssize_t)iov->iov_len) { - wlen -= iov->iov_len; - iov++; - iovcnt--; - } - if (iovcnt > 0) { - iov->iov_len -= wlen; - iov->iov_base = __DECONST(char *, iov->iov_base) + wlen; - } } + /* a partial write is failure */ + if (iovcnt > 0) + return (-1); return (total); } diff --git a/lib/libfetch/common.h b/lib/libfetch/common.h index 06089aae5451..479c5e03c0d1 100644 --- a/lib/libfetch/common.h +++ b/lib/libfetch/common.h @@ -119,9 +119,9 @@ conn_t *fetch_ref(conn_t *); int fetch_ssl_cb_verify_crt(int, X509_STORE_CTX*); #endif int fetch_ssl(conn_t *, const struct url *, int); -ssize_t fetch_read(conn_t *, char *, size_t); +ssize_t fetch_read(conn_t *, void *, size_t); int fetch_getln(conn_t *); -ssize_t fetch_write(conn_t *, const char *, size_t); +ssize_t fetch_write(conn_t *, const void *, size_t); ssize_t fetch_writev(conn_t *, struct iovec *, int); int fetch_putln(conn_t *, const char *, size_t); int fetch_close(conn_t *);home | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6a4294ad.3c153.42c0ca4e>