szapp / GothicFreeAim

Free aiming for the video game series Gothic
https://gfa.szapp.de
MIT License
36 stars 4 forks source link

Confirm that there is no memory leak in reseting class strings #117

Closed szapp closed 7 years ago

szapp commented 7 years ago

This code seems suspicious as it might allocate a new string every time it's called without freeing the memory of the previous string (which is still present thanks to deadalus's pseudo-locals - OR IS IT?).

var int autoAlloc[8]; var Weakspot weakspot; weakspot = _^(_@(autoAlloc)); // Gothic takes care of freeing this ptr
MEM_CopyWords(_@s(""), _@(autoAlloc), 5); // weakspot.node (reset string)

Same with the weakspot class

var int autoAlloc[7]; var Reticle reticle; reticle = _^(_@(autoAlloc)); // Gothic takes care of freeing this ptr
MEM_CopyWords(_@s(""), _@(autoAlloc), 5); // reticle.texture (reset string)

This is the zString class.

class zString {
    var int _vtbl;
    var int _allocater;
    var int ptr;
    var int len;
    var int res;
};

The property _vtbl should be checked to see whether there is already a string in place. If not, create it like it is done above, otherwise set to empty string like usual.

if (MEM_ReadInt(_@(autoAlloc)) != zSTRING_vtbl) { // zSTRING_vtbl has to be found and defined first
    MEM_CopyWords(_@s(""), _@(autoAlloc), 5); // Allocate string
} else {
    // One of these
    reticle.texture = ""; // reticle.texture (reset string)
    weakspot.node = ""; // weakspot.node (reset string)
};

However, this should be tested first, because I am not sure whether the string will survive between function calls.

szapp commented 7 years ago

This is way to sketchy. Instead just use MEM_Alloc() and MEM_Free().