incidentalstoat/blankart
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Introducing: __attribute__((__cleanup__))

Browse source 
I don't wanna hear about MSVC... or C++...
This fixes NUMEROUS memory leaks introduced by G_ReadDemoHeader,
which I was utterly oblivious to up until an hour ago
This commit is contained in:
GenericHeroGuy 2025-09-20 16:52:44 +02:00
parent b36b327a00
commit e5c97ca233
4 changed files with 48 additions and 77 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
src/doomdef.h
View file

@ -475,6 +475,12 @@ UINT32 quickncasehash (const char *p, size_t n)
#endif
#endif
// the GNU cleanup attribute: plugging memory leaks since 2003!
// on scope exit, the cleanup function is called with a pointer to the declared variable,
// essentially behaving like a C++ destructor
// NOTE: you WILL have nasal troubles if the variable is not initialized
#define CLEANUP(f) __attribute__((__cleanup__(f)))
// An assert-type mechanism.
// NOTE: USE SRB2_ASSERT FOR C++ CODE INSTEAD
#ifdef PARANOIA

112
src/g_demo.c
View file

@ -649,7 +649,7 @@ static UINT8 *G_ReadRawExtraData(extradata_t *extra, UINT8 *dp, UINT16 version)
}
// parses a demo header from the given byte pointer
// remember to call G_FreeDemoHeader! (unless an error occurs)
// remember to call G_FreeDemoHeader!
static headerstatus_e G_ReadDemoHeader(UINT8 *dp, demoheader_t *header)
{
UINT8 *startdp = dp;
@ -3182,13 +3182,11 @@ UINT8 G_CmpDemoTime(char *oldname, char *newname)
I_Assert(bufsize != 0);
// read demo header
demoheader_t header;
CLEANUP(G_FreeDemoHeader) demoheader_t header;
headerstatus_e status = G_ReadDemoHeader(buffer, &header);
(void)status;
I_Assert(status == HEADER_OK);
I_Assert(header.version == VERSION);
I_Assert(header.subversion == SUBVERSION);
I_Assert(header.demoversion == DEMOVERSION);
I_Assert(status == HEADER_OK && header.version == VERSION
&& header.subversion == SUBVERSION && header.demoversion == DEMOVERSION);
Z_Free(buffer);
@ -3204,8 +3202,6 @@ UINT8 G_CmpDemoTime(char *oldname, char *newname)
else
newlap = UINT32_MAX;
G_FreeDemoHeader(&header);
// load old file
FIL_DefaultExtension(oldname, ".lmp");
if (!FIL_ReadFile(oldname, &buffer))
@ -3215,7 +3211,7 @@ UINT8 G_CmpDemoTime(char *oldname, char *newname)
}
// read demo header
demoheader_t oldheader;
CLEANUP(G_FreeDemoHeader) demoheader_t oldheader;
if (G_ReadDemoHeader(buffer, &oldheader) != HEADER_OK)
{
CONS_Alert(CONS_NOTICE, M_GetText("File '%s' invalid format. It will be overwritten.\n"), oldname);
@ -3228,7 +3224,6 @@ UINT8 G_CmpDemoTime(char *oldname, char *newname)
if (!(oldheader.demoflags & aflags))
{
CONS_Alert(CONS_NOTICE, M_GetText("File '%s' not from same game mode. It will be overwritten.\n"), oldname);
G_FreeDemoHeader(&oldheader);
return UINT8_MAX;
}
@ -3238,8 +3233,6 @@ UINT8 G_CmpDemoTime(char *oldname, char *newname)
else
oldlap = 0;
G_FreeDemoHeader(&oldheader);
c = 0;
if (uselaps)
@ -3262,20 +3255,20 @@ UINT8 G_CmpDemoTime(char *oldname, char *newname)
void G_LoadDemoInfo(menudemo_t *pdemo)
{
UINT8 *infobuffer, *extrainfo_p;
CLEANUP(Z_Pfree) UINT8 *infobuffer = NULL;
const UINT8 *extrainfo_p;
if (!FIL_ReadFile(pdemo->filepath, &infobuffer))
{
CONS_Alert(CONS_ERROR, M_GetText("Failed to read file '%s'.\n"), pdemo->filepath);
pdemo->type = MD_INVALID;
sprintf(pdemo->title, "INVALID REPLAY");
return;
}
pdemo->type = MD_LOADED;
demoheader_t header;
CLEANUP(G_FreeDemoHeader) demoheader_t header;
switch (G_ReadDemoHeader(infobuffer, &header))
{
case HEADER_OK:
@ -3285,21 +3278,18 @@ void G_LoadDemoInfo(menudemo_t *pdemo)
CONS_Alert(CONS_ERROR, M_GetText("%s is not a SRB2Kart replay file.\n"), pdemo->filepath);
pdemo->type = MD_INVALID;
sprintf(pdemo->title, "INVALID REPLAY");
Z_Free(infobuffer);
return;
case HEADER_BADVERSION:
CONS_Alert(CONS_ERROR, M_GetText("%s is an incompatible replay format and cannot be played.\n"), pdemo->filepath);
pdemo->type = MD_INVALID;
sprintf(pdemo->title, "INVALID REPLAY");
Z_Free(infobuffer);
return;
case HEADER_BADFORMAT:
CONS_Alert(CONS_ERROR, M_GetText("%s is the wrong type of recording and cannot be played.\n"), pdemo->filepath);
pdemo->type = MD_INVALID;
sprintf(pdemo->title, "INVALID REPLAY");
Z_Free(infobuffer);
return;
}
@ -3314,7 +3304,6 @@ void G_LoadDemoInfo(menudemo_t *pdemo)
if (!(header.demoflags & DF_MULTIPLAYER))
{
CONS_Alert(CONS_ERROR, M_GetText("%s is not a multiplayer replay and can't be listed on this menu fully yet.\n"), pdemo->filepath);
Z_Free(infobuffer);
return;
}
@ -3387,10 +3376,6 @@ void G_LoadDemoInfo(menudemo_t *pdemo)
if (count >= MAXPLAYERS)
break; //@TODO still cycle through the rest of these if extra demo data is ever used
}
// I think that's everything we need?
Z_Free(infobuffer);
G_FreeDemoHeader(&header);
}
//
@ -3413,7 +3398,8 @@ void G_DoPlayDemo(char *defdemoname)
{
UINT16 i, j;
lumpnum_t l;
char *n,*pdemoname;
CLEANUP(Z_Pfree) char *pdemoname = NULL;
const char *n;
char msg[1024];
UINT8 pnum;
@ -3511,12 +3497,21 @@ void G_DoPlayDemo(char *defdemoname)
}
}
goto wemadeit; // :face_holding_back_tears:
lumperror:
CONS_Alert(CONS_ERROR, "%s", msg);
demo.playback = false;
demo.title = false;
if (!CON_Ready()) // In the console they'll just see the notice there! No point pulling them out.
M_StartMessage(msg, NULL, MM_NOTHING);
wemadeit:
// read demo header
demo.playback = true;
demo.buffer = &demobuf;
demoheader_t header;
CLEANUP(G_FreeDemoHeader) demoheader_t header;
switch (G_ReadDemoHeader(demobuf.p, &header))
{
case HEADER_OK:
@ -3525,15 +3520,15 @@ void G_DoPlayDemo(char *defdemoname)
case HEADER_BADMAGIC:
snprintf(msg, 1024, M_GetText("%s is not a SRB2Kart replay file.\n"), pdemoname);
goto headererror;
goto error;
case HEADER_BADVERSION:
snprintf(msg, 1024, M_GetText("%s is an incompatible replay format and cannot be played.\n"), pdemoname);
goto headererror;
goto error;
case HEADER_BADFORMAT:
snprintf(msg, 1024, M_GetText("%s is the wrong type of recording and cannot be played.\n"), pdemoname);
goto headererror;
goto error;
}
demo.version = header.demoversion;
@ -3660,8 +3655,6 @@ void G_DoPlayDemo(char *defdemoname)
// Load "mapmusrng" used for altmusic selection
mapmusrng = header.mapmusrng;
Z_Free(pdemoname);
memset(&oldcmd,0,sizeof(oldcmd));
memset(&oldghost,0,sizeof(oldghost));
memset(&ghostext,0,sizeof(ghostext));
@ -3809,16 +3802,11 @@ void G_DoPlayDemo(char *defdemoname)
}
demo.deferstart = true;
G_FreeDemoHeader(&header);
return;
error:
G_FreeDemoHeader(&header);
headererror:
P_SaveBufferFree(&demobuf);
lumperror:
CONS_Alert(CONS_ERROR, "%s", msg);
Z_Free(pdemoname);
demo.playback = false;
demo.title = false;
if (!CON_Ready()) // In the console they'll just see the notice there! No point pulling them out.
@ -3837,11 +3825,12 @@ void G_AddGhost(char *defdemoname)
{
INT32 i;
lumpnum_t l;
char *n,*pdemoname;
CLEANUP(Z_Pfree) char *pdemoname = NULL;
const char *n;
UINT64 demohash;
demoghost *gh;
UINT8 flags;
UINT8 *buffer;
CLEANUP(Z_Pfree) UINT8 *buffer = NULL;
mapthing_t *mthing;
skin_t *ghskin = &skins[0];
@ -3860,7 +3849,6 @@ void G_AddGhost(char *defdemoname)
if (!FIL_ReadFileTag(defdemoname, &buffer, PU_LEVEL))
{
CONS_Alert(CONS_ERROR, M_GetText("Failed to read file '%s'.\n"), defdemoname);
Z_Free(pdemoname);
return;
}
}
@ -3868,13 +3856,12 @@ void G_AddGhost(char *defdemoname)
else if ((l = W_CheckNumForName(defdemoname)) == LUMPERROR)
{
CONS_Alert(CONS_ERROR, M_GetText("Failed to read lump '%s'.\n"), defdemoname);
Z_Free(pdemoname);
return;
}
else // it's an internal demo
buffer = W_CacheLumpNum(l, PU_LEVEL);
demoheader_t header;
CLEANUP(G_FreeDemoHeader) demoheader_t header;
switch (G_ReadDemoHeader(buffer, &header))
{
case HEADER_OK:
@ -3882,20 +3869,14 @@ void G_AddGhost(char *defdemoname)
case HEADER_BADMAGIC:
CONS_Alert(CONS_NOTICE, M_GetText("Ghost %s: Not a SRB2 replay.\n"), pdemoname);
Z_Free(pdemoname);
Z_Free(buffer);
return;
case HEADER_BADVERSION:
CONS_Alert(CONS_NOTICE, M_GetText("Ghost %s: Demo version incompatible.\n"), pdemoname);
Z_Free(pdemoname);
Z_Free(buffer);
return;
case HEADER_BADFORMAT:
CONS_Alert(CONS_NOTICE, M_GetText("Ghost %s: Demo format unacceptable.\n"), pdemoname);
Z_Free(pdemoname);
Z_Free(buffer);
return;
}
@ -3905,9 +3886,6 @@ void G_AddGhost(char *defdemoname)
if (demohash == gh->checksum) // another ghost in the game already has this checksum?
{ // Don't add another one, then!
CONS_Debug(DBG_SETUP, "Rejecting duplicate ghost %s (hash was matched)\n", pdemoname);
Z_Free(pdemoname);
Z_Free(buffer);
G_FreeDemoHeader(&header);
return;
}
@ -3916,27 +3894,18 @@ void G_AddGhost(char *defdemoname)
if (!(flags & DF_GHOST))
{
CONS_Alert(CONS_NOTICE, M_GetText("Ghost %s: No ghost data in this demo.\n"), pdemoname);
Z_Free(pdemoname);
Z_Free(buffer);
G_FreeDemoHeader(&header);
return;
}
if (flags & DF_LUAVARS) // can't be arsed to add support for grinding away ported lua material
{
CONS_Alert(CONS_NOTICE, M_GetText("Ghost %s: Replay data contains luavars, cannot continue.\n"), pdemoname);
Z_Free(pdemoname);
Z_Free(buffer);
G_FreeDemoHeader(&header);
return;
}
if (header.empty)
{
CONS_Alert(CONS_NOTICE, M_GetText("Failed to add ghost %s: Replay is empty.\n"), pdemoname);
Z_Free(pdemoname);
Z_Free(buffer);
G_FreeDemoHeader(&header);
return;
}
@ -3946,9 +3915,6 @@ void G_AddGhost(char *defdemoname)
if ((plr->flags & (DEMO_SPECTATOR|DEMO_BOT)) != 0)
{
CONS_Alert(CONS_NOTICE, M_GetText("Failed to add ghost %s: Invalid player slot (spectator/bot)\n"), defdemoname);
Z_Free(pdemoname);
Z_Free(buffer);
G_FreeDemoHeader(&header);
return;
}
@ -3972,6 +3938,7 @@ void G_AddGhost(char *defdemoname)
gh->buffer = buffer;
gh->checksum = demohash;
gh->p = buffer + header.endofs;
buffer = NULL; // buffer can't be freed now!
ghosts = gh;
@ -4026,8 +3993,6 @@ void G_AddGhost(char *defdemoname)
gh->oldmo.color = gh->mo->color;
CONS_Printf(M_GetText("Added ghost %s from %s\n"), plr->name, pdemoname);
G_FreeDemoHeader(&header);
Z_Free(pdemoname);
}
// Clean up all ghosts
@ -4045,26 +4010,21 @@ void G_FreeGhosts(void)
// A simplified version of G_AddGhost...
void G_UpdateStaffGhostName(lumpnum_t l)
{
UINT8 *buffer = W_CacheLumpNum(l, PU_CACHE);
CLEANUP(Z_Pfree) UINT8 *buffer = W_CacheLumpNum(l, PU_CACHE);
demoheader_t header;
CLEANUP(G_FreeDemoHeader) demoheader_t header;
if (G_ReadDemoHeader(buffer, &header) != HEADER_OK)
goto fail;
return;
if (!(header.demoflags & DF_GHOST))
goto fail; // we don't NEED to do it here, but whatever
return; // we don't NEED to do it here, but whatever
if (header.numplayers == 0 || header.playerdata[0].playernum != 0)
goto fail;
return;
if (header.playerdata[0].flags & (DEMO_SPECTATOR|DEMO_BOT))
goto fail;
strcpy(dummystaffname, header.playerdata[0].name);
return;
// Ok, no longer any reason to care, bye
fail:
G_FreeDemoHeader(&header);
Z_Free(buffer);
return;
strcpy(dummystaffname, header.playerdata[0].name);
}
//

1
src/y_inter.c
View file

@ -992,7 +992,6 @@ static void Y_FollowIntermission(void)
}
#define UNLOAD(x) {if ((x) != NULL) {Patch_Free(x);} x = NULL;}
#define CLEANUP(x) x = NULL;
//
// Y_UnloadData

6
src/z_zone.h
View file

@ -147,6 +147,12 @@ char *Z_StrDup(const char *in);
#define Z_Unlock(p) (void)p // TODO: remove this now that NDS code has been removed
char *zva(INT32 tag, void *user, const char *format, ...);
// for use with CLEANUP macro
FUNCINLINE static ATTRINLINE void Z_Pfree(void *p)
{
Z_Free(*(void **)p);
}
#ifdef __cplusplus
} // extern "C"
#endif