[ZH] Minor memory corruption and leak fixes#716
Conversation
# Conflicts: # GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/DockUpdate/DockUpdate.cpp
Mauller
left a comment
Mauller
left a comment
There was a problem hiding this comment.
They look good overall. But you need to rebase this PR and only have it include commits related to your changes.
I would also pullout the Iterator changes into their own PR and include similar generals related changes as these won't affect runtime. But the other changes need investigating first.
| if (items) | ||
| { | ||
| for (ContainedItemsList::const_iterator it = items->begin(); *it != NULL && numKilled < d->m_garrisonHitKillCount; ) | ||
| for (ContainedItemsList::const_iterator it = items->begin(); it != items->end() && numKilled < d->m_garrisonHitKillCount; ) |
There was a problem hiding this comment.
Looks genuine to me, should not affect VC6 as the original behaviour was fine for STLPort.
If you can replicate this and the other iterator fix to generals as well, that would be great.
|
|
||
| // TheSuperHackers @fix helmutbuhler 04/19/2025 | ||
| // This originally mistakenly assigned to m_orbitToTargetBeamID and thus caused a leak. | ||
| m_groundToOrbitBeamID = INVALID_DRAWABLE_ID; |
There was a problem hiding this comment.
Looks legitimate to me, will just need to check against VC6 that we don't cause a mismatch by this change,
| TheThingFactory->newDrawable( tTemplate ); | ||
| // TheSuperHackers @bugfix helmutbuhler 04/13/2025 | ||
| // draw was originally not assigned here and potentially caused memory corruption | ||
| draw = TheThingFactory->newDrawable( tTemplate ); |
There was a problem hiding this comment.
Looks genuine, might need checking again against VC6 since it could cause a mismatch even though it fixes behaviour.
|
Please make this 4 pull requests and replicate them in Generals. |
|
I have pulled this PR apart into 4 seperate pull requests. It should get closed when the iterator usage PR is merged. |
|
Thank you, I'll close this right away |
4 participants
4 easy fixes:
I tested all changes by checking some replays for mismatches.
Note about the second fix: You can check that it reads that uninitialized memory by setting all bits to one:
memset(approachBones, -1, sizeof(approachBones));It will then mismatch on the golden replay 1. It seems we are lucky that that memory is all 0 on retail version.
About the last fix: I'm not sure what drawable the retail binary is accessing when using the just freed drawable, but I have a replay which uses that code path and the fix works fine there.