Skip to content

Commit eaf2c46

Browse files
committed
Merge bitcoin/bitcoin#33378: Remove unnecessary casts when calling socket operations
67f632b net: remove unnecessary casts in socket operations (Matthew Zipkin) Pull request description: During review of bitcoin/bitcoin#32747 several casting operations were questioned in existing code that had been copied or moved. That lead me to find a few other similar casts in the codebase. It turns out that since the `Sock` class wraps syscalls with its own internal casting (see bitcoin/bitcoin#24357 and bitcoin/bitcoin#20788 written in 2020-2022) we no longer need to cast the arguments when calling these functions. The original argument-casts are old and were cleaned up a bit in bitcoin/bitcoin#12855 written in 2018. The casting is only needed for windows compatibility, where those syscalls require a data argument to be of type `char*` specifically: https://learn.microsoft.com/en-us/windows/win32/api/winsock/nf-winsock-getsockopt ``` int getsockopt( [in] SOCKET s, [in] int level, [in] int optname, [out] char *optval, [in, out] int *optlen ); ``` but on POSIX the argument is `void*`: https://www.man7.org/linux/man-pages/man2/getsockopt.2.html ``` int getsockopt(socklen *restrict optlen; int sockfd, int level, int optname, void optval[_Nullable restrict *optlen], socklen_t *restrict optlen); ``` ACKs for top commit: Raimo33: ACK 67f632b achow101: ACK 67f632b hodlinator: ACK 67f632b vasild: ACK 67f632b davidgumberg: ACK bitcoin/bitcoin@67f632b Tree-SHA512: c326d7242698b8d4d019f630fb6281398da2773c4e5aad1e3bba093a012c2119ad8815f42bd009e61a9a90db9b8e6ed5c75174aac059c9df83dd3aa5618a9ba6
2 parents 5aec516 + 67f632b commit eaf2c46

File tree

5 files changed

+8
-16
lines changed

5 files changed

+8
-16
lines changed

src/compat/compat.h

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -75,14 +75,6 @@ typedef unsigned int SOCKET;
7575
typedef SSIZE_T ssize_t;
7676
#endif
7777

78-
// The type of the option value passed to getsockopt & setsockopt
79-
// differs between Windows and non-Windows.
80-
#ifndef WIN32
81-
typedef void* sockopt_arg_type;
82-
#else
83-
typedef char* sockopt_arg_type;
84-
#endif
85-
8678
#ifdef WIN32
8779
// Export main() and ensure working ASLR when using mingw-w64.
8880
// Exporting a symbol will prevent the linker from stripping

src/httpserver.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,7 @@ static bool HTTPBindAddresses(struct evhttp* http)
401401
// Set the no-delay option (disable Nagle's algorithm) on the TCP socket.
402402
evutil_socket_t fd = evhttp_bound_socket_get_fd(bind_handle);
403403
int one = 1;
404-
if (setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, (sockopt_arg_type)&one, sizeof(one)) == SOCKET_ERROR) {
404+
if (setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, reinterpret_cast<char*>(&one), sizeof(one)) == SOCKET_ERROR) {
405405
LogInfo("WARNING: Unable to set TCP_NODELAY on RPC server socket, continuing anyway\n");
406406
}
407407
boundSockets.push_back(bind_handle);

src/net.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1638,7 +1638,7 @@ std::pair<size_t, bool> CConnman::SocketSendData(CNode& node) const
16381638
flags |= MSG_MORE;
16391639
}
16401640
#endif
1641-
nBytes = node.m_sock->Send(reinterpret_cast<const char*>(data.data()), data.size(), flags);
1641+
nBytes = node.m_sock->Send(data.data(), data.size(), flags);
16421642
}
16431643
if (nBytes > 0) {
16441644
node.m_last_send = GetTime<std::chrono::seconds>();
@@ -3139,7 +3139,7 @@ bool CConnman::BindListenPort(const CService& addrBind, bilingual_str& strError,
31393139

31403140
// Allow binding if the port is still in TIME_WAIT state after
31413141
// the program was closed and restarted.
3142-
if (sock->SetSockOpt(SOL_SOCKET, SO_REUSEADDR, (sockopt_arg_type)&nOne, sizeof(int)) == SOCKET_ERROR) {
3142+
if (sock->SetSockOpt(SOL_SOCKET, SO_REUSEADDR, &nOne, sizeof(int)) == SOCKET_ERROR) {
31433143
strError = Untranslated(strprintf("Error setting SO_REUSEADDR on socket: %s, continuing anyway", NetworkErrorString(WSAGetLastError())));
31443144
LogPrintf("%s\n", strError.original);
31453145
}
@@ -3148,14 +3148,14 @@ bool CConnman::BindListenPort(const CService& addrBind, bilingual_str& strError,
31483148
// and enable it by default or not. Try to enable it, if possible.
31493149
if (addrBind.IsIPv6()) {
31503150
#ifdef IPV6_V6ONLY
3151-
if (sock->SetSockOpt(IPPROTO_IPV6, IPV6_V6ONLY, (sockopt_arg_type)&nOne, sizeof(int)) == SOCKET_ERROR) {
3151+
if (sock->SetSockOpt(IPPROTO_IPV6, IPV6_V6ONLY, &nOne, sizeof(int)) == SOCKET_ERROR) {
31523152
strError = Untranslated(strprintf("Error setting IPV6_V6ONLY on socket: %s, continuing anyway", NetworkErrorString(WSAGetLastError())));
31533153
LogPrintf("%s\n", strError.original);
31543154
}
31553155
#endif
31563156
#ifdef WIN32
31573157
int nProtLevel = PROTECTION_LEVEL_UNRESTRICTED;
3158-
if (sock->SetSockOpt(IPPROTO_IPV6, IPV6_PROTECTION_LEVEL, (const char*)&nProtLevel, sizeof(int)) == SOCKET_ERROR) {
3158+
if (sock->SetSockOpt(IPPROTO_IPV6, IPV6_PROTECTION_LEVEL, &nProtLevel, sizeof(int)) == SOCKET_ERROR) {
31593159
strError = Untranslated(strprintf("Error setting IPV6_PROTECTION_LEVEL on socket: %s, continuing anyway", NetworkErrorString(WSAGetLastError())));
31603160
LogPrintf("%s\n", strError.original);
31613161
}

src/netbase.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -551,7 +551,7 @@ std::unique_ptr<Sock> CreateSockOS(int domain, int type, int protocol)
551551
int set = 1;
552552
// Set the no-sigpipe option on the socket for BSD systems, other UNIXes
553553
// should use the MSG_NOSIGNAL flag for every send.
554-
if (sock->SetSockOpt(SOL_SOCKET, SO_NOSIGPIPE, (void*)&set, sizeof(int)) == SOCKET_ERROR) {
554+
if (sock->SetSockOpt(SOL_SOCKET, SO_NOSIGPIPE, &set, sizeof(int)) == SOCKET_ERROR) {
555555
LogPrintf("Error setting SO_NOSIGPIPE on socket: %s, continuing anyway\n",
556556
NetworkErrorString(WSAGetLastError()));
557557
}
@@ -620,7 +620,7 @@ static bool ConnectToSocket(const Sock& sock, struct sockaddr* sockaddr, socklen
620620
// sockerr here.
621621
int sockerr;
622622
socklen_t sockerr_len = sizeof(sockerr);
623-
if (sock.GetSockOpt(SOL_SOCKET, SO_ERROR, (sockopt_arg_type)&sockerr, &sockerr_len) ==
623+
if (sock.GetSockOpt(SOL_SOCKET, SO_ERROR, &sockerr, &sockerr_len) ==
624624
SOCKET_ERROR) {
625625
LogPrintf("getsockopt() for %s failed: %s\n", dest_str, NetworkErrorString(WSAGetLastError()));
626626
return false;

src/test/sock_tests.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ static bool SocketIsClosed(const SOCKET& s)
2424
// wrongly pretend that the socket is not closed.
2525
int type;
2626
socklen_t len = sizeof(type);
27-
return getsockopt(s, SOL_SOCKET, SO_TYPE, (sockopt_arg_type)&type, &len) == SOCKET_ERROR;
27+
return getsockopt(s, SOL_SOCKET, SO_TYPE, reinterpret_cast<char*>(&type), &len) == SOCKET_ERROR;
2828
}
2929

3030
static SOCKET CreateSocket()

0 commit comments

Comments
 (0)