Maintenance: use SetSocketOption() template

src/Makefile.am

@@ -912,13 +912,16 @@ nodist_tests_testURL_SOURCES = \
 	tests/stub_HelperChildConfig.cc \
 	tests/stub_HttpHeader.cc \
 	tests/stub_HttpRequest.cc \
+	tests/stub_MemBuf.cc \
 	tests/stub_StatHist.cc \
 	String.cc \
 	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_testURL_LDADD = \
@@ -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=\

src/client_side.cc

@@ -75,6 +75,7 @@
 #include "comm/Connection.h"
 #include "comm/Loops.h"
 #include "comm/Read.h"
+#include "comm/SocketOptions.h"
 #include "comm/TcpAcceptor.h"
 #include "comm/Write.h"
 #include "CommCalls.h"
@@ -2134,10 +2135,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));
-        }
+        (void)Comm::SetSocketOption(clientConnection->fd, SOL_IP, IP_MTU_DISCOVER, i, ToSBuf("IP_MTU_DISCOVER disabled for client ", clientConnection));
 #else
         static bool reported = false;
 

src/client_side_request.cc

@@ -1708,17 +1708,16 @@ ClientHttpRequest::doCallouts()
 
         if (!calloutContext->toClientMarkingDone) {
             calloutContext->toClientMarkingDone = true;
-            tos_t tos = aclMapTOS(Ip::Qos::TheConfig.tosToClient, &ch);
-            if (tos)
-                Ip::Qos::setSockTos(getConn()->clientConnection, tos);
+            if (tos_t tos = aclMapTOS(Ip::Qos::TheConfig.tosToClient, &ch))
+                (void)Ip::Qos::setSockTos(getConn()->clientConnection, tos);
 
             const auto packetMark = aclFindNfMarkConfig(Ip::Qos::TheConfig.nfmarkToClient, &ch);
             if (!packetMark.isEmpty())
-                Ip::Qos::setSockNfmark(getConn()->clientConnection, packetMark.mark);
+                (void)Ip::Qos::setSockNfmark(getConn()->clientConnection, packetMark.mark);
 
             const auto connmark = aclFindNfMarkConfig(Ip::Qos::TheConfig.nfConnmarkToClient, &ch);
             if (!connmark.isEmpty())
-                Ip::Qos::setNfConnmark(getConn()->clientConnection, Ip::Qos::dirAccepted, connmark);
+                (void)Ip::Qos::setNfConnmark(getConn()->clientConnection, Ip::Qos::dirAccepted, connmark);
         }
     }
 

src/clients/FtpGateway.cc

@@ -16,6 +16,7 @@
 #include "comm.h"
 #include "comm/ConnOpener.h"
 #include "comm/Read.h"
+#include "comm/SocketOptions.h"
 #include "comm/TcpAcceptor.h"
 #include "CommCalls.h"
 #include "compat/strtoll.h"
@@ -1774,15 +1775,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 {

src/comm.cc

@@ -18,6 +18,7 @@
 #include "comm/IoCallback.h"
 #include "comm/Loops.h"
 #include "comm/Read.h"
+#include "comm/SocketOptions.h"
 #include "comm/TcpAcceptor.h"
 #include "comm/Write.h"
 #include "compat/cmsg.h"
@@ -210,11 +211,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<char*>(&flag), sizeof(flag)) < 0) {
-        const auto savedErrno = errno;
-        debugs(50, DBG_IMPORTANT, "ERROR: setsockopt(IP_BIND_ADDRESS_NO_PORT) failure: " << xstrerr(savedErrno));
-    }
+    (void)Comm::SetBooleanSocketOption(fd, IPPROTO_IP, IP_BIND_ADDRESS_NO_PORT, true, SBuf("IP_BIND_ADDRESS_NO_PORT"));
 #else
     (void)fd;
 #endif
@@ -291,16 +288,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)
+    (void)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 +309,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 +332,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 +416,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 +498,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<char*>(&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 +762,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<char*>(&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));
-    }
+    (void)Comm::SetSocketOption(fd, SOL_SOCKET, SO_LINGER, l, ToSBuf("SO_LINGER (0 seconds) ", (l.l_onoff?"enabled":"disabled")));
 }
 
 /**
@@ -1011,29 +1000,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));
-    }
+    (void)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));
-    }
+    (void)Comm::SetSocketOption(fd, SOL_SOCKET, SO_RCVBUF, size, ToSBuf("SO_RCVBUF to ", size, " bytes"));
+    (void)Comm::SetSocketOption(fd, SOL_SOCKET, SO_SNDBUF, size, ToSBuf("SO_SNDBUF to ", size, " bytes"));
+#if defined(TCP_WINDOW_CLAMP)
+    (void)Comm::SetSocketOption(fd, SOL_TCP, TCP_WINDOW_CLAMP, size, ToSBuf("TCP_WINDOW_CLAMP to ", size, " bytes"));
 #endif
 }
 
@@ -1118,20 +1094,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));
-    }
-
+    (void)Comm::SetBooleanSocketOption(fd, IPPROTO_TCP, TCP_NODELAY, true, SBuf("TCP_NODELAY"));
     fd_table[fd].flags.nodelay = true;
 }
-
 #endif
 
 void

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
 

src/comm/Makefile.am

@@ -32,6 +32,8 @@ libcomm_la_SOURCES = \
 	ModSelect.cc \
 	Read.cc \
 	Read.h \
+	SocketOptions.cc \
+	SocketOptions.h \
 	Tcp.cc \
 	Tcp.h \
 	TcpAcceptor.cc \
@@ -43,4 +45,6 @@ libcomm_la_SOURCES = \
 
 # a bare-bones implementation of few Comm APIs sufficient for helpers use
 libminimal_la_SOURCES = \
+	SocketOptions.cc \
+	SocketOptions.h \
 	minimal.cc

src/comm/SocketOptions.h

@@ -0,0 +1,44 @@
+/*
+ * Copyright (C) 1996-2025 The Squid Software Foundation and contributors
+ *
+ * Squid software is distributed under GPLv2+ license and includes
+ * contributions from numerous individuals and organizations.
+ * Please see the COPYING and CONTRIBUTORS files for details.
+ */
+
+#ifndef SQUID_SRC_COMM_SOCKETOPTIONS_H
+#define SQUID_SRC_COMM_SOCKETOPTIONS_H
+
+#include "debug/Stream.h"
+#include "sbuf/SBuf.h"
+
+#if HAVE_SYS_SOCKET_H
+#include <sys/socket.h>
+#endif
+
+namespace Comm
+{
+
+/// setsockopt(2) wrapper
+template <typename Option>
+bool
+SetSocketOption(const int fd, const int level, const int optName, const Option &optValue, const SBuf &description)
+{
+    static_assert(std::is_trivially_copyable<Option>::value, "setsockopt() expects POD-like options");
+    static_assert(!std::is_same<Option, bool>::value, "setsockopt() uses int to represent boolean options");
+    if (setsockopt(fd, level, optName, reinterpret_cast<const char *>(&optValue), sizeof(optValue)) < 0) {
+        const auto xerrno = errno;
+        debugs(5, DBG_IMPORTANT, "ERROR: setsockopt(2) failure on FD " << fd << " : " << xstrerr(xerrno)
+               << Debug::Extra << "setting " << description);
+        // TODO: Generalize to throw on errors when some callers need that.
+        return false;
+    }
+    return true;
+}
+
+/// setsockopt(2) wrapper for setting typical on/off options
+bool SetBooleanSocketOption(const int fd, const int level, const int optName, const bool enable, const SBuf &description);
+
+} // namespace Comm
+
+#endif /* SQUID_SRC_COMM_SOCKETOPTIONS_H */