diff --git a/src/doomdef.h b/src/doomdef.h index f1e9a57fd..7d85d7b85 100644 --- a/src/doomdef.h +++ b/src/doomdef.h @@ -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 diff --git a/src/g_demo.c b/src/g_demo.c index ba55c2863..510d5adbd 100644 --- a/src/g_demo.c +++ b/src/g_demo.c @@ -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); } // diff --git a/src/y_inter.c b/src/y_inter.c index c16feb7ae..c79af403e 100644 --- a/src/y_inter.c +++ b/src/y_inter.c @@ -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 diff --git a/src/z_zone.h b/src/z_zone.h index f7287795f..1876fa0be 100644 --- a/src/z_zone.h +++ b/src/z_zone.h @@ -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