shithub: choc

Download patch

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;
 }
 
 
-- 
⑨