fix: exception handling, switch/if chain and others (opentibiabr#798)

This fixes exception handling, switch/if chain, uninitialized variable, combat null pointers, and signed/unsigned comparison issues. Fixes several issues identified in the codebase. Firstly, it ensures that proper exception handling is in place to prevent unexpected termination of the program. Secondly, it eliminates the redundancy in switch and if chains by making sure that each branch has a unique implementation. Thirdly, it eliminates potential bugs caused by uninitialized variables by initializing all non-class type fields. Fourthly, it replaces dangerous comparison between signed and unsigned integers with std::cmp_* functions from C++20 to prevent counterintuitive results and lossy integer conversion. This improves the overall stability and security of the program.
LeoTKBR · Jan 20, 2023 · fad589e · fad589e
1 parent 4b911bc
commit fad589e
Show file tree

Hide file tree

Showing 6 changed files with 15 additions and 31 deletions.
diff --git a/src/creatures/combat/combat.cpp b/src/creatures/combat/combat.cpp
@@ -1036,7 +1036,7 @@ void Combat::doCombatMana(Creature* caster, Creature* target, CombatDamage& dama
 	}
 
 	if (canCombat) {
-		if (caster && params.distanceEffect != CONST_ANI_NONE) {
+		if (caster && target && params.distanceEffect != CONST_ANI_NONE) {
 			addDistanceEffect(caster, caster->getPosition(), target->getPosition(), params.distanceEffect);
 		}
 
@@ -1076,7 +1076,7 @@ void Combat::doCombatCondition(Creature* caster, Creature* target, const CombatP
 	}
 
 	if (canCombat) {
-		if (caster && params.distanceEffect != CONST_ANI_NONE) {
+		if (caster && target && params.distanceEffect != CONST_ANI_NONE) {
 			addDistanceEffect(caster, caster->getPosition(), target->getPosition(), params.distanceEffect);
 		}
 

diff --git a/src/database/database.h b/src/database/database.h
@@ -208,7 +208,11 @@ class DBTransaction
 
 		~DBTransaction() {
 			if (state == STATE_START) {
-				Database::getInstance().rollback();
+				try {
+					Database::getInstance().rollback();
+				} catch (std::exception &exception) {
+					SPDLOG_ERROR("{} - Catch exception error: {}", __FUNCTION__, exception.what());
+				}
 			}
 		}
 

diff --git a/src/game/movement/position.h b/src/game/movement/position.h
@@ -68,31 +68,7 @@ struct Position
 	uint8_t z = 0;
 
 	bool operator<(const Position& p) const {
-		if (z < p.z) {
-			return true;
-		}
-
-		if (z > p.z) {
-			return false;
-		}
-
-		if (y < p.y) {
-			return true;
-		}
-
-		if (y > p.y) {
-			return false;
-		}
-
-		if (x < p.x) {
-			return true;
-		}
-
-		if (x > p.x) {
-			return false;
-		}
-
-		return false;
+		return (z < p.z) || (z == p.z && y < p.y) || (z == p.z && y == p.y && x < p.x);
 	}
 
 	bool operator>(const Position& p) const {

diff --git a/src/lua/callbacks/creaturecallback.h b/src/lua/callbacks/creaturecallback.h
@@ -62,7 +62,7 @@ class CreatureCallback {
 		LuaScriptInterface* scriptInterface;
 		Creature* targetCreature;
 		uint32_t params = 0;
-		lua_State* L;
+		lua_State* L = nullptr;
 };
 
 #endif  // SRC_LUA_CALLBACKS_CREATURECALLBACK_H_
diff --git a/src/server/network/protocol/protocol.cpp b/src/server/network/protocol/protocol.cpp
@@ -184,7 +184,7 @@ bool Protocol::XTEA_decrypt(NetworkMessage& msg) const
 	}
 
 	uint16_t innerLength = msg.get<uint16_t>();
-	if (innerLength > msgLength - 2) {
+	if (std::cmp_greater(innerLength, msgLength - 2)) {
 		return false;
 	}
 

diff --git a/src/server/server.cpp b/src/server/server.cpp
@@ -19,7 +19,11 @@ Ban g_bans;
 
 ServiceManager::~ServiceManager()
 {
-	stop();
+	try {
+		stop();
+	} catch (std::exception &exception) {
+		SPDLOG_ERROR("{} - Catch exception error: {}", __FUNCTION__, exception.what());
+	}
 }
 
 void ServiceManager::die()