ref: 3cf762118e2817f8e5ce59cc408bb8c876f5ba48
parent: 75bedca7fb08240bc28a6cd6f7f0604231b5a403
author: Simon Howard <fraggle@soulsphere.org>
date: Sat Sep 2 15:25:10 EDT 2017
Fix warnings about P_InitSwitchList() overruns. This function is weirdly written and the logic is hard to follow, so clean it up. This resolves warnings about array index overruns surfaced by cppcheck and blocking #939. Thanks @turol for discovering.
--- a/src/doom/p_switch.c
+++ b/src/doom/p_switch.c
@@ -61,7 +61,7 @@
{"SW1STON2", "SW2STON2", 1}, {"SW1STONE", "SW2STONE", 1}, {"SW1STRTN", "SW2STRTN", 1},-
+
// Doom registered episodes 2&3 switches
{"SW1BLUE", "SW2BLUE", 2}, {"SW1CMT", "SW2CMT", 2},@@ -73,7 +73,7 @@
{"SW1SKIN", "SW2SKIN", 2}, {"SW1VINE", "SW2VINE", 2}, {"SW1WOOD", "SW2WOOD", 2},-
+
// Doom II switches
{"SW1PANEL", "SW2PANEL", 3}, {"SW1ROCK", "SW2ROCK", 3},@@ -86,8 +86,6 @@
{"SW1TEK", "SW2TEK", 3}, {"SW1MARB", "SW2MARB", 3}, {"SW1SKULL", "SW2SKULL", 3},-
- {"\0", "\0", 0}};
int switchlist[MAXSWITCHES * 2];
@@ -100,45 +98,40 @@
//
void P_InitSwitchList(void)
{- int i;
- int index;
- int episode;
-
- episode = 1;
+ int i, slindex, episode;
- if (gamemode == registered || gamemode == retail)
- episode = 2;
- else
- if ( gamemode == commercial )
- episode = 3;
-
- for (index = 0,i = 0;i < MAXSWITCHES;i++)
+ // Note that this is called "episode" here but it's actually something
+ // quite different. As we progress from Shareware->Registered->Doom II
+ // we support more switch textures.
+ switch (gamemode)
{- if (!alphSwitchList[i].episode)
- {- numswitches = index/2;
- switchlist[index] = -1;
- break;
- }
-
+ case registered:
+ case retail:
+ episode = 2;
+ break;
+ case commercial:
+ episode = 3;
+ break;
+ default:
+ episode = 1;
+ break;
+ }
+
+ slindex = 0;
+
+ for (i = 0; i < arrlen(alphSwitchList); i++)
+ {if (alphSwitchList[i].episode <= episode)
{-#if 0 // UNUSED - debug?
- int value;
-
- if (R_CheckTextureNumForName(alphSwitchList[i].name1) < 0)
- {- I_Error("Can't find switch texture '%s'!",- alphSwitchList[i].name1);
- continue;
- }
-
- value = R_TextureNumForName(alphSwitchList[i].name1);
-#endif
- switchlist[index++] = R_TextureNumForName(DEH_String(alphSwitchList[i].name1));
- switchlist[index++] = R_TextureNumForName(DEH_String(alphSwitchList[i].name2));
+ switchlist[slindex++] =
+ R_TextureNumForName(DEH_String(alphSwitchList[i].name1));
+ switchlist[slindex++] =
+ R_TextureNumForName(DEH_String(alphSwitchList[i].name2));
}
}
+
+ numswitches = slindex / 2;
+ switchlist[slindex] = -1;
}
--- a/src/heretic/p_switch.c
+++ b/src/heretic/p_switch.c
@@ -102,31 +102,35 @@
void P_InitSwitchList(void)
{- int i;
- int index;
- int episode;
+ int i, slindex, episode;
- episode = 1;
- if (gamemode != shareware)
+ // Note that this is called "episode" here but it's actually something
+ // quite different. As we progress from Shareware->Registered->Doom II
+ // we support more switch textures.
+ if (gamemode == shareware)
+ {+ episode = 1;
+ }
+ else
+ {episode = 2;
+ }
- for (index = 0, i = 0; i < MAXSWITCHES; i++)
- {- if (!alphSwitchList[i].episode)
- {- numswitches = index / 2;
- switchlist[index] = -1;
- break;
- }
+ slindex = 0;
- if (alphSwitchList[i].episode <= episode)
- {- switchlist[index++] =
+ for (i = 0; i < arrlen(alphSwitchList); i++)
+ {+ if (alphSwitchList[i].episode <= episode)
+ {+ switchlist[slindex++] =
R_TextureNumForName(DEH_String(alphSwitchList[i].name1));
- switchlist[index++] =
+ switchlist[slindex++] =
R_TextureNumForName(DEH_String(alphSwitchList[i].name2));
- }
+ }
}
+
+ numswitches = slindex / 2;
+ switchlist[slindex] = -1;
}
//==================================================================
--- a/src/strife/p_switch.c
+++ b/src/strife/p_switch.c
@@ -117,7 +117,6 @@
{ "SWITE03", "SWITE04", 2, sfx_None }, { "SWTELP01", "SWTELP02", 2, sfx_None }, { "BRNSCN01", "BRNSCN05", 2, sfx_firxpl },- { "\0", "\0", 0, sfx_None }};
int switchlist[MAXSWITCHES * 2];
@@ -130,34 +129,35 @@
//
void P_InitSwitchList(void)
{- int i;
- int index;
- int episode;
-
- episode = 1;
+ int i, slindex, episode;
- if(isregistered)
+ // Note that this is called "episode" here but it's actually something
+ // quite different. As we progress from Shareware->Registered->Doom II
+ // we support more switch textures.
+ if (isregistered)
+ {episode = 2;
- // villsa [STRIFE] unused
- /*else
- if ( gamemode == commercial )
- episode = 3;*/
-
- for(index = 0, i = 0; i < MAXSWITCHES; i++)
+ }
+ else
{- if(!alphSwitchList[i].episode)
- {- numswitches = index/2;
- switchlist[index] = -1;
- break;
- }
-
+ episode = 1;
+ }
+
+ slindex = 0;
+
+ for (i = 0; i < arrlen(alphSwitchList); i++)
+ {if (alphSwitchList[i].episode <= episode)
{- switchlist[index++] = R_TextureNumForName(DEH_String(alphSwitchList[i].name1));
- switchlist[index++] = R_TextureNumForName(DEH_String(alphSwitchList[i].name2));
+ switchlist[slindex++] =
+ R_TextureNumForName(DEH_String(alphSwitchList[i].name1));
+ switchlist[slindex++] =
+ R_TextureNumForName(DEH_String(alphSwitchList[i].name2));
}
}
+
+ numswitches = slindex / 2;
+ switchlist[slindex] = -1;
}