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.
LoneGazebo · Jul 3, 2024 · 0402069 · 0402069
1 parent 8ce49c8
commit 0402069
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 25 deletions.
diff --git a/CvGameCoreDLL_Expansion2/CvPreGame.cpp b/CvGameCoreDLL_Expansion2/CvPreGame.cpp
@@ -491,7 +491,8 @@ int calcActiveSlotCount(const std::vector<SlotStatus>& 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<CivilizationTypes>	dummyCivilizations;
 	std::vector<CvString> dummyNicknames;
 	std::vector<TeamTypes> dummyTeamTypes;
-	std::vector<SlotStatus> slotStatus;
-	std::vector<SlotClaim> slotClaims;
+	std::vector<SlotStatus> slotStatus(MAX_PLAYERS);
+	std::vector<SlotClaim> slotClaims(MAX_PLAYERS);
 	std::vector<HandicapTypes> dummyHandicapTypes;
 	std::vector<CvString> civilizationKeys;
 	std::vector<CvString> 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
@@ -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<int, CvPlot*>::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<int, CvPlot*> aNumFriendlies;
-			for (multimap<int, CvPlot*>::iterator it = aNumEnemies.begin(); it != aNumEnemies.end(); ++it)
+			// remove plots that have more enemies than the minimum
+			multimap<int, CvPlot*>::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<int, CvPlot*> aNumFriendlies;
+				for (multimap<int, CvPlot*>::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<int, CvPlot*>::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<int, CvPlot*>::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
@@ -155,6 +155,10 @@ FObjectPool<T>::FObjectPool( const FObjectPool<T>& source )
 template<class T> 
 FObjectPool<T>& FObjectPool<T>::operator=( const FObjectPool<T>& source )
 {
+	// Self-assignment check
+	if (this == &source)
+		return *this;
+
 	// Lock for thread safety
 	Lock();
 	source.Lock();
@@ -180,8 +184,8 @@ FObjectPool<T>& FObjectPool<T>::operator=( const FObjectPool<T>& 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<T>& FObjectPool<T>::operator=( const FObjectPool<T>& source )
 
 	source.Unlock();
 	Unlock();
+
+	return *this;
 }
 
 //---------------------------------------------------------------------------------------