Skip to content

Commit

Permalink
[core] Fix a bunch of "new" with no "delete" from Valgrind tests
Browse files Browse the repository at this point in the history
  • Loading branch information
WinterSolstice8 committed Mar 3, 2024
1 parent 02a6f7d commit 4da606d
Show file tree
Hide file tree
Showing 30 changed files with 399 additions and 86 deletions.
13 changes: 13 additions & 0 deletions src/common/taskmgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,17 @@
#include "common/tracy.h"
#include "common/utils.h"

CTaskMgr::~CTaskMgr()
{
while (!m_TaskList.empty())
{
CTask* PTask = m_TaskList.top();
m_TaskList.pop();

destroy(PTask);
}
}

CTaskMgr::CTask* CTaskMgr::AddTask(std::string const& InitName, time_point InitTick, std::any InitData, TASKTYPE InitType, TaskFunc_t InitFunc, duration InitInterval)
{
TracyZoneScoped;
Expand Down Expand Up @@ -56,13 +67,15 @@ void CTaskMgr::RemoveTask(std::string const& TaskName)
m_TaskList.pop();

// Don't add tasks we're trying to remove to the new pq
// FIXME: duplicate task names AREN'T checked on insert!
if (PTask->m_name != TaskName)
{
newPq.push(PTask);
}
else
{
++tasksRemoved;
destroy(PTask);
}
}

Expand Down
1 change: 1 addition & 0 deletions src/common/taskmgr.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class CTaskMgr : public Singleton<CTaskMgr>
{
public:
class CTask;
~CTaskMgr();
enum TASKTYPE
{
TASK_INTERVAL,
Expand Down
25 changes: 25 additions & 0 deletions src/map/ability.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,31 @@ namespace ability
}
}

void CleanupAbilitiesList()
{
// Call delete on Charge_t* in PChargeslist, because these are not STL containers and will not be destructed
for (auto* charge : PChargesList)
{
destroy(charge);
}

// Delete everything in the abilities list
for (int i = 0; i < MAX_ABILITY_ID; i++)
{
if (PAbilityList[i])
{
destroy(PAbilityList[i]);
}
}

// Clear every vector that now has invalid pointers
for (auto vec : PAbilitiesList)
{
vec.clear();
}

PChargesList.clear();
}
/************************************************************************
* *
* Get Ability By ID *
Expand Down
1 change: 1 addition & 0 deletions src/map/ability.h
Original file line number Diff line number Diff line change
Expand Up @@ -773,6 +773,7 @@ class CAbility
namespace ability
{
void LoadAbilitiesList();
void CleanupAbilitiesList();

CAbility* GetAbility(uint16 AbilityID);

Expand Down
2 changes: 1 addition & 1 deletion src/map/ai/controllers/automaton_controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ bool CAutomatonController::shouldStandBack()

if (PMaster)
{
CItemWeapon* animator = static_cast<CItemWeapon*>(PMaster->m_Weapons[SLOT_AMMO]);
CItemWeapon* animator = dynamic_cast<CItemWeapon*>(PMaster->m_Weapons[SLOT_AMMO]);

if (animator && animator->getSubSkillType() == SUBSKILLTYPE::SUBSKILL_ANIMATOR_II)
{
Expand Down
2 changes: 1 addition & 1 deletion src/map/attackround.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ void CAttackRound::CreateAttacks(CItemWeapon* PWeapon, PHYSICAL_ATTACK_DIRECTION
// Daken is handled separately in CreateDakenAttack() and Zanshin in src/map/entities/battleentity.cpp#L1768

// Checking Mikage Effect - Hits Vary With Num of Utsusemi Shadows for Main Weapon
if (m_attacker->StatusEffectContainer->HasStatusEffect(EFFECT_MIKAGE) && m_attacker->m_Weapons[SLOT_MAIN]->getID() == PWeapon->getID())
if (m_attacker->StatusEffectContainer->HasStatusEffect(EFFECT_MIKAGE) && m_attacker->m_Weapons[SLOT_MAIN] && m_attacker->m_Weapons[SLOT_MAIN]->getID() == PWeapon->getID())
{
auto shadows = (uint8)m_attacker->getMod(Mod::UTSUSEMI);
AddAttackSwing(PHYSICAL_ATTACK_TYPE::NORMAL, direction, shadows);
Expand Down
4 changes: 2 additions & 2 deletions src/map/entities/baseentity.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -197,12 +197,12 @@ void CBaseEntity::ResetLocalVars()
m_localVars.clear();
}

uint32 CBaseEntity::GetLocalVar(const char* var)
uint32 CBaseEntity::GetLocalVar(std::string var)
{
return m_localVars[var];
}

void CBaseEntity::SetLocalVar(const char* var, uint32 val)
void CBaseEntity::SetLocalVar(std::string var, uint32 val)
{
m_localVars[var] = val;
}
Expand Down
4 changes: 2 additions & 2 deletions src/map/entities/baseentity.h
Original file line number Diff line number Diff line change
Expand Up @@ -281,8 +281,8 @@ class CBaseEntity
void SendZoneUpdate();

void ResetLocalVars();
uint32 GetLocalVar(const char* var);
void SetLocalVar(const char* var, uint32 val);
uint32 GetLocalVar(std::string var);
void SetLocalVar(std::string var, uint32 val);

// pre-tick update
virtual void Tick(time_point) = 0;
Expand Down
14 changes: 7 additions & 7 deletions src/map/entities/battleentity.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,10 @@ CBattleEntity::CBattleEntity()

m_magicEvasion = 0;

m_Weapons[SLOT_MAIN] = new CItemWeapon(0);
m_Weapons[SLOT_SUB] = new CItemWeapon(0);
m_Weapons[SLOT_RANGED] = new CItemWeapon(0);
m_Weapons[SLOT_AMMO] = new CItemWeapon(0);
m_Weapons[SLOT_MAIN] = nullptr;
m_Weapons[SLOT_SUB] = nullptr;
m_Weapons[SLOT_RANGED] = nullptr;
m_Weapons[SLOT_AMMO] = nullptr;
m_dualWield = false;

memset(&health, 0, sizeof(health));
Expand Down Expand Up @@ -381,8 +381,8 @@ float CBattleEntity::GetMeleeRange() const

int16 CBattleEntity::GetRangedWeaponDelay(bool tp)
{
CItemWeapon* PRange = (CItemWeapon*)m_Weapons[SLOT_RANGED];
CItemWeapon* PAmmo = (CItemWeapon*)m_Weapons[SLOT_AMMO];
CItemWeapon* PRange = dynamic_cast<CItemWeapon*>(m_Weapons[SLOT_RANGED]);
CItemWeapon* PAmmo = dynamic_cast<CItemWeapon*>(m_Weapons[SLOT_AMMO]);

// base delay
int16 delay = 0;
Expand All @@ -408,7 +408,7 @@ int16 CBattleEntity::GetRangedWeaponDelay(bool tp)

int16 CBattleEntity::GetAmmoDelay()
{
CItemWeapon* PAmmo = (CItemWeapon*)m_Weapons[SLOT_AMMO];
CItemWeapon* PAmmo = dynamic_cast<CItemWeapon*>(m_Weapons[SLOT_AMMO]);

int delay = 0;
if (PAmmo != nullptr && PAmmo->getDamage() != 0)
Expand Down
1 change: 1 addition & 0 deletions src/map/entities/charentity.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ CCharEntity::~CCharEntity()
destroy(CraftContainer);
destroy(PMeritPoints);
destroy(PJobPoints);
destroy(PLatentEffectContainer);

PGuildShop = nullptr;

Expand Down
21 changes: 21 additions & 0 deletions src/map/entities/mobentity.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,11 @@ CMobEntity::CMobEntity()
PEnmityContainer = new CEnmityContainer(this);
SpellContainer = new CMobSpellContainer(this);

m_Weapons[SLOT_MAIN] = new CItemWeapon(0);
m_Weapons[SLOT_SUB] = new CItemWeapon(0);
m_Weapons[SLOT_RANGED] = new CItemWeapon(0);
m_Weapons[SLOT_AMMO] = new CItemWeapon(0);

PAI = std::make_unique<CAIContainer>(this, std::make_unique<CPathFind>(this), std::make_unique<CMobController>(this), std::make_unique<CTargetFind>(this));
}

Expand All @@ -136,8 +141,24 @@ void CMobEntity::setEntityFlags(uint32 EntityFlags)

CMobEntity::~CMobEntity()
{
destroy(m_Weapons[SLOT_MAIN]);
destroy(m_Weapons[SLOT_SUB]);
destroy(m_Weapons[SLOT_RANGED]);
destroy(m_Weapons[SLOT_AMMO]);
destroy(PEnmityContainer);
destroy(SpellContainer);

if (PParty)
{
if (PParty->HasOnlyOneMember())
{
destroy(PParty);
}
else
{
PParty->DelMember(this);
}
}
}

/************************************************************************
Expand Down
10 changes: 8 additions & 2 deletions src/map/lua/lua_baseentity.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15805,7 +15805,10 @@ void CLuaBaseEntity::setDelay(uint16 delay)
}

auto* PMobEntity = static_cast<CMobEntity*>(m_PBaseEntity);
static_cast<CItemWeapon*>(PMobEntity->m_Weapons[SLOT_MAIN])->setDelay(delay);
if (auto* PItemWeapon = dynamic_cast<CItemWeapon*>(PMobEntity->m_Weapons[SLOT_MAIN]))
{
PItemWeapon->setDelay(delay);
}
}

/************************************************************************
Expand All @@ -15823,7 +15826,10 @@ void CLuaBaseEntity::setDamage(uint16 damage)
}

auto* PMobEntity = static_cast<CMobEntity*>(m_PBaseEntity);
static_cast<CItemWeapon*>(PMobEntity->m_Weapons[SLOT_MAIN])->setDamage(damage);
if (auto* PItemWeapon = dynamic_cast<CItemWeapon*>(PMobEntity->m_Weapons[SLOT_MAIN]))
{
PItemWeapon->setDamage(damage);
}
}

/************************************************************************
Expand Down
12 changes: 12 additions & 0 deletions src/map/map.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -396,10 +396,15 @@ void do_final(int code)
destroy_arr(g_PBuff);
destroy_arr(PTempBuff);

ability::CleanupAbilitiesList();
itemutils::FreeItemList();
battleutils::FreeWeaponSkillsList();
battleutils::FreeMobSkillList();
battleutils::FreePetSkillList();
fishingutils::CleanupFishing();
guildutils::Cleanup();
mobutils::Cleanup();
traits::ClearTraitsList();

petutils::FreePetList();
trustutils::FreeTrustList();
Expand All @@ -417,6 +422,13 @@ void do_final(int code)

timer_final();
socket_final();

for (auto session : map_session_list)
{
destroy_arr(session.second->server_packet_data);
destroy(session.second);
}

luautils::cleanup();
logging::ShutDown();

Expand Down
15 changes: 11 additions & 4 deletions src/map/navmesh.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,13 @@ CNavMesh::CNavMesh(uint16 zoneID)
m_hit.maxPath = 20;
}

CNavMesh::~CNavMesh() = default;
CNavMesh::~CNavMesh()
{
if (m_navMesh)
{
dtFreeNavMesh(m_navMesh);
}
}

bool CNavMesh::load(std::string const& filename)
{
Expand All @@ -142,7 +148,7 @@ bool CNavMesh::load(std::string const& filename)
return false;
}

m_navMesh.reset(dtAllocNavMesh());
m_navMesh = dtAllocNavMesh();
if (!m_navMesh)
{
return false;
Expand Down Expand Up @@ -178,7 +184,7 @@ bool CNavMesh::load(std::string const& filename)
}

// init detour nav mesh path finder
status = m_navMeshQuery.init(m_navMesh.get(), MAX_NAV_POLYS);
status = m_navMeshQuery.init(m_navMesh, MAX_NAV_POLYS);

if (dtStatusFailed(status))
{
Expand All @@ -198,7 +204,8 @@ void CNavMesh::reload()

void CNavMesh::unload()
{
m_navMesh.reset();
dtFreeNavMesh(m_navMesh);
m_navMesh = nullptr;
}

std::vector<pathpoint_t> CNavMesh::findPath(const position_t& start, const position_t& end)
Expand Down
12 changes: 6 additions & 6 deletions src/map/navmesh.h
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,12 @@ class CNavMesh
private:
bool onSameFloor(const position_t& start, float* spos, const position_t& end, float* epos, dtQueryFilter& filter);

std::string m_filename;
uint16 m_zoneID;
dtRaycastHit m_hit{};
dtPolyRef m_hitPath[20]{};
std::shared_ptr<dtNavMesh> m_navMesh;
dtNavMeshQuery m_navMeshQuery;
std::string m_filename;
uint16 m_zoneID;
dtRaycastHit m_hit{};
dtPolyRef m_hitPath[20]{};
dtNavMesh* m_navMesh;
dtNavMeshQuery m_navMeshQuery;
};

#endif
3 changes: 2 additions & 1 deletion src/map/packets/server_message.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ CServerMessagePacket::CServerMessagePacket(const std::string& message, int8 lang
auto textSize = (uint8)(sndLength + sndLength % 2);
this->setSize(((((0x14 + textSize) + 4) >> 1) & 0xFE) * 2);

std::memcpy(data + 0x18, message.c_str() + message_offset, sndLength);
// TODO: can
std::memcpy(data + 0x18, message.c_str() + message_offset, std::min(sndLength, message.length() - message_offset));
}
}
8 changes: 4 additions & 4 deletions src/map/party.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,13 +117,12 @@ void CParty::DisbandParty(bool playerInitiated)
m_PAlliance->removeParty(this);
}
m_PSyncTarget = nullptr;
SetQuarterMaster("");

m_PLeader = nullptr;
m_PAlliance = nullptr;
m_PLeader = nullptr;
m_PAlliance = nullptr;

if (m_PartyType == PARTY_PCS)
{
SetQuarterMaster("");
PushPacket(0, 0, new CPartyDefinePacket(nullptr));

for (auto& member : members)
Expand Down Expand Up @@ -1130,6 +1129,7 @@ void CParty::SetSyncTarget(const std::string& MemberName, uint16 message)
}
}

// FIXME: add case for "" membername
void CParty::SetQuarterMaster(const std::string& MemberName)
{
CBattleEntity* PEntity = GetMemberByName(MemberName);
Expand Down
12 changes: 12 additions & 0 deletions src/map/trait.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,18 @@ namespace traits
}
}

void ClearTraitsList()
{
// Manually cleanup traits list
for (auto jobTraitList : PTraitsList)
{
for (auto traitList : jobTraitList)
{
destroy(traitList);
}
jobTraitList.clear();
}
}
/************************************************************************
* *
* Get List of Traits by Main Job or Sub Job *
Expand Down
1 change: 1 addition & 0 deletions src/map/trait.h
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ typedef std::vector<CTrait*> TraitList_t;
namespace traits
{
void LoadTraitsList();
void ClearTraitsList();

TraitList_t* GetTraits(uint8 JobID);
}; // namespace traits
Expand Down
Loading

0 comments on commit 4da606d

Please sign in to comment.