fix(memory): Fix memory leak for RecorderClass::m_crcInfo#2713
fix(memory): Fix memory leak for RecorderClass::m_crcInfo#2713Caball009 wants to merge 5 commits into
Conversation
|
| Filename | Overview |
|---|---|
| GeneralsMD/Code/GameEngine/Include/Common/Recorder.h | CRCInfo moved here as a nested protected class with a new default constructor; m_crcInfo changed from pointer to value member. |
| GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp | CRCInfo class definition and file-local implementation removed; moved to header/scoped definitions. All pointer dereferences updated to value access. |
Reviews (3): Last reviewed commit: "Made 'CRCInfo' class part of 'RecorderCl..." | Re-trigger Greptile
282478b to
c5e3079
Compare
This avoids an unnecessary dynamic memory allocation and fixes a memory leak.
xezon
left a comment
There was a problem hiding this comment.
Looking good. Needs replicate to Generals.
| int GetQueueSize() const { return m_data.size(); } | ||
|
|
||
| UnsignedInt getLocalPlayer() { return m_localPlayer; } | ||
|
|
There was a problem hiding this comment.
Can remove the empty lines here for compactness.
| // compare those with the local gamestate, they wouldn't sync up. | ||
| // So, in order to fix this, we need to queue up our local CRCs, | ||
| // so that we can check it with the crc messages that come later. | ||
| // This class is basically that queue. |
There was a problem hiding this comment.
While at it, perhaps compact the format of the massive comment a bit.
| #endif | ||
|
|
||
| RecorderClass::CRCInfo::CRCInfo() : | ||
| m_sawCRCMismatch(FALSE), |
There was a problem hiding this comment.
While at it, prefer format like:
RecorderClass::CRCInfo::CRCInfo()
: m_sawCRCMismatch(FALSE)
, m_skippedOne(FALSE)|
|
||
| class CRCInfo; | ||
|
|
||
| class RecorderClass : public SubsystemInterface { |
There was a problem hiding this comment.
While at it
class RecorderClass : public SubsystemInterface
{
This PR fixes a memory leak for
RecorderClass::m_crcInfo. I decided to fix this one by not allocating theCRCInfoobject dynamically. This did require a larger change of moving the class to the header, though.See commits for clean diffs.
TODO: