From b92c1259281dcae5e2440488e56de7dc89a5fbc2 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 | 35 +--------------------------- src/d_netcmd.c | 20 +++++++++++----- src/deh_soc.c | 58 +++++++++++++++++++++++------------------------ src/g_game.c | 32 ++++++++++---------------- src/lua_baselib.c | 6 ++--- src/lua_maplib.c | 6 ++--- src/m_menu.c | 4 ++-- src/p_saveg.c | 8 +++---- src/y_inter.c | 4 ++-- 9 files changed, 69 insertions(+), 104 deletions(-) diff --git a/src/command.c b/src/command.c index 29039d574..9a29966f9 100644 --- a/src/command.c +++ b/src/command.c @@ -1992,42 +1992,9 @@ 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; - } - } #define MINVAL 0 #define MAXVAL 1 - else if (var->PossibleValue[MINVAL].strvalue && !strcmp(var->PossibleValue[MINVAL].strvalue, "MIN")) + if (var->PossibleValue[MINVAL].strvalue && !strcmp(var->PossibleValue[MINVAL].strvalue, "MIN")) { #ifdef PARANOIA if (!var->PossibleValue[MAXVAL].strvalue) diff --git a/src/d_netcmd.c b/src/d_netcmd.c index dbadef71f..149c8835e 100644 --- a/src/d_netcmd.c +++ b/src/d_netcmd.c @@ -5384,8 +5384,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) @@ -5398,13 +5399,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 923e7b448..0e94eea05 100644 --- a/src/deh_soc.c +++ b/src/deh_soc.c @@ -142,41 +142,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 } static boolean findFreeSlot(INT32 *num) @@ -2827,7 +2825,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; } @@ -2847,7 +2845,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; } } @@ -2869,9 +2867,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; } } @@ -2882,9 +2880,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; } } @@ -2897,7 +2895,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; } } @@ -2915,7 +2913,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; } } @@ -2927,7 +2925,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; } } @@ -2939,13 +2937,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_game.c b/src/g_game.c index b835074de..2e0716c20 100644 --- a/src/g_game.c +++ b/src/g_game.c @@ -3455,13 +3455,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; @@ -3493,7 +3493,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) @@ -3637,7 +3637,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--) @@ -3846,7 +3846,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)); @@ -3874,7 +3874,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. @@ -3899,7 +3899,7 @@ static void G_DoCompleted(void) nextmap = cm; } - 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) @@ -3924,7 +3924,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); @@ -4772,11 +4772,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; @@ -4813,11 +4808,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, "")) { diff --git a/src/lua_baselib.c b/src/lua_baselib.c index 0ff37d383..dd1a8c04a 100644 --- a/src/lua_baselib.c +++ b/src/lua_baselib.c @@ -3165,12 +3165,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 1391ad98d..e1bc3892e 100644 --- a/src/lua_maplib.c +++ b/src/lua_maplib.c @@ -2468,8 +2468,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); @@ -2487,7 +2487,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/m_menu.c b/src/m_menu.c index 2bae68dc5..d1dd737a9 100644 --- a/src/m_menu.c +++ b/src/m_menu.c @@ -4535,7 +4535,7 @@ static INT32 M_CountLevelsToShowInList(void) { INT32 mapnum, count = 0; - for (mapnum = 0; mapnum < NUMMAPS; mapnum++) + for (mapnum = 0; mapnum < nummapheaders; mapnum++) if (M_CanShowLevelInList(mapnum, -1)) count++; @@ -4546,7 +4546,7 @@ static INT32 M_GetFirstLevelInList(void) { INT32 mapnum; - for (mapnum = 0; mapnum < NUMMAPS; mapnum++) + for (mapnum = 0; mapnum < nummapheaders; mapnum++) if (M_CanShowLevelInList(mapnum, -1)) return mapnum + 1; diff --git a/src/p_saveg.c b/src/p_saveg.c index 859418d99..0efd5a250 100644 --- a/src/p_saveg.c +++ b/src/p_saveg.c @@ -4888,8 +4888,8 @@ static inline void P_UnArchiveSPGame(savebuffer_t *save, 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; @@ -5077,8 +5077,8 @@ FUNCINLINE static ATTRINLINE boolean P_NetUnArchiveMisc(savebuffer_t *save, bool // 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 a25e3ba7c..db7f30f64 100644 --- a/src/y_inter.c +++ b/src/y_inter.c @@ -1035,8 +1035,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) {