Skip to content

Commit

Permalink
refactor: replace C-style array with std::array for buffer management (
Browse files Browse the repository at this point in the history
…#2914)

Refactored buffer management by replacing C-style arrays with std::array<uint8_t, NETWORKMESSAGE_MAXSIZE> in the NetworkMessage and OutputMessage classes. This change enhances memory safety by providing compile-time size checks and prevents buffer overflows through safer access methods. Transitioned all instances of memcpy to std::ranges::copy and replaced memset with std::fill for improved readability and type safety. Removed the use of reinterpret_cast for safer type conversions.

Introduced enhanced logging using std::source_location and added an optional function parameter to the addString and getString functions, providing better contextual information for both C++ and Lua scripts. Fixed a bug in the decodeHeader function and added corresponding unit tests to validate the fix.

This update addresses potential buffer overflow issues, improves code clarity, and enhances debugging capabilities. It also includes integration tests to ensure the correct behavior of buffer management under various conditions, such as large message handling and stress testing. The changes aim to modernize the codebase, reduce manual memory management risks, and align the implementation with modern C++ best practices.
  • Loading branch information
dudantas authored Sep 29, 2024
1 parent 09c16ff commit 04fa248
Show file tree
Hide file tree
Showing 17 changed files with 698 additions and 329 deletions.
14 changes: 7 additions & 7 deletions src/creatures/monsters/spawns/spawn_monster.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,12 @@ class SpawnMonster {

// moveable
SpawnMonster(SpawnMonster &&rhs) noexcept :
spawnMonsterMap(std::move(rhs.spawnMonsterMap)),
spawnedMonsterMap(std::move(rhs.spawnedMonsterMap)),
checkSpawnMonsterEvent(rhs.checkSpawnMonsterEvent), centerPos(rhs.centerPos), radius(rhs.radius), interval(rhs.interval) { }
spawnMonsterMap(std::move(rhs.spawnMonsterMap)),
centerPos(rhs.centerPos),
radius(rhs.radius),
interval(rhs.interval),
checkSpawnMonsterEvent(rhs.checkSpawnMonsterEvent) { }

SpawnMonster &operator=(SpawnMonster &&rhs) noexcept {
if (this != &rhs) {
Expand Down Expand Up @@ -77,15 +80,12 @@ class SpawnMonster {
void setMonsterVariant(const std::string &variant);

private:
// map of the spawned creatures
// The map of the spawned creatures
std::map<uint32_t, std::shared_ptr<Monster>> spawnedMonsterMap;

// map of creatures in the spawn
// The map of creatures in the spawn
std::map<uint32_t, spawnBlock_t> spawnMonsterMap;

Position centerPos;
int32_t radius;

uint32_t interval = 30000;
uint32_t checkSpawnMonsterEvent = 0;

Expand Down
2 changes: 1 addition & 1 deletion src/creatures/players/player.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5575,7 +5575,7 @@ int32_t Player::getMagicShieldCapacityPercent(bool useCharges) const {

double_t Player::getReflectPercent(CombatType_t combat, bool useCharges) const {
double_t result = reflectPercent[combatTypeToIndex(combat)];
for (const auto item : getEquippedItems()) {
for (const auto &item : getEquippedItems()) {
const ItemType &itemType = Item::items[item->getID()];
if (!itemType.abilities) {
continue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ int NetworkMessageFunctions::luaNetworkMessageAddString(lua_State* L) {
const std::string &function = getString(L, 3);
const auto &message = getUserdataShared<NetworkMessage>(L, 1);
if (message) {
message->addString(string, function);
message->addString(string, std::source_location::current(), function);
pushBoolean(L, true);
} else {
lua_pushnil(L);
Expand Down
22 changes: 18 additions & 4 deletions src/map/utils/astarnodes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,21 @@
#include "creatures/combat/combat.hpp"

AStarNodes::AStarNodes(uint32_t x, uint32_t y, int_fast32_t extraCost) :
openNodes(), nodes() {
#if defined(__AVX2__) || defined(__SSE2__)
nodesTable(), // 1. nodesTable
calculatedNodes(), // 2. calculatedNodes
nodes(), // 3. nodes
closedNodes(0), // 4. closedNodes
curNode(0), // 5. curNode
openNodes() // 6. openNodes
#else
nodes(), // 1. nodes
nodesTable(), // 2. nodesTable
closedNodes(0), // 3. closedNodes
curNode(0), // 4. curNode
openNodes() // 5. openNodes
#endif
{
#if defined(__AVX2__)
__m256i defaultCost = _mm256_set1_epi32(std::numeric_limits<int32_t>::max());
for (int32_t i = 0; i < MAX_NODES; i += 32) {
Expand Down Expand Up @@ -47,7 +61,7 @@ AStarNodes::AStarNodes(uint32_t x, uint32_t y, int_fast32_t extraCost) :
startNode.g = 0;
startNode.c = extraCost;
nodesTable[0] = (x << 16) | y;
#if defined(__SSE2__)
#if defined(__SSE2__) || defined(__AVX2__)
calculatedNodes[0] = 0;
#endif
}
Expand Down Expand Up @@ -265,9 +279,9 @@ int_fast32_t AStarNodes::getMapWalkCost(AStarNode* node, const Position &neighbo
int_fast32_t AStarNodes::getTileWalkCost(const std::shared_ptr<Creature> &creature, const std::shared_ptr<Tile> &tile) {
int_fast32_t cost = 0;

if (creature) {
if (creature && tile) {
// Destroy creature cost
if (tile->getTopVisibleCreature(creature) != nullptr) {
// destroy creature cost
cost += MAP_NORMALWALKCOST * 4;
}
if (const auto &field = tile->getFieldItem()) {
Expand Down
2 changes: 2 additions & 0 deletions src/pch.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@
#include <cmath>
#include <mutex>
#include <stack>
#include <source_location>
#include <span>

// --------------------
// System Includes
Expand Down
Loading

0 comments on commit 04fa248

Please sign in to comment.