From 0402069300d9e18c8b67306dcd792cbca084ef57 Mon Sep 17 00:00:00 2001 From: JohnsterID <69278611+JohnsterID@users.noreply.github.com> Date: Wed, 3 Jul 2024 14:22:47 +0930 Subject: [PATCH] cppcheck (#11021) * operatorEqToSelf Id: operatorEqToSelf CWE: 398 'operator=' should check for assignment to self to ensure that each block of dynamically allocated memory is owned and managed by only one instance of the class. * containerOutOfBounds Id: containerOutOfBounds CWE: 398 Out of bounds access in expression 'slotStatus[i]' because 'slotStatus' is empty. * containerOutOfBounds Id: containerOutOfBounds CWE: 398 Either the condition 'itr->second<=s_GameOptions.size()' is redundant or 'itr->second' can have the value s_GameOptions.size(). Expression 's_GameOptions[itr->second]' causes access out of bounds. Id: containerOutOfBounds CWE: 398 Assuming that condition 'itr->second<=s_GameOptions.size()' is not redundant * containerOutOfBounds Id: containerOutOfBounds CWE: 398 Out of bounds access in expression 'aNumEnemies.begin()' because 'aNumEnemies' is empty. Id: containerOutOfBounds CWE: 398 Out of bounds access in expression 'aNumFriendlies.rbegin()' because 'aNumFriendlies' is empty. --- CvGameCoreDLL_Expansion2/CvPreGame.cpp | 9 +++-- CvGameCoreDLL_Expansion2/CvUnit.cpp | 46 +++++++++++++---------- FirePlace/include/FireWorks/FObjectPool.h | 10 ++++- 3 files changed, 40 insertions(+), 25 deletions(-) diff --git a/CvGameCoreDLL_Expansion2/CvPreGame.cpp b/CvGameCoreDLL_Expansion2/CvPreGame.cpp index e0ef4c736e..dfe3ec452f 100644 --- a/CvGameCoreDLL_Expansion2/CvPreGame.cpp +++ b/CvGameCoreDLL_Expansion2/CvPreGame.cpp @@ -491,7 +491,8 @@ int calcActiveSlotCount(const std::vector& slotStatus, const std::ve { int iCount = 0; int i = 0; - for(i = 0; i < MAX_PLAYERS; ++i) + int maxPlayers = std::min(slotStatus.size(), slotClaims.size()); + for(i = 0; i < maxPlayers; ++i) { SlotStatus eStatus = slotStatus[i]; SlotClaim eClaim = slotClaims[i]; @@ -564,8 +565,8 @@ int readActiveSlotCountFromSaveGame(FDataStream& loadFrom, bool bReadVersion) std::vector dummyCivilizations; std::vector dummyNicknames; std::vector dummyTeamTypes; - std::vector slotStatus; - std::vector slotClaims; + std::vector slotStatus(MAX_PLAYERS); + std::vector slotClaims(MAX_PLAYERS); std::vector dummyHandicapTypes; std::vector civilizationKeys; std::vector leaderKeys; @@ -966,7 +967,7 @@ bool GetGameOption(GameOptionTypes eOption, int& iValue) HashToOptionMap::const_iterator itr = s_GameOptionsHash.find((uint)eOption); if(itr != s_GameOptionsHash.end()) { - if(itr->second <= s_GameOptions.size()) + if(itr->second < s_GameOptions.size()) { iValue = s_GameOptions[itr->second].GetValue(); return true; diff --git a/CvGameCoreDLL_Expansion2/CvUnit.cpp b/CvGameCoreDLL_Expansion2/CvUnit.cpp index 8a6548f481..6fc99d6351 100644 --- a/CvGameCoreDLL_Expansion2/CvUnit.cpp +++ b/CvGameCoreDLL_Expansion2/CvUnit.cpp @@ -30709,29 +30709,37 @@ bool CvUnit::DoFallBack(const CvUnit& attacker, bool bWithdraw, bool bCaptured) { aNumEnemies.insert(make_pair((*it)->GetNumEnemyUnitsAdjacent(getTeam(), getDomainType()), (*it))); } - // remove plots that have more enemies than the minimum - multimap::iterator itRemove = aNumEnemies.lower_bound(aNumEnemies.begin()->first + 1); - aNumEnemies.erase(itRemove, aNumEnemies.end()); - if (aNumEnemies.size() == 1) - pDestPlot = aNumEnemies.begin()->second; - else + if (!aNumEnemies.empty()) { - // if there is still more than one possible plot, select the plot with the highest number of adjacent friendly units - multimap aNumFriendlies; - for (multimap::iterator it = aNumEnemies.begin(); it != aNumEnemies.end(); ++it) + // remove plots that have more enemies than the minimum + multimap::iterator itRemove = aNumEnemies.lower_bound(aNumEnemies.begin()->first + 1); + aNumEnemies.erase(itRemove, aNumEnemies.end()); + + if (aNumEnemies.size() == 1) + pDestPlot = aNumEnemies.begin()->second; + else { - aNumFriendlies.insert(make_pair(it->second->GetNumFriendlyUnitsAdjacent(getTeam(), getDomainType(), true, this), it->second)); - } - // remove plots that have fewer friendlies than the maximum - itRemove = aNumFriendlies.lower_bound(aNumFriendlies.rbegin()->first); // highest key is the first key in reverse order - aNumFriendlies.erase(aNumFriendlies.begin(), itRemove); + // if there is still more than one possible plot, select the plot with the highest number of adjacent friendly units + multimap aNumFriendlies; + for (multimap::iterator it = aNumEnemies.begin(); it != aNumEnemies.end(); ++it) + { + aNumFriendlies.insert(make_pair(it->second->GetNumFriendlyUnitsAdjacent(getTeam(), getDomainType(), true, this), it->second)); + } + + if (!aNumFriendlies.empty()) + { + // remove plots that have fewer friendlies than the maximum + itRemove = aNumFriendlies.lower_bound(aNumFriendlies.rbegin()->first); // highest key is the first key in reverse order + aNumFriendlies.erase(aNumFriendlies.begin(), itRemove); - // make a random selection from the remaining plots - multimap::iterator itChosenPlot = aNumFriendlies.begin(); - if (aNumFriendlies.size() > 1) - advance(itChosenPlot, GC.getGame().urandLimitExclusive(aNumFriendlies.size(), CvSeeder(plot()->GetPseudoRandomSeed()).mix(GetID()).mix(getDamage()))); - pDestPlot = itChosenPlot->second; + // make a random selection from the remaining plots + multimap::iterator itChosenPlot = aNumFriendlies.begin(); + if (aNumFriendlies.size() > 1) + advance(itChosenPlot, GC.getGame().urandLimitExclusive(aNumFriendlies.size(), CvSeeder(plot()->GetPseudoRandomSeed()).mix(GetID()).mix(getDamage()))); + pDestPlot = itChosenPlot->second; + } + } } } diff --git a/FirePlace/include/FireWorks/FObjectPool.h b/FirePlace/include/FireWorks/FObjectPool.h index 3964587b9a..d1d3be0eea 100644 --- a/FirePlace/include/FireWorks/FObjectPool.h +++ b/FirePlace/include/FireWorks/FObjectPool.h @@ -155,6 +155,10 @@ FObjectPool::FObjectPool( const FObjectPool& source ) template FObjectPool& FObjectPool::operator=( const FObjectPool& source ) { + // Self-assignment check + if (this == &source) + return *this; + // Lock for thread safety Lock(); source.Lock(); @@ -180,8 +184,8 @@ FObjectPool& FObjectPool::operator=( const FObjectPool& source ) if (i >= m_uiSize) m_pStorage[i].pObject = FNEW( T(), c_eMPoolTypeContainer, 0 ); - // Set the data - m_pStorage[i].pObject = source.m_pStorage[i].pObject; + // Deep copy the data + *m_pStorage[i].pObject = *source.m_pStorage[i].pObject; m_pStorage[i].bFree = source.m_pStorage[i].bFree; } @@ -192,6 +196,8 @@ FObjectPool& FObjectPool::operator=( const FObjectPool& source ) source.Unlock(); Unlock(); + + return *this; } //---------------------------------------------------------------------------------------