fix(network): Initialize isDirectConnect field in LAN game announcements #1836
Conversation
| strlcpy(reply.GameInfo.options, gameOpts.str(), ARRAY_SIZE(reply.GameInfo.options)); | ||
| wcslcpy(reply.GameInfo.gameName, m_currentGame->getName().str(), ARRAY_SIZE(reply.GameInfo.gameName)); | ||
| reply.GameInfo.inProgress = m_currentGame->isGameInProgress(); | ||
| reply.GameInfo.isDirectConnect = m_currentGame->getIsDirectConnect(); |
There was a problem hiding this comment.
What issue did this cause then? Was Direct Connect game visible in Network Lobby when it should not have been?
There was a problem hiding this comment.
I don't have my PC to check this, I was just following the notes in the Issue. handleRequestLocations handler wasn't setting reply.GameInfo.isDirectConnect when building the game announcement message, so could we end up sending uninitialized data in that specific message struct? Does LANMessage reply also get auto zeroed?
There was a problem hiding this comment.
You need to test the consequence of this bug so we can properly describe what this change fixes.
| //Initializtions missing and needed | ||
| m_lastHeard = 0; | ||
| m_next = NULL; | ||
| m_isDirectConnect = false; |
There was a problem hiding this comment.
Are you certain this caused uninitialized memory use on runtime? LANGameInfo is allocated through Game Memory overloads, which means its memory is memset 0 before the constructor is called. So if you look at this object in debugger, then this field should 0 regardless.
There was a problem hiding this comment.
No, I think you're right here. Is it still good practice to do it explicitly here? If not, I can just drop that commit.
There was a problem hiding this comment.
This happens all over the code base. Class members are not explicitly zero initialized, which is a mistake in principle, but the memory allocator is forgiving (and very pessimistic) by zeroing every allocation.
|
In some cases Reproduction:
Maybe it only happens the first time that code executes after joining the lobby. |
3 participants
Summary
Fixes uninitialized memory bug in LAN game announcement messages when announcing games to the lobby.
TODO
Testing