Rework the opening of UDP sockets for IPv4 and IPv6 - #3893
Conversation
| // faulty router gets stuck and confused by a particular port (like | ||
| // the starting port). Might work around frustrating "cannot connect" | ||
| // problems (#568) | ||
| const quint16 startingPortNumber = iPortNumber + rand() % NUM_SOCKET_PORTS_TO_TRY; |
There was a problem hiding this comment.
Since this was the first call to rand() after program startup, and rand() always starts from the same seed, this was not providing any randomness at all. The same start port was chosen on each invocation, so this wouldn't actually address #568 as claimed. Verified on Linux and Mac.
|
|
||
| quint16 iClientPortIncrement = 0; | ||
| bSuccess = false; // initialization for while loop | ||
| const quint32 startingPortNumber = static_cast<quint32> ( iPortNumber ) + QRandomGenerator::global()->bounded ( NUM_SOCKET_PORTS_TO_TRY ); |
There was a problem hiding this comment.
This provides a truly random starting port.
There was a problem hiding this comment.
Doesn't C++ have something inbuilt? I'd prefer something that not depends on Qt.
This corrects logic errors for bind() failure handling found by AI. bIPv6Available is now only set if bind() succeeds for IPv6.
Previously, rand() was used and is always seeded with 1, providing no randomness at program start.
c56534e to
0d7b521
Compare
| if ( !bSuccess ) | ||
| { | ||
| // we cannot bind socket, throw error | ||
| throw CGenErr ( "Cannot bind the socket (maybe " |
There was a problem hiding this comment.
Might be worth expanding this to explain more about what failed as there are more possible causes now. (Not due to this PR -- I've just read it to get more context and realised it's a bit weak.)
There was a problem hiding this comment.
Actually, bSuccess only now applies to IPv4. If we fail to create and bind IPv4, bSuccess will be false and we throw the exception. If we succeed with IPv4 but fail with IPv6, it's just logged with a message, but bSuccess is still true. So there isn't really any more info to pass to the exception apart from what is already there.
softins
changed the title
3 participants
Short description of changes
Reworks the socket opening logic introduced in #3774, which had potential shortcomings identified by AI.
Not for backporting, as #3774 was not also.
CHANGELOG: Client/Server: reworked and improved UDP socket opening logic
Context: Fixes an issue?
Fixes #3885
Does this change need documentation? What needs to be documented and how?
No, bug fixes only
Status of this Pull Request
Ready for review
What is missing until this pull request can be merged?
Checking that it satisfies CoPilot AI shortcomings that were flagged in #3885 for socket.cpp
Checklist