git: 298f37a71ae6 - main - libfetch: Overhaul socket read / write
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Mon, 29 Jun 2026 15:52:13 UTC
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 *);