From 54f113f6c767471c6380d6066aa0ca9f08402dd1 Mon Sep 17 00:00:00 2001 From: Amos Jeffries Date: Fri, 9 May 2025 10:32:44 +1200 Subject: [PATCH 1/3] Maintenance: use SetSocketOption() template --- src/Makefile.am | 16 +++++- src/client_side.cc | 5 +- src/client_side_request.cc | 3 +- src/clients/FtpGateway.cc | 10 +--- src/comm.cc | 74 ++++++++------------------- src/comm/ConnOpener.cc | 6 +-- src/comm/Makefile.am | 2 + src/comm/Tcp.cc | 32 +++--------- src/comm/Tcp.h | 23 +++++++++ src/comm/TcpAcceptor.cc | 13 ++--- src/ip/Intercept.cc | 5 +- src/ip/QosConfig.cc | 102 +++++++++++++++---------------------- src/ip/QosConfig.h | 25 +++------ src/ip/tools.cc | 4 +- src/ipc_win32.cc | 2 +- src/multicast.cc | 24 ++++----- src/tests/stub_libcomm.cc | 1 + src/tests/stub_libip.cc | 16 +++--- src/wccp2.cc | 12 ++--- 19 files changed, 154 insertions(+), 221 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 609572f26c6..e90c1e61082 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -917,10 +917,13 @@ nodist_tests_testURL_SOURCES = \ tests/stub_access_log.cc \ anyp/Uri.h \ anyp/UriScheme.h \ + tests/stub_cache_manager.cc \ tests/stub_cbdata.cc \ tests/stub_debug.cc \ + tests/stub_libcomm.cc \ tests/stub_libhttp.cc \ - tests/stub_libmem.cc + tests/stub_libmem.cc \ + tests/stub_MemBuf.cc tests_testURL_LDADD = \ libsquid.la \ anyp/libanyp.la \ @@ -2033,6 +2036,7 @@ tests_testHttp1Parser_SOURCES = \ wordlist.h nodist_tests_testHttp1Parser_SOURCES = \ $(TESTSOURCES) \ + tests/stub_libcomm.cc \ tests/stub_libtime.cc tests_testHttp1Parser_LDADD= \ http/libhttp.la \ @@ -2363,8 +2367,8 @@ tests_testHttpRequest_LDADD = \ anyp/libanyp.la \ $(SNMP_LIBS) \ icmp/libicmp.la \ - comm/libcomm.la \ ip/libip.la \ + comm/libcomm.la \ log/liblog.la \ format/libformat.la \ store/libstore.la \ @@ -2397,8 +2401,12 @@ check_PROGRAMS += tests/testIpAddress tests_testIpAddress_SOURCES = \ tests/testIpAddress.cc nodist_tests_testIpAddress_SOURCES = \ + tests/stub_MemBuf.cc \ tests/stub_SBuf.cc \ + tests/stub_cache_manager.cc \ + tests/stub_cbdata.cc \ tests/stub_debug.cc \ + tests/stub_libcomm.cc \ tests/stub_libmem.cc \ tests/stub_tools.cc tests_testIpAddress_LDADD = \ @@ -2416,9 +2424,13 @@ check_PROGRAMS += tests/testIcmp tests_testIcmp_SOURCES = \ tests/testIcmp.cc nodist_tests_testIcmp_SOURCES = \ + tests/stub_MemBuf.cc \ tests/stub_SBuf.cc \ + tests/stub_cache_manager.cc \ + tests/stub_cbdata.cc \ tests/stub_debug.cc \ icmp/Icmp.h \ + tests/stub_libcomm.cc \ tests/stub_libmem.cc \ tests/stub_libtime.cc tests_testIcmp_LDADD=\ diff --git a/src/client_side.cc b/src/client_side.cc index 17d377dfefd..60a4e93c878 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -2134,10 +2134,7 @@ ConnStateData::start() (transparent() || port->disable_pmtu_discovery == DISABLE_PMTU_ALWAYS)) { #if defined(IP_MTU_DISCOVER) && defined(IP_PMTUDISC_DONT) int i = IP_PMTUDISC_DONT; - if (setsockopt(clientConnection->fd, SOL_IP, IP_MTU_DISCOVER, &i, sizeof(i)) < 0) { - int xerrno = errno; - debugs(33, 2, "WARNING: Path MTU discovery disabling failed on " << clientConnection << " : " << xstrerr(xerrno)); - } + Comm::SetSocketOption(clientConnection->fd, SOL_IP, IP_MTU_DISCOVER, i, ToSBuf("IP_MTU_DISCOVER disabled for client ", clientConnection)); #else static bool reported = false; diff --git a/src/client_side_request.cc b/src/client_side_request.cc index 797de8ee2df..140b7ef423c 100644 --- a/src/client_side_request.cc +++ b/src/client_side_request.cc @@ -1708,8 +1708,7 @@ ClientHttpRequest::doCallouts() if (!calloutContext->toClientMarkingDone) { calloutContext->toClientMarkingDone = true; - tos_t tos = aclMapTOS(Ip::Qos::TheConfig.tosToClient, &ch); - if (tos) + if (tos_t tos = aclMapTOS(Ip::Qos::TheConfig.tosToClient, &ch)) Ip::Qos::setSockTos(getConn()->clientConnection, tos); const auto packetMark = aclFindNfMarkConfig(Ip::Qos::TheConfig.nfmarkToClient, &ch); diff --git a/src/clients/FtpGateway.cc b/src/clients/FtpGateway.cc index fc0f3a2e94d..9accdce277c 100644 --- a/src/clients/FtpGateway.cc +++ b/src/clients/FtpGateway.cc @@ -1774,15 +1774,7 @@ ftpOpenListenSocket(Ftp::Gateway * ftpState, int fallback) * REUSEADDR is needed in fallback mode, since the same port is * used for both control and data. */ - if (fallback) { - int on = 1; - errno = 0; - if (setsockopt(ftpState->ctrl.conn->fd, SOL_SOCKET, SO_REUSEADDR, - (char *) &on, sizeof(on)) == -1) { - int xerrno = errno; - // SO_REUSEADDR is only an optimization, no need to be verbose about error - debugs(9, 4, "setsockopt failed: " << xstrerr(xerrno)); - } + if (fallback && Comm::SetBooleanSocketOption(ftpState->ctrl.conn->fd, SOL_SOCKET, SO_REUSEADDR, true, SBuf("SO_REUSEADDR"))) { ftpState->ctrl.conn->flags |= COMM_REUSEADDR; temp->flags |= COMM_REUSEADDR; } else { diff --git a/src/comm.cc b/src/comm.cc index 4b88933c015..68e97bb6e01 100644 --- a/src/comm.cc +++ b/src/comm.cc @@ -210,11 +210,7 @@ static void commSetBindAddressNoPort(const int fd) { #if defined(IP_BIND_ADDRESS_NO_PORT) - int flag = 1; - if (setsockopt(fd, IPPROTO_IP, IP_BIND_ADDRESS_NO_PORT, reinterpret_cast(&flag), sizeof(flag)) < 0) { - const auto savedErrno = errno; - debugs(50, DBG_IMPORTANT, "ERROR: setsockopt(IP_BIND_ADDRESS_NO_PORT) failure: " << xstrerr(savedErrno)); - } + Comm::SetBooleanSocketOption(fd, IPPROTO_IP, IP_BIND_ADDRESS_NO_PORT, true, SBuf("IP_BIND_ADDRESS_NO_PORT")); #else (void)fd; #endif @@ -291,16 +287,13 @@ limitError(int const anErrno) } static void -comm_set_v6only(int fd, int tos) +comm_set_v6only(int fd, bool enabled) { -#ifdef IPV6_V6ONLY - if (setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, (char *) &tos, sizeof(int)) < 0) { - int xerrno = errno; - debugs(50, DBG_IMPORTANT, MYNAME << "setsockopt(IPV6_V6ONLY) " << (tos?"ON":"OFF") << " for FD " << fd << ": " << xstrerr(xerrno)); - } +#if defined(IPV6_V6ONLY) + Comm::SetBooleanSocketOption(fd, IPPROTO_IPV6, IPV6_V6ONLY, enabled, SBuf("IPV6_V6ONLY")); #else - debugs(50, DBG_CRITICAL, MYNAME << "WARNING: setsockopt(IPV6_V6ONLY) not supported on this platform"); -#endif /* sockopt */ + debugs(50, DBG_CRITICAL, "WARNING: setsockopt(IPV6_V6ONLY) not supported on this platform"); +#endif } /** @@ -315,17 +308,20 @@ comm_set_transparent(int fd) #if _SQUID_LINUX_ && defined(IP_TRANSPARENT) // Linux # define soLevel SOL_IP # define soFlag IP_TRANSPARENT + const SBuf name("IP_TRANSPARENT"); bool doneSuid = false; #elif defined(SO_BINDANY) // OpenBSD 4.7+ and NetBSD with PF # define soLevel SOL_SOCKET # define soFlag SO_BINDANY + const SBuf name("SO_BINDANY"); enter_suid(); bool doneSuid = true; #elif defined(IP_BINDANY) // FreeBSD with IPFW # define soLevel IPPROTO_IP # define soFlag IP_BINDANY + const SBuf name("IP_BINDANY"); enter_suid(); bool doneSuid = true; @@ -335,11 +331,7 @@ comm_set_transparent(int fd) #endif /* sockopt */ #if defined(soLevel) && defined(soFlag) - int tos = 1; - if (setsockopt(fd, soLevel, soFlag, (char *) &tos, sizeof(int)) < 0) { - int xerrno = errno; - debugs(50, DBG_IMPORTANT, MYNAME << "setsockopt(TPROXY) on FD " << fd << ": " << xstrerr(xerrno)); - } else { + if (Comm::SetBooleanSocketOption(fd, soLevel, soFlag, true, name)) { /* mark the socket as having transparent options */ fd_table[fd].flags.transparent = true; } @@ -423,12 +415,12 @@ comm_openex(int sock_type, debugs(50, 3, "comm_openex: Opened socket " << conn << " : family=" << AI->ai_family << ", type=" << AI->ai_socktype << ", protocol=" << AI->ai_protocol ); if ( Ip::EnableIpv6&IPV6_SPECIAL_SPLITSTACK && addr.isIPv6() ) - comm_set_v6only(conn->fd, 1); + comm_set_v6only(conn->fd, true); /* Windows Vista supports Dual-Sockets. BUT defaults them to V6ONLY. Turn it OFF. */ /* Other OS may have this administratively disabled for general use. Same deal. */ if ( Ip::EnableIpv6&IPV6_SPECIAL_V4MAPPING && addr.isIPv6() ) - comm_set_v6only(conn->fd, 0); + comm_set_v6only(conn->fd, false); comm_init_opened(conn, note, AI); new_socket = comm_apply_flags(conn->fd, addr, flags, AI); @@ -505,8 +497,7 @@ comm_apply_flags(int new_socket, #if defined(SO_REUSEPORT) if (flags & COMM_REUSEPORT) { - int on = 1; - if (setsockopt(new_socket, SOL_SOCKET, SO_REUSEPORT, reinterpret_cast(&on), sizeof(on)) < 0) { + if (!Comm::SetBooleanSocketOption(new_socket, SOL_SOCKET, SO_REUSEPORT, true, ToSBuf("SO_REUSEPORT on ", addr))) { const auto savedErrno = errno; const auto errorMessage = ToSBuf("cannot enable SO_REUSEPORT socket option when binding to ", addr, ": ", xstrerr(savedErrno)); @@ -770,10 +761,7 @@ commConfigureLinger(const int fd, const OnOff enabled) fd_table[fd].flags.harshClosureRequested = (l.l_onoff && !l.l_linger); // close(2) sends TCP RST if true - if (setsockopt(fd, SOL_SOCKET, SO_LINGER, reinterpret_cast(&l), sizeof(l)) < 0) { - const auto xerrno = errno; - debugs(50, DBG_CRITICAL, "ERROR: Failed to set closure behavior (SO_LINGER) for FD " << fd << ": " << xstrerr(xerrno)); - } + Comm::SetSocketOption(fd, SOL_SOCKET, SO_LINGER, l, ToSBuf("SO_LINGER (0 seconds) ", (l.l_onoff?"enabled":"disabled"))); } /** @@ -1011,29 +999,16 @@ comm_remove_close_handler(int fd, AsyncCall::Pointer &call) static void commSetReuseAddr(int fd) { - int on = 1; - if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (char *) &on, sizeof(on)) < 0) { - int xerrno = errno; - debugs(50, DBG_IMPORTANT, MYNAME << "FD " << fd << ": " << xstrerr(xerrno)); - } + Comm::SetBooleanSocketOption(fd, SOL_SOCKET, SO_REUSEADDR, true, SBuf("SO_REUSEADDR")); } static void commSetTcpRcvbuf(int fd, int size) { - if (setsockopt(fd, SOL_SOCKET, SO_RCVBUF, (char *) &size, sizeof(size)) < 0) { - int xerrno = errno; - debugs(50, DBG_IMPORTANT, MYNAME << "FD " << fd << ", SIZE " << size << ": " << xstrerr(xerrno)); - } - if (setsockopt(fd, SOL_SOCKET, SO_SNDBUF, (char *) &size, sizeof(size)) < 0) { - int xerrno = errno; - debugs(50, DBG_IMPORTANT, MYNAME << "FD " << fd << ", SIZE " << size << ": " << xstrerr(xerrno)); - } -#ifdef TCP_WINDOW_CLAMP - if (setsockopt(fd, SOL_TCP, TCP_WINDOW_CLAMP, (char *) &size, sizeof(size)) < 0) { - int xerrno = errno; - debugs(50, DBG_IMPORTANT, MYNAME << "FD " << fd << ", SIZE " << size << ": " << xstrerr(xerrno)); - } + Comm::SetSocketOption(fd, SOL_SOCKET, SO_RCVBUF, size, ToSBuf("SO_RCVBUF to ", size, " bytes")); + Comm::SetSocketOption(fd, SOL_SOCKET, SO_SNDBUF, size, ToSBuf("SO_SNDBUF to ", size, " bytes")); +#if defined(TCP_WINDOW_CLAMP) + Comm::SetSocketOption(fd, SOL_TCP, TCP_WINDOW_CLAMP, size, ToSBuf("TCP_WINDOW_CLAMP to ", size, " bytes")); #endif } @@ -1118,20 +1093,13 @@ commSetCloseOnExec(int fd) #endif } -#ifdef TCP_NODELAY +#if defined(TCP_NODELAY) static void commSetTcpNoDelay(int fd) { - int on = 1; - - if (setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, (char *) &on, sizeof(on)) < 0) { - int xerrno = errno; - debugs(50, DBG_IMPORTANT, MYNAME << "FD " << fd << ": " << xstrerr(xerrno)); - } - + Comm::SetBooleanSocketOption(fd, IPPROTO_TCP, TCP_NODELAY, true, SBuf("TCP_NODELAY")); fd_table[fd].flags.nodelay = true; } - #endif void diff --git a/src/comm/ConnOpener.cc b/src/comm/ConnOpener.cc index 25aad6bb4b7..838c781e08b 100644 --- a/src/comm/ConnOpener.cc +++ b/src/comm/ConnOpener.cc @@ -291,12 +291,10 @@ Comm::ConnOpener::createFd() } // Set TOS if needed. - if (conn_->tos && - Ip::Qos::setSockTos(temporaryFd_, conn_->tos, conn_->remote.isIPv4() ? AF_INET : AF_INET6) < 0) + if (conn_->tos && !Ip::Qos::setSockTos(temporaryFd_, conn_->tos, conn_->remote.isIPv4() ? AF_INET : AF_INET6)) conn_->tos = 0; #if SO_MARK - if (conn_->nfmark && - Ip::Qos::setSockNfmark(temporaryFd_, conn_->nfmark) < 0) + if (conn_->nfmark && !Ip::Qos::setSockNfmark(temporaryFd_, conn_->nfmark)) conn_->nfmark = 0; #endif diff --git a/src/comm/Makefile.am b/src/comm/Makefile.am index 7d0d647f8e2..d49e27e9a6e 100644 --- a/src/comm/Makefile.am +++ b/src/comm/Makefile.am @@ -43,4 +43,6 @@ libcomm_la_SOURCES = \ # a bare-bones implementation of few Comm APIs sufficient for helpers use libminimal_la_SOURCES = \ + Tcp.cc \ + Tcp.h \ minimal.cc diff --git a/src/comm/Tcp.cc b/src/comm/Tcp.cc index d42c2908213..aef0ab47ba7 100644 --- a/src/comm/Tcp.cc +++ b/src/comm/Tcp.cc @@ -11,6 +11,7 @@ #include "squid.h" #include "comm/Tcp.h" #include "debug/Stream.h" +#include "sbuf/Stream.h" #if HAVE_NETINET_TCP_H #include @@ -23,28 +24,11 @@ #endif #include -/// setsockopt(2) wrapper -template -static bool -SetSocketOption(const int fd, const int level, const int optName, const Option &optValue) -{ - static_assert(std::is_trivially_copyable