From dc4ee0013150965646cb62c0dc549fedc1054ff7 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 5 Sep 2026 16:19:06 +0200 Subject: [PATCH] Trim comments --- .../components/async_tcp/async_tcp_socket.cpp | 3 +-- .../components/socket/lwip_raw_tcp_impl.cpp | 24 +++++++------------ esphome/components/socket/lwip_raw_tcp_impl.h | 22 +++++++---------- esphome/components/socket/socket.h | 7 ++---- .../fixtures/socket_set_sockaddr.yaml | 3 +-- tests/integration/test_socket_set_sockaddr.py | 7 +----- 6 files changed, 22 insertions(+), 44 deletions(-) diff --git a/esphome/components/async_tcp/async_tcp_socket.cpp b/esphome/components/async_tcp/async_tcp_socket.cpp index 08bc1ae49c..853ff206c2 100644 --- a/esphome/components/async_tcp/async_tcp_socket.cpp +++ b/esphome/components/async_tcp/async_tcp_socket.cpp @@ -42,8 +42,7 @@ bool AsyncClient::connect(const char *host, uint16_t port) { } if (socket_->setblocking(false) != 0) { - // Capture before the log and reset() below can clobber errno; a blocking - // connect()/read() would otherwise stall the whole loop + // Capture before the log and close() clobber errno const int saved_errno = errno; ESP_LOGE(TAG, "Failed to set nonblocking: errno %d", saved_errno); close(); diff --git a/esphome/components/socket/lwip_raw_tcp_impl.cpp b/esphome/components/socket/lwip_raw_tcp_impl.cpp index 3a90b169cb..b411445b98 100644 --- a/esphome/components/socket/lwip_raw_tcp_impl.cpp +++ b/esphome/components/socket/lwip_raw_tcp_impl.cpp @@ -87,9 +87,8 @@ static int lwip_err_to_errno(err_t err) { // Must be called before destroying the object that tcp_arg points to — // tcp_abort() triggers the err callback synchronously, which would // otherwise call back into a partially-destroyed object. -// tcp_sent/tcp_poll are not cleared because this implementation -// never registers them, and the tcp_connect callback only fires on -// SYN_SENT -> ESTABLISHED, which cannot follow an abort or close. +// tcp_sent/tcp_poll are never registered and the connect callback cannot +// fire after abort or close, so neither is cleared. static void pcb_detach_abort(struct tcp_pcb *pcb) { tcp_arg(pcb, nullptr); tcp_recv(pcb, nullptr); @@ -102,8 +101,7 @@ static void pcb_detach_abort(struct tcp_pcb *pcb) { // After tcp_close(), the PCB remains alive during the TCP close handshake // (FIN_WAIT, TIME_WAIT states). Without clearing callbacks first, LWIP // would call recv/err on a destroyed socket object, corrupting the heap. -// tcp_sent/tcp_poll and the tcp_connect callback are not cleared for the -// same reasons as in pcb_detach_abort(). +// Callbacks are left as in pcb_detach_abort(). // Returns ERR_OK on success; on failure the PCB is aborted instead. static err_t pcb_detach_close(struct tcp_pcb *pcb) { tcp_arg(pcb, nullptr); @@ -436,9 +434,8 @@ void LWIPRawImpl::s_err_fn(void *arg, err_t err) { auto *arg_this = reinterpret_cast(arg); ESP_LOGVV(TAG, "socket %p: err(err=%d)", arg_this, err); if (arg_this->connect_err_ == EINPROGRESS) { - // A connect that never established: RST is a refusal, anything else is - // the SYN retransmits giving up. Written before pcb_ so poll_connect() - // never sees a dead pcb without its reason. + // Refused (RST) or SYN retries exhausted; written before pcb_ so + // poll_connect() never sees a dead pcb without its reason arg_this->connect_err_ = err == ERR_RST ? ECONNREFUSED : ETIMEDOUT; } arg_this->pcb_ = nullptr; @@ -446,8 +443,7 @@ void LWIPRawImpl::s_err_fn(void *arg, err_t err) { } err_t LWIPRawImpl::s_connected_fn(void *arg, struct tcp_pcb *pcb, err_t err) { - // LWIP CALLBACK — same constraints as s_err_fn. err is always ERR_OK; a - // failed connect arrives through s_err_fn instead. + // LWIP CALLBACK, same constraints as s_err_fn; err is always ERR_OK auto *arg_this = reinterpret_cast(arg); arg_this->connect_err_ = EISCONN; esphome::wake_loop_any_context(); @@ -470,8 +466,7 @@ int LWIPRawImpl::connect(const struct sockaddr *addr, socklen_t addrlen) { return -1; } #if LWIP_IPV6 - // tcp_connect needs a concrete address type. A remembered IPv4 peer on an - // IPv6 build arrives as a v4-mapped address and must be dialed as IPv4. + // tcp_connect needs a concrete type; a remembered IPv4 peer arrives v4-mapped if (IP_IS_ANY_TYPE_VAL(ip)) { if (ip6_addr_isipv4mappedipv6(ip_2_ip6(&ip))) { unmap_ipv4_mapped_ipv6(ip_2_ip4(&ip), ip_2_ip6(&ip)); @@ -628,9 +623,8 @@ ssize_t LWIPRawImpl::read_locked_(void *buf, size_t len) { } ssize_t LWIPRawImpl::read(void *buf, size_t len) { - // Would block: let queued WiFi RX reach lwip first so this read may - // succeed; otherwise inbound segments can sit unprocessed for seconds - // while the main loop polls + // Let queued WiFi RX reach lwip first; otherwise inbound segments can + // sit unprocessed for seconds while the main loop polls if (this->waiting_for_data_()) { yield_to_sys(); } diff --git a/esphome/components/socket/lwip_raw_tcp_impl.h b/esphome/components/socket/lwip_raw_tcp_impl.h index 5d3dd36536..ed8057e289 100644 --- a/esphome/components/socket/lwip_raw_tcp_impl.h +++ b/esphome/components/socket/lwip_raw_tcp_impl.h @@ -50,8 +50,7 @@ class LWIPRawCommon { protected: int ip2sockaddr_(ip_addr_t *ip, uint16_t port, struct sockaddr *name, socklen_t *addrlen); - /// Convert a sockaddr of this socket's family to an lwip address and port. - /// Returns false with errno set on a family or length mismatch. + /// sockaddr of this socket's family to lwip address and port; false with errno on mismatch bool sockaddr2ip_(const struct sockaddr *name, socklen_t addrlen, ip_addr_t *ip, uint16_t *port) const; // Member ordering optimized to minimize padding on 32-bit systems @@ -61,15 +60,13 @@ class LWIPRawCommon { bool nodelay_ = false; sa_family_t family_ = 0; uint8_t recv_timeout_cs_ = 0; // SO_RCVTIMEO in centiseconds (0 = no timeout, max 2.55s) - // State of connect() on this socket: 0 before one was started, EINPROGRESS - // while the SYN is out, EISCONN once established, otherwise the errno the - // lwip callbacks recorded for its failure. Fits the padding byte here. + // 0 before connect(), EINPROGRESS while pending, EISCONN once established, + // else the failure errno the callbacks recorded; fills the padding byte uint8_t connect_err_ = 0; static_assert(EINPROGRESS < 256 && EISCONN < 256 && ECONNREFUSED < 256 && ECONNRESET < 256 && ETIMEDOUT < 256, "connect_err_ stores errno values in a byte"); }; -// The connect state must stay inside the padding: no socket, listening or -// accepted, pays RAM for it +// The connect state must stay in the padding so no socket pays RAM for it static_assert(sizeof(LWIPRawCommon) == sizeof(struct tcp_pcb *) + 4, "LWIPRawCommon grew past one word of flags"); /// Connected socket implementation for LWIP raw TCP. @@ -95,14 +92,11 @@ class LWIPRawImpl : public LWIPRawCommon { errno = EOPNOTSUPP; return -1; } - /// Start a non-blocking connect. Always returns -1 with errno EINPROGRESS - /// when the SYN was queued; completion is reported by poll_connect(). - /// addr must be of the family the socket was created with; an IPv4 peer - /// on an AF_INET6 socket arrives as a v4-mapped sockaddr_in6. + /// Non-blocking: returns -1/EINPROGRESS once the SYN is queued, see poll_connect(). + /// addr must match the socket family; an IPv4 peer on AF_INET6 arrives v4-mapped. int connect(const struct sockaddr *addr, socklen_t addrlen); - // Intentionally unlocked like ready(): reads one pointer and one byte that - // the callbacks write in the order the checks depend on (error byte first, - // then pcb_), so a torn read only costs one extra poll. + // Unlocked like ready(): the callbacks write the error byte before pcb_, + // so a torn read only costs one extra poll ConnectPollResult poll_connect(int &err_out) const; ssize_t read(void *buf, size_t len); ssize_t readv(const struct iovec *iov, int iovcnt); diff --git a/esphome/components/socket/socket.h b/esphome/components/socket/socket.h index 24bd63d964..8ce392ff1d 100644 --- a/esphome/components/socket/socket.h +++ b/esphome/components/socket/socket.h @@ -145,11 +145,8 @@ inline socklen_t set_sockaddr(struct sockaddr *addr, socklen_t addrlen, const st /// Set a sockaddr to the any address and specified port for the IP version used by socket_ip(). socklen_t set_sockaddr_any(struct sockaddr *addr, socklen_t addrlen, uint16_t port); -/// Check a non-blocking connect() for completion without blocking. Only -/// meaningful after connect() returned -1 with errno EINPROGRESS. On -/// CONNECT_POLL_RESULT_ERROR, err_out holds the socket's SO_ERROR (or errno when the -/// poll itself failed) on fd based implementations, and the failure recorded -/// by the lwip callbacks on the raw lwip implementation. +/// Poll a connect() that returned EINPROGRESS. On error, err_out is SO_ERROR (or +/// errno) on fd implementations and the failure the callbacks recorded on raw lwip. #ifdef USE_SOCKET_IMPL_LWIP_TCP inline ConnectPollResult poll_connect(Socket &sock, int &err_out) { return sock.poll_connect(err_out); } #else diff --git a/tests/integration/fixtures/socket_set_sockaddr.yaml b/tests/integration/fixtures/socket_set_sockaddr.yaml index f0a14d24f8..3f63584d1d 100644 --- a/tests/integration/fixtures/socket_set_sockaddr.yaml +++ b/tests/integration/fixtures/socket_set_sockaddr.yaml @@ -3,8 +3,7 @@ esphome: on_boot: then: - lambda: |- - // Exercise the contract callers rely on: 0 for text that is not an - // address, the sockaddr length otherwise, broadcast included + // 0 for text that is not an address, the length otherwise, broadcast included struct sockaddr_storage addr; auto *sa = reinterpret_cast(&addr); ESP_LOGI("test", "SET_SOCKADDR invalid=%u valid=%u broadcast=%u", diff --git a/tests/integration/test_socket_set_sockaddr.py b/tests/integration/test_socket_set_sockaddr.py index 9b6c2a6c9a..cfa6d3c996 100644 --- a/tests/integration/test_socket_set_sockaddr.py +++ b/tests/integration/test_socket_set_sockaddr.py @@ -1,9 +1,4 @@ -"""Integration test for the socket::set_sockaddr failure contract. - -Callers skip an address when set_sockaddr returns 0, so text that is not an -address must return 0 while a real address, including the broadcast address -that inet_addr() would have reported as a failure, returns its length. -""" +"""Integration test for the socket::set_sockaddr failure contract.""" import asyncio import re