From 7e17f5456cd31fb8f2b31fa404493eb2021d734b Mon Sep 17 00:00:00 2001 From: toaster Date: Sat, 17 Sep 2022 13:07:48 +0100 Subject: [PATCH] I_Error in all situations where mapheaders were previously allocated outside of SOC. Also: - improved error prints for SOC condition definitions - improved bounds checking to use `nummapheaders` for iterating over mapheaderinfo There are still situations that use NUMMAPS like mapvisited, randmapbuffer, etc, which need to be addressed before merger. --- src/command.c | 36 ----------------------------- src/d_netcmd.c | 20 +++++++++++----- src/deh_soc.c | 58 +++++++++++++++++++++++------------------------ src/g_demo.c | 2 +- src/g_game.c | 45 ++++++++++++------------------------ src/k_menudraw.c | 4 ++-- src/k_menufunc.c | 10 ++++---- src/lua_baselib.c | 6 ++--- src/lua_maplib.c | 6 ++--- src/p_saveg.c | 8 +++---- src/y_inter.c | 4 ++-- 11 files changed, 77 insertions(+), 122 deletions(-) diff --git a/src/command.c b/src/command.c index 1439463c1..ec56ad20f 100644 --- a/src/command.c +++ b/src/command.c @@ -1961,42 +1961,6 @@ void CV_AddValue(consvar_t *var, INT32 increment) if (var->PossibleValue) { - /* - if (var == &cv_nextmap) - { - // Special case for the nextmap variable, used only directly from the menu - INT32 oldvalue = var->value - 1, gt; - gt = cv_newgametype.value; - { - newvalue = var->value - 1; - do - { - if(increment > 0) // Going up! - { - if (++newvalue == NUMMAPS) - newvalue = -1; - } - else // Going down! - { - if (--newvalue == -2) - newvalue = NUMMAPS-1; - } - - if (newvalue == oldvalue) - break; // don't loop forever if there's none of a certain gametype - - if(!mapheaderinfo[newvalue]) - continue; // Don't allocate the header. That just makes memory usage skyrocket. - - } while (!M_CanShowLevelInList(newvalue, gt)); - - var->value = newvalue + 1; - var->func(); - return; - } - } - else - */ #define MINVAL 0 #define MAXVAL 1 if (var->PossibleValue[MINVAL].strvalue && !strcmp(var->PossibleValue[MINVAL].strvalue, "MIN")) diff --git a/src/d_netcmd.c b/src/d_netcmd.c index b2333902b..ab56fc296 100644 --- a/src/d_netcmd.c +++ b/src/d_netcmd.c @@ -4905,8 +4905,9 @@ static void Got_SetupVotecmd(UINT8 **cp, INT32 playernum) { INT32 i; UINT8 gt, secondgt; + INT16 tempvotelevels[4][2]; - if (playernum != serverplayer && !IsPlayerAdmin(playernum)) + if (playernum != serverplayer) // admin shouldn't be able to set up vote... { CONS_Alert(CONS_WARNING, M_GetText("Illegal vote setup received from %s\n"), player_names[playernum]); if (server) @@ -4919,13 +4920,20 @@ static void Got_SetupVotecmd(UINT8 **cp, INT32 playernum) for (i = 0; i < 4; i++) { - votelevels[i][0] = (UINT16)READUINT16(*cp); - votelevels[i][1] = gt; - if (!mapheaderinfo[votelevels[i][0]]) - P_AllocMapHeader(votelevels[i][0]); + tempvotelevels[i][0] = (UINT16)READUINT16(*cp); + tempvotelevels[i][1] = gt; + if (tempvotelevels[i][0] < nummapheaders && mapheaderinfo[tempvotelevels[i][0]]) + continue; + + if (server) + I_Error("Got_SetupVotecmd: Internal map ID %d not found (nummapheaders = %d)", tempvotelevels[i][0], nummapheaders); + CONS_Alert(CONS_WARNING, M_GetText("Vote setup with bad map ID %d received from %s\n"), tempvotelevels[i][0], player_names[playernum]); + return; } - votelevels[2][1] = secondgt; + tempvotelevels[2][1] = secondgt; + + memcpy(votelevels, tempvotelevels, sizeof(votelevels)); G_SetGamestate(GS_VOTING); Y_StartVote(); diff --git a/src/deh_soc.c b/src/deh_soc.c index 8d988968b..b060ae538 100644 --- a/src/deh_soc.c +++ b/src/deh_soc.c @@ -140,41 +140,39 @@ void clear_conditionsets(void) void clear_levels(void) { - INT16 i; - // This is potentially dangerous but if we're resetting these headers, // we may as well try to save some memory, right? - for (i = 0; i < NUMMAPS; ++i) + while (nummapheaders > 0) { - if (!mapheaderinfo[i]) + nummapheaders--; + + if (!mapheaderinfo[nummapheaders]) continue; - if (strcmp(mapheaderinfo[i]->lumpname, tutorialmap) == 0) // Sal: Is this needed...? + if (strcmp(mapheaderinfo[nummapheaders]->lumpname, tutorialmap) == 0) // Sal: Is this needed...? continue; // Custom map header info // (no need to set num to 0, we're freeing the entire header shortly) - Z_Free(mapheaderinfo[i]->customopts); + Z_Free(mapheaderinfo[nummapheaders]->customopts); - P_DeleteFlickies(i); - P_DeleteGrades(i); + P_DeleteFlickies(nummapheaders); + P_DeleteGrades(nummapheaders); - Patch_Free(mapheaderinfo[i]->thumbnailPic); - Patch_Free(mapheaderinfo[i]->minimapPic); - Z_Free(mapheaderinfo[i]->nextlevel); - Z_Free(mapheaderinfo[i]->marathonnext); + Patch_Free(mapheaderinfo[nummapheaders]->thumbnailPic); + Patch_Free(mapheaderinfo[nummapheaders]->minimapPic); + Z_Free(mapheaderinfo[nummapheaders]->nextlevel); + Z_Free(mapheaderinfo[nummapheaders]->marathonnext); - Z_Free(mapheaderinfo[i]->lumpname); + Z_Free(mapheaderinfo[nummapheaders]->lumpname); - Z_Free(mapheaderinfo[i]); - mapheaderinfo[i] = NULL; + Z_Free(mapheaderinfo[nummapheaders]); + mapheaderinfo[nummapheaders] = NULL; } - nummapheaders = 0; - - // Realloc the one for the current gamemap as a safeguard -- TODO: BAD + // Realloc the one for the current gamemap as a safeguard if (Playing()) - P_AllocMapHeader(gamemap-1); + COM_BufAddText("exitgame"); // Command_ExitGame_f() but delayed } // TODO: Figure out how to do undolines for this.... @@ -2610,7 +2608,7 @@ static void readcondition(UINT8 set, UINT32 id, char *word2) if (!params[0]) { - deh_warning("condition line is empty"); + deh_warning("condition line is empty for condition ID %d", id); return; } @@ -2630,7 +2628,7 @@ static void readcondition(UINT8 set, UINT32 id, char *word2) if (x1 < 0 || x1 >= PWRLV_NUMTYPES) { - deh_warning("Power level type %d out of range (0 - %d)", x1, PWRLV_NUMTYPES-1); + deh_warning("Power level type %d out of range (0 - %d) for condition ID %d", x1, PWRLV_NUMTYPES-1, id); return; } } @@ -2652,9 +2650,9 @@ static void readcondition(UINT8 set, UINT32 id, char *word2) ty = UC_MAPVISITED + offset; re = G_MapNumber(params[1]); - if (re < 0 || re >= NUMMAPS) + if (re == nummapheaders) { - deh_warning("Level number %d out of range (1 - %d)", re, NUMMAPS); + deh_warning("Invalid level %s for condition ID %d", params[1], id); return; } } @@ -2665,9 +2663,9 @@ static void readcondition(UINT8 set, UINT32 id, char *word2) re = atoi(params[2]); x1 = G_MapNumber(params[1]); - if (x1 < 0 || x1 >= NUMMAPS) + if (x1 == nummapheaders) { - deh_warning("Level number %d out of range (1 - %d)", x1, NUMMAPS); + deh_warning("Invalid level %s for condition ID %d", params[1], id); return; } } @@ -2680,7 +2678,7 @@ static void readcondition(UINT8 set, UINT32 id, char *word2) // constrained by 32 bits if (re < 0 || re > 31) { - deh_warning("Trigger ID %d out of range (0 - 31)", re); + deh_warning("Trigger ID %d out of range (0 - 31) for condition ID %d", re, id); return; } } @@ -2698,7 +2696,7 @@ static void readcondition(UINT8 set, UINT32 id, char *word2) if (re <= 0 || re > MAXEMBLEMS) { - deh_warning("Emblem %d out of range (1 - %d)", re, MAXEMBLEMS); + deh_warning("Emblem %d out of range (1 - %d) for condition ID %d", re, MAXEMBLEMS, id); return; } } @@ -2710,7 +2708,7 @@ static void readcondition(UINT8 set, UINT32 id, char *word2) if (re <= 0 || re > MAXEXTRAEMBLEMS) { - deh_warning("Extra emblem %d out of range (1 - %d)", re, MAXEXTRAEMBLEMS); + deh_warning("Extra emblem %d out of range (1 - %d) for condition ID %d", re, MAXEXTRAEMBLEMS, id); return; } } @@ -2722,13 +2720,13 @@ static void readcondition(UINT8 set, UINT32 id, char *word2) if (re <= 0 || re > MAXCONDITIONSETS) { - deh_warning("Condition set %d out of range (1 - %d)", re, MAXCONDITIONSETS); + deh_warning("Condition set %d out of range (1 - %d) for condition ID %d", re, MAXCONDITIONSETS, id); return; } } else { - deh_warning("Invalid condition name %s", params[0]); + deh_warning("Invalid condition name %s for condition ID %d", params[0], id); return; } diff --git a/src/g_demo.c b/src/g_demo.c index a0d7ccbd6..59b943d97 100644 --- a/src/g_demo.c +++ b/src/g_demo.c @@ -2918,7 +2918,7 @@ void G_DoPlayDemo(char *defdemoname) demo_p += 4; // Extrainfo location // ...*map* not loaded? - if (!gamemap || (gamemap > NUMMAPS) || !mapheaderinfo[gamemap-1] || mapheaderinfo[gamemap-1]->lumpnum == LUMPERROR) + if (!gamemap || (gamemap > nummapheaders) || !mapheaderinfo[gamemap-1] || mapheaderinfo[gamemap-1]->lumpnum == LUMPERROR) { snprintf(msg, 1024, M_GetText("%s features a course that is not currently loaded.\n"), pdemoname); CONS_Alert(CONS_ERROR, "%s", msg); diff --git a/src/g_game.c b/src/g_game.c index 85c2264e4..1cdcaaae4 100644 --- a/src/g_game.c +++ b/src/g_game.c @@ -3292,13 +3292,13 @@ UINT32 G_TOLFlag(INT32 pgametype) return gametypetol[pgametype]; } -static UINT32 TOLMaps(UINT32 tolflags) +static INT32 TOLMaps(UINT32 tolflags) { - UINT32 num = 0; - UINT32 i; + INT32 num = 0; + INT32 i; // Find all the maps that are ok and and put them in an array. - for (i = 0; i < NUMMAPS; i++) + for (i = 0; i < nummapheaders; i++) { if (!mapheaderinfo[i]) continue; @@ -3330,7 +3330,7 @@ INT16 G_RandMap(UINT32 tolflags, INT16 pprevmap, UINT8 ignorebuffer, UINT8 maphe if (!okmaps) { //CONS_Printf("(making okmaps)\n"); - okmaps = Z_Malloc(NUMMAPS * sizeof(INT16), PU_STATIC, NULL); + okmaps = Z_Malloc(nummapheaders * sizeof(INT16), PU_STATIC, NULL); } if (extbuffer != NULL) @@ -3347,7 +3347,7 @@ tryagain: usehellmaps = (maphell == 0 ? false : (maphell == 2 || M_RandomChance(FRACUNIT/100))); // 1% chance of Hell // Find all the maps that are ok and and put them in an array. - for (ix = 0; ix < NUMMAPS; ix++) + for (ix = 0; ix < nummapheaders; ix++) { boolean isokmap = true; @@ -3456,7 +3456,7 @@ tryagain: void G_AddMapToBuffer(INT16 map) { - INT16 bufx, refreshnum = max(0, ((INT32)TOLMaps(G_TOLFlag(gametype)))-3); + INT16 bufx, refreshnum = max(0, (TOLMaps(G_TOLFlag(gametype)))-3); // Add the map to the buffer. for (bufx = NUMMAPS-1; bufx > 0; bufx--) @@ -3665,7 +3665,7 @@ static void G_DoCompleted(void) { register INT16 cm = nextmap; UINT32 tolflag = G_TOLFlag(gametype); - UINT8 visitedmap[(NUMMAPS+7)/8]; + UINT8 visitedmap[(nummapheaders+7)/8]; memset(visitedmap, 0, sizeof (visitedmap)); @@ -3693,7 +3693,7 @@ static void G_DoCompleted(void) cm = (INT16)nextNum; } - if (cm >= NUMMAPS || cm < 0) // out of range (either 1100ish or error) + if (cm >= nummapheaders || cm < 0) // out of range (either 1100ish or error) { cm = nextmap; //Start the loop again so that the error checking below is executed. @@ -3722,7 +3722,7 @@ static void G_DoCompleted(void) if (nextmap >= 1100-1 && nextmap <= 1102-1 && !(gametyperules & GTR_CAMPAIGN)) nextmap = (INT16)(spstage_start-1); - if (nextmap < 0 || (nextmap >= NUMMAPS && nextmap < 1100-1) || nextmap > 1103-1) + if (nextmap < 0 || (nextmap >= nummapheaders && nextmap < 1100-1) || nextmap > 1103-1) I_Error("Followed map %d to invalid map %d\n", prevmap + 1, nextmap + 1); if (!spec) @@ -3747,7 +3747,7 @@ static void G_DoCompleted(void) // We may as well allocate its header if it doesn't exist // (That is, if it's a real map) if (nextmap < NUMMAPS && !mapheaderinfo[nextmap]) - P_AllocMapHeader(nextmap); + I_Error("G_DoCompleted: Internal map ID %d not found (nummapheaders = %d)\n", nextmap, nummapheaders); // Set up power level gametype scrambles K_SetPowerLevelScrambles(powertype); @@ -4614,11 +4614,6 @@ void G_InitNew(UINT8 pencoremode, INT32 map, boolean resetplayer, boolean skippr gamemap = map; - // gamemap changed; we assume that its map header is always valid, - // so make it so - if(!mapheaderinfo[gamemap-1]) - P_AllocMapHeader(gamemap-1); - maptol = mapheaderinfo[gamemap-1]->typeoflevel; globalweather = mapheaderinfo[gamemap-1]->weather; @@ -4655,11 +4650,8 @@ char *G_BuildMapTitle(INT32 mapnum) { char *title = NULL; - if (mapnum == 0) - return Z_StrDup("Random"); - - if (!mapheaderinfo[mapnum-1]) - P_AllocMapHeader(mapnum-1); + if (!mapnum || mapnum > nummapheaders || !mapheaderinfo[mapnum-1]) + I_Error("G_BuildMapTitle: Internal map ID %d not found (nummapheaders = %d)", mapnum-1, nummapheaders); if (strcmp(mapheaderinfo[mapnum-1]->lvlttl, "")) { @@ -4755,19 +4747,12 @@ INT32 G_FindMap(const char *mapname, char **foundmapnamep, mapnamelen = strlen(mapname); - /* Count available maps; how ugly. */ - for (i = 0, freqc = 0; i < NUMMAPS; ++i) - { - if (mapheaderinfo[i]) - freqc++; - } - - freq = ZZ_Calloc(freqc * sizeof (mapsearchfreq_t)); + freq = ZZ_Calloc(nummapheaders * sizeof (mapsearchfreq_t)); wanttable = !!( freqp ); freqc = 0; - for (i = 0, mapnum = 1; i < NUMMAPS; ++i, ++mapnum) + for (i = 0, mapnum = 1; i < nummapheaders; ++i, ++mapnum) if (mapheaderinfo[i]) { if (!( realmapname = G_BuildMapTitle(mapnum) )) diff --git a/src/k_menudraw.c b/src/k_menudraw.c index c51f9fdaa..54513b105 100644 --- a/src/k_menudraw.c +++ b/src/k_menudraw.c @@ -2117,10 +2117,10 @@ void M_DrawLevelSelect(void) { INT16 lvlx = t, lvly = y; - while (!M_CanShowLevelInList(map, levellist.newgametype) && map < NUMMAPS) + while (!M_CanShowLevelInList(map, levellist.newgametype) && map < nummapheaders) map++; - if (map >= NUMMAPS) + if (map >= nummapheaders) break; if (i == levellist.cursor && tatransition) diff --git a/src/k_menufunc.c b/src/k_menufunc.c index 0df40ab4c..f930b1dfa 100644 --- a/src/k_menufunc.c +++ b/src/k_menufunc.c @@ -3229,7 +3229,7 @@ INT16 M_CountLevelsToShowInList(UINT8 gt) { INT16 mapnum, count = 0; - for (mapnum = 0; mapnum < NUMMAPS; mapnum++) + for (mapnum = 0; mapnum < nummapheaders; mapnum++) if (M_CanShowLevelInList(mapnum, gt)) count++; @@ -3240,7 +3240,7 @@ INT16 M_GetFirstLevelInList(UINT8 gt) { INT16 mapnum; - for (mapnum = 0; mapnum < NUMMAPS; mapnum++) + for (mapnum = 0; mapnum < nummapheaders; mapnum++) if (M_CanShowLevelInList(mapnum, gt)) return mapnum; @@ -3549,16 +3549,16 @@ void M_LevelSelectHandler(INT32 choice) { map++; - while (!M_CanShowLevelInList(map, levellist.newgametype) && map < NUMMAPS) + while (!M_CanShowLevelInList(map, levellist.newgametype) && map < nummapheaders) map++; - if (map >= NUMMAPS) + if (map >= nummapheaders) break; add--; } - if (map >= NUMMAPS) + if (map >= nummapheaders) { // This shouldn't happen return; diff --git a/src/lua_baselib.c b/src/lua_baselib.c index 41768b3da..1219c7ba9 100644 --- a/src/lua_baselib.c +++ b/src/lua_baselib.c @@ -3046,12 +3046,12 @@ static int lib_gBuildMapTitle(lua_State *L) { INT32 map = Lcheckmapnumber(L, 1, "G_BuildMapTitle"); char *name; - if (map < 1 || map > NUMMAPS) + if (map < 1 || map > nummapheaders) { return luaL_error(L, - "map number %d out of range (1 - %d)", + "map ID %d out of range (1 - %d)", map, - NUMMAPS + nummapheaders ); } name = G_BuildMapTitle(map); diff --git a/src/lua_maplib.c b/src/lua_maplib.c index ec5f18127..863821a9b 100644 --- a/src/lua_maplib.c +++ b/src/lua_maplib.c @@ -2085,8 +2085,8 @@ static int lib_getMapheaderinfo(lua_State *L) lua_remove(L, 1); // dummy userdata table is unused. if (lua_isnumber(L, 1)) { - size_t i = lua_tointeger(L, 1)-1; - if (i >= NUMMAPS) + INT32 i = lua_tointeger(L, 1)-1; + if (i < 0 || i >= nummapheaders) return 0; LUA_PushUserdata(L, mapheaderinfo[i], META_MAPHEADER); //CONS_Printf(mapheaderinfo[i]->lvlttl); @@ -2104,7 +2104,7 @@ static int lib_getMapheaderinfo(lua_State *L) static int lib_nummapheaders(lua_State *L) { - lua_pushinteger(L, NUMMAPS); + lua_pushinteger(L, nummapheaders); return 1; } diff --git a/src/p_saveg.c b/src/p_saveg.c index b50175ee1..eaab7e899 100644 --- a/src/p_saveg.c +++ b/src/p_saveg.c @@ -4396,8 +4396,8 @@ static inline void P_UnArchiveSPGame(INT16 mapoverride) // gamemap changed; we assume that its map header is always valid, // so make it so - if(!mapheaderinfo[gamemap-1]) - P_AllocMapHeader(gamemap-1); + if (!gamemap || gamemap > nummapheaders || !mapheaderinfo[gamemap-1]) + I_Error("P_UnArchiveSPGame: Internal map ID %d not found (nummapheaders = %d)", gamemap-1, nummapheaders); //lastmapsaved = gamemap; lastmaploaded = gamemap; @@ -4568,8 +4568,8 @@ static inline boolean P_NetUnArchiveMisc(boolean reloading) // gamemap changed; we assume that its map header is always valid, // so make it so - if(!mapheaderinfo[gamemap-1]) - P_AllocMapHeader(gamemap-1); + if (!gamemap || gamemap > nummapheaders || !mapheaderinfo[gamemap-1]) + I_Error("P_NetUnArchiveMisc: Internal map ID %d not found (nummapheaders = %d)", gamemap-1, nummapheaders); // tell the sound code to reset the music since we're skipping what // normally sets this flag diff --git a/src/y_inter.c b/src/y_inter.c index 61ebc20d5..ac6767f35 100644 --- a/src/y_inter.c +++ b/src/y_inter.c @@ -833,8 +833,8 @@ void Y_StartIntermission(void) //if (dedicated) return; // This should always exist, but just in case... - if (!mapheaderinfo[prevmap]) - P_AllocMapHeader(prevmap); + if (prevmap >= nummapheaders || !mapheaderinfo[prevmap]) + I_Error("Y_StartIntermission: Internal map ID %d not found (nummapheaders = %d)", prevmap, nummapheaders); switch (intertype) {