This is a piece of #21878, chopped off to ease review.
Listening sockets in CConnman::vhListenSocket are always valid
(underlying file descriptor is not INVALID_SOCKET).
Listening sockets in `CConnman::vhListenSocket` are always valid
(underlying file descriptor is not `INVALID_SOCKET`).
Code-review ACK 6c9ee92ffe390064881ef5bf47e38e345d78a2a1 🔌
Verified that it's currently not possible that CConnman::vhListenSocket ever contains invalid sockets. The only place where an element gets added to the vector is in the method CConnman::BindListenPort:
https://github.com/bitcoin/bitcoin/blob/111c3e06b35b7867f2e0f740e988f648ac6c325d/src/net.cpp#L2394
which would already return earlier (directly after CreateSock(...)) if the socket contained in the sock wrapper was not valid. Also built and ran unit tests locally with success.
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--174a7506f384e20aa4161008e828411d-->
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
utACK.
If INVALID_SOCKET can get into recv_set, we have UB earlier anyway. Therefore, .count should always be 0 even if somehow listen_socket.socket were INVALID_SOCKET.
If this is safe, is there a reason why it can't be removed here as well?
src/net.cpp=void CConnman::StopNodes()
src/net.cpp-{
...
src/net.cpp-
src/net.cpp- // Close listening sockets.
src/net.cpp: for (ListenSocket& hListenSocket : vhListenSocket) {
src/net.cpp- if (hListenSocket.socket != INVALID_SOCKET) {
@MarcoFalke, that's right - can be removed from here too and should have been part of this PR, I missed it.
I missed it because in #21878 that "close listening sockets" code becomes unnecessary and is removed:
- // Close listening sockets.
- for (ListenSocket& hListenSocket : vhListenSocket) {
- if (hListenSocket.socket != INVALID_SOCKET) {
- if (!CloseSocket(hListenSocket.socket)) {
- LogPrintf("CloseSocket(hListenSocket) failed with error %s\n", NetworkErrorString(WSAG
- }
- }
- }
This removal is also in #21879 which is chopped off from the big #21878 to ease review.
I will leave it as is to avoid further conflicts, hoping that #21879 will eventually make it in.