Re: [PATCH 3/4] flow: Safer errno handling in flowside_connect() callers

On Tue, 9 Jun 2026 12:32:25 +1000 David Gibson wrote:

flowside_connect() behaves much like connect(2) itself, returning -1 on error with errno set to the error code. One of the callers, in udp_flow_sock(), uses the errno code with flow_dbg_perror() *after* it's called epoll_del() and close() either of which could clobber errno.

Change flowside_connect() to use the more regular convention for internal functions: return a negative errno code on error, rather than just -1. Save it in the callers and use that rather than raw errno to print the message.

Signed-off-by: David Gibson --- flow.c | 6 ++++-- tcp.c | 4 ++-- udp_flow.c | 7 +++---- 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/flow.c b/flow.c index dd92bad7..98828430 100644 --- a/flow.c +++ b/flow.c @@ -259,7 +259,7 @@ int flowside_sock_l4(const struct ctx *c, enum epoll_type type, uint8_t pif, * * Connect @s to the endpoint address and port from @tgt. * - * Return: 0 on success, negative on error + * Return: 0 on success, negative error code on error */ int flowside_connect(const struct ctx *c, int s, uint8_t pif, const struct flowside *tgt) @@ -267,7 +267,9 @@ int flowside_connect(const struct ctx *c, int s, union sockaddr_inany sa;

pif_sockaddr(c, &sa, pif, &tgt->eaddr, tgt->eport); - return connect(s, &sa.sa, socklen_inany(&sa)); + if (connect(s, &sa.sa, socklen_inany(&sa)) < 0) + return -errno; + return 0;

This looks like a good idea nevertheless, but:

}

/** flow_log__ - Log flow-related message, internal helper diff --git a/tcp.c b/tcp.c index 6fba865f..81813643 100644 --- a/tcp.c +++ b/tcp.c @@ -3744,8 +3744,8 @@ static int tcp_flow_repair_connect(const struct ctx *c,

rc = flowside_connect(c, conn->sock, PIF_HOST, tgt); if (rc) { - rc = -errno; - flow_perror(conn, "Failed to connect migrated socket"); + flow_err(conn, "Failed to connect migrated socket: %s", + strerror_(-rc));

...wouldn't it be more convenient to establish that flowside_connect() behaves like connect() also by setting errno (by updating the comment to flowside_connect() accordingly) and use that fact here to keep calling flow_perror(), so that we don't need to open code the strerror_() call?

return rc; }

diff --git a/udp_flow.c b/udp_flow.c index 35417bc4..6edfa65a 100644 --- a/udp_flow.c +++ b/udp_flow.c @@ -88,13 +88,12 @@ static int udp_flow_sock(const struct ctx *c, return rc; }

- if (flowside_connect(c, s, pif, side) < 0) { - rc = -errno; - + if ((rc = flowside_connect(c, s, pif, side)) < 0) { epoll_del(flow_epollfd(&uflow->f), s); close(s);

- flow_dbg_perror(uflow, "Couldn't connect flow socket"); + flow_dbg(uflow, "Couldn't connect flow socket: %s", + strerror_(-rc));

For the same reason, couldn't we just move the existing flow_dbg_perror() call before epoll_del() instead?

return rc; } uflow->s[sidei] = s;

-- Stefano