- Cleanup and safety tweaks for NetMesh and settings file IO

Author: x64-dev

Core/Libraries/Source/WWVegas/WWLib/mutex.cpp

@@ -95,8 +95,21 @@ CriticalSectionClass::CriticalSectionClass() : handle(nullptr), locked(false)
 	#ifdef _UNIX
 		//assert(0);
 	#else
-		handle=W3DNEWARRAY char[sizeof(CRITICAL_SECTION)];
-		InitializeCriticalSection((CRITICAL_SECTION*)handle);
+		// Allocate with proper alignment for CRITICAL_SECTION
+		// On 64-bit: requires 8-byte alignment, on 32-bit: requires 4-byte alignment
+		#if defined(_WIN64) || defined(__LP64__)
+			size_t alignment = 8;
+		#else
+			size_t alignment = 4;
+		#endif
+		
+		handle = _aligned_malloc(sizeof(CRITICAL_SECTION), alignment);
+		if (handle != nullptr) {
+			InitializeCriticalSection((CRITICAL_SECTION*)handle);
+		}
+		else {
+			WWASSERT(false); // Allocation failed
+		}
 	#endif
 }
 
@@ -105,9 +118,11 @@ CriticalSectionClass::~CriticalSectionClass()
 	#ifdef _UNIX
 		//assert(0);
 	#else
-		WWASSERT(!locked); // Can't delete locked mutex!
-		DeleteCriticalSection((CRITICAL_SECTION*)handle);
-		delete[] handle;
+		WWASSERT(!locked); // Can't delete locked critical section!
+		if (handle != nullptr) {
+			DeleteCriticalSection((CRITICAL_SECTION*)handle);
+			_aligned_free(handle);
+		}
 	#endif
 }
 

GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/NGMP_Helpers.cpp

@@ -150,6 +150,7 @@ std::vector<uint8_t> Base64Decode(const std::string& encodedData) {
 	uint8_t charArray4[4], charArray3[3];
 
 	while (inLen-- && (encodedData[in_] != '=') && isBase64(encodedData[in_])) {
+		if (i >= 4) break;
 		charArray4[i++] = encodedData[in_]; in_++;
 		if (i == 4) {
 			for (i = 0; i < 4; i++)

GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/NetworkMesh.cpp

@@ -753,9 +753,10 @@ QueuedGamePacket NetworkMesh::RecvGamePacket()
 
 int NetworkMesh::SendGamePacket(void* pBuffer, uint32_t totalDataSize, int64_t user_id)
 {
-	if (m_mapConnections.contains(user_id))
+	auto it = m_mapConnections.find(user_id);
+	if (it != m_mapConnections.end())
 	{
-		return m_mapConnections[user_id].SendGamePacket(pBuffer, totalDataSize);
+		return it->second.SendGamePacket(pBuffer, totalDataSize);
 	}
 
 	return -2;
@@ -765,21 +766,22 @@ int NetworkMesh::SendGamePacket(void* pBuffer, uint32_t totalDataSize, int64_t u
 void NetworkMesh::StartConnectionSignalling(int64_t remoteUserID, uint16_t preferredPort)
 {
 	// if we already have a connection to this use, drop it, having a single-direction connection will break signalling
-	if (m_mapConnections.find(remoteUserID) != m_mapConnections.end())
+	auto it = m_mapConnections.find(remoteUserID);
+	if (it != m_mapConnections.end())
 	{
-		if (m_mapConnections[remoteUserID].m_hSteamConnection != k_HSteamNetConnection_Invalid)
+		if (it->second.m_hSteamConnection != k_HSteamNetConnection_Invalid)
 		{
 			NetworkLog(ELogVerbosity::LOG_RELEASE, "[DC] Closing connection %lld, new connection is being negotiated", remoteUserID);
-			SteamNetworkingSockets()->CloseConnection(m_mapConnections[remoteUserID].m_hSteamConnection, 0, "Client Disconnecting Gracefully (new connection being negotiated)", false);
+			SteamNetworkingSockets()->CloseConnection(it->second.m_hSteamConnection, 0, "Client Disconnecting Gracefully (new connection being negotiated)", false);
 
 			if (TheNetwork != nullptr)
 			{
 				TheNetwork->GetConnectionManager()->disconnectPlayer(remoteUserID);
 			}
 		}
 
-		NetworkLog(ELogVerbosity::LOG_RELEASE, "[ERASE 3] Removing user %lld", m_mapConnections[remoteUserID].m_userID);
-		m_mapConnections.erase(remoteUserID);
+		NetworkLog(ELogVerbosity::LOG_RELEASE, "[ERASE 3] Removing user %lld", it->second.m_userID);
+		m_mapConnections.erase(it);
 	}
 
 	NGMP_OnlineServicesManager* pOnlineServicesMgr = NGMP_OnlineServicesManager::GetInstance();
@@ -887,14 +889,18 @@ void NetworkMesh::DisconnectUser(int64_t remoteUserID)
 
 		if (TheNGMPGame && !TheNGMPGame->isGameInProgress())
 		{
-			for (auto& kvPair : m_mapConnections)
+			for (auto it = m_mapConnections.begin(); it != m_mapConnections.end(); )
 			{
-				if (kvPair.second.m_userID == remoteUserID)
+				if (it->second.m_userID == remoteUserID)
 				{
-					NetworkLog(ELogVerbosity::LOG_RELEASE, "[ERASE] Removing user %lld", kvPair.second.m_userID);
-					m_mapConnections.erase(kvPair.first);
+					NetworkLog(ELogVerbosity::LOG_RELEASE, "[ERASE] Removing user %lld", it->second.m_userID);
+					it = m_mapConnections.erase(it);
 					break;
 				}
+				else
+				{
+					++it;
+				}
 			}
 		}
 	}

GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_Init.cpp

@@ -120,8 +120,10 @@ void NGMP_OnlineServicesManager::CaptureScreenshotToDisk()
 			{
 				// write to disk
 				FILE* pFile = fopen(strFilePath.c_str(), "wb");
-				fwrite(vecBuffer.data(), sizeof(uint8_t), vecBuffer.size(), pFile);
-				fclose(pFile);
+				if (pFile != nullptr) {
+					fwrite(vecBuffer.data(), sizeof(uint8_t), vecBuffer.size(), pFile);
+					fclose(pFile);
+				}
 			}
 		});
 }
@@ -426,8 +428,10 @@ void NGMP_OnlineServicesManager::ContinueUpdate()
 					}
 
 					FILE* pFile = fopen(strOutPath.c_str(), "wb");
-					fwrite(vecBuffer.data(), sizeof(uint8_t), bufSize, pFile);
-					fclose(pFile);
+					if (pFile != nullptr) {
+						fwrite(vecBuffer.data(), sizeof(uint8_t), bufSize, pFile);
+						fclose(pFile);
+					}
 
 					// call continue update again, thisll check if we're done or have more work to do
 					ContinueUpdate();