From b2e09b5a18fe48258d83dc82d6ed5afca193a3e9 Mon Sep 17 00:00:00 2001
From: helixhorned <helixhorned@1a8010ca-5511-0410-912e-c29ae57300e0>
Date: Fri, 26 Dec 2014 17:29:56 +0000
Subject: [PATCH] In C-CON build, prevent leak of gamevar/gamearray labels on
 savegame loading.

Also, rewrite the loops in Gv_Free() and Gv_Clear() in the plain fashion so
that they are correct even if the number of gamearrays exceeds the number of
gamevars.

git-svn-id: https://svn.eduke32.com/eduke32@4839 1a8010ca-5511-0410-912e-c29ae57300e0
---
 polymer/eduke32/source/gamevars.c | 76 +++++++++++++++++++------------
 polymer/eduke32/source/gamevars.h |  2 +-
 polymer/eduke32/source/m32vars.c  |  8 +---
 3 files changed, 50 insertions(+), 36 deletions(-)

diff --git a/polymer/eduke32/source/gamevars.c b/polymer/eduke32/source/gamevars.c
index b4a7c349d..f633343d1 100644
--- a/polymer/eduke32/source/gamevars.c
+++ b/polymer/eduke32/source/gamevars.c
@@ -36,51 +36,52 @@ LUNATIC_CB void (*A_ResetVars)(int32_t iActor);
 #else
 # include "gamestructures.c"
 
-static void Gv_Free(void) /* called from Gv_ReadSave() and Gv_ResetVars() */
+// Frees the memory for the *values* of game variables and arrays. Resets their
+// counts to zero. Call this function as many times as needed.
+//
+// Returns: old g_gameVarCount | (g_gameArrayCount<<16).
+static int32_t Gv_Free(void)
 {
-    // call this function as many times as needed.
-    int32_t i;
-
-    for (i=0; i<g_gameVarCount; i++)
+    for (int32_t i=0; i<g_gameVarCount; i++)
     {
         if (aGameVars[i].dwFlags & GAMEVAR_USER_MASK)
             ALIGNED_FREE_AND_NULL(aGameVars[i].val.plValues);
 
         aGameVars[i].dwFlags |= GAMEVAR_RESET;
+    }
 
-        if (i >= MAXGAMEARRAYS)
-            continue;
-
+    for (int32_t i=0; i<g_gameArrayCount; i++)
+    {
         if (aGameArrays[i].dwFlags & GAMEARRAY_NORMAL)
             ALIGNED_FREE_AND_NULL(aGameArrays[i].plValues);
 
         aGameArrays[i].dwFlags |= GAMEARRAY_RESET;
     }
 
+    EDUKE32_STATIC_ASSERT(MAXGAMEVARS < 32768);
+    int32_t ret = g_gameVarCount | (g_gameArrayCount<<16);
     g_gameVarCount = g_gameArrayCount = 0;
+
     hash_init(&h_gamevars);
     hash_init(&h_arrays);
+
+    return ret;
 }
 
+// Calls Gv_Free() and in addition frees the labels of all game variables and
+// arrays.
+// Only call this function ONCE...
 static void Gv_Clear(void)
 {
-    // only call this function ONCE...
-    int32_t i;
-
-    Gv_Free();
+    int32_t n = Gv_Free();
+    int32_t gameVarCount = n&65535, gameArrayCount = n>>16;
 
     // Now, only do work that Gv_Free() hasn't done.
-    for (i=0; i<g_gameVarCount; i++)
-    {
+    for (int32_t i=0; i<gameVarCount; i++)
         DO_FREE_AND_NULL(aGameVars[i].szLabel);
 
-        aGameVars[i].val.lValue=0;
-
-        if (i >= MAXGAMEARRAYS)
-            continue;
-
+    for (int32_t i=0; i<gameArrayCount; i++)
         DO_FREE_AND_NULL(aGameArrays[i].szLabel);
-    }
 }
 
 int32_t Gv_ReadSave(int32_t fil, int32_t newbehav)
@@ -107,9 +108,18 @@ int32_t Gv_ReadSave(int32_t fil, int32_t newbehav)
     if (kdfread(&g_gameVarCount,sizeof(g_gameVarCount),1,fil) != 1) goto corrupt;
     for (i=0; i<g_gameVarCount; i++)
     {
-        if (kdfread(&(aGameVars[i]),sizeof(gamevar_t),1,fil) != 1) goto corrupt;
-        aGameVars[i].szLabel = (char *)Xmalloc(MAXVARLABEL * sizeof(uint8_t));
-        if (kdfread(aGameVars[i].szLabel,sizeof(uint8_t) * MAXVARLABEL, 1, fil) != 1) goto corrupt;
+        char *const olabel = aGameVars[i].szLabel;
+
+        if (kdfread(&aGameVars[i], sizeof(gamevar_t), 1, fil) != 1)
+            goto corrupt;
+
+        if (olabel == NULL)
+            aGameVars[i].szLabel = (char *)Xmalloc(MAXVARLABEL * sizeof(uint8_t));
+        else
+            aGameVars[i].szLabel = olabel;
+
+        if (kdfread(aGameVars[i].szLabel, MAXVARLABEL, 1, fil) != 1)
+            goto corrupt;
         hash_add(&h_gamevars, aGameVars[i].szLabel,i, 1);
 
         if (aGameVars[i].dwFlags & GAMEVAR_PERPLAYER)
@@ -138,11 +148,19 @@ int32_t Gv_ReadSave(int32_t fil, int32_t newbehav)
         if (aGameArrays[i].dwFlags&GAMEARRAY_READONLY)
             continue;
 
-        // read for .size and .dwFlags (the rest are pointers):
-        if (kdfread(&aGameArrays[i],sizeof(gamearray_t),1,fil) != 1) goto corrupt;
+        char *const olabel = aGameArrays[i].szLabel;
 
-        aGameArrays[i].szLabel = (char *)Xmalloc(MAXARRAYLABEL * sizeof(uint8_t));
-        if (kdfread(aGameArrays[i].szLabel,sizeof(uint8_t) * MAXARRAYLABEL, 1, fil) != 1) goto corrupt;
+        // read for .size and .dwFlags (the rest are pointers):
+        if (kdfread(&aGameArrays[i], sizeof(gamearray_t), 1, fil) != 1)
+            goto corrupt;
+
+        if (olabel == NULL)
+            aGameArrays[i].szLabel = (char *)Xmalloc(MAXARRAYLABEL * sizeof(uint8_t));
+        else
+            aGameArrays[i].szLabel = olabel;
+
+        if (kdfread(aGameArrays[i].szLabel,sizeof(uint8_t) * MAXARRAYLABEL, 1, fil) != 1)
+            goto corrupt;
         hash_add(&h_arrays, aGameArrays[i].szLabel, i, 1);
 
         aGameArrays[i].plValues = (intptr_t *)Xaligned_alloc(ACTOR_VAR_ALIGNMENT, aGameArrays[i].size * GAR_ELTSZ);
@@ -417,7 +435,7 @@ int32_t Gv_NewArray(const char *pszLabel, void *arrayptr, intptr_t asize, uint32
     i = g_gameArrayCount;
 
     if (aGameArrays[i].szLabel == NULL)
-        aGameArrays[i].szLabel=(char *)Xcalloc(MAXVARLABEL,sizeof(uint8_t));
+        aGameArrays[i].szLabel = (char *)Xcalloc(MAXVARLABEL,sizeof(uint8_t));
 
     if (aGameArrays[i].szLabel != pszLabel)
         Bstrcpy(aGameArrays[i].szLabel,pszLabel);
@@ -808,7 +826,7 @@ nastyhacks:
         if (EDUKE32_PREDICT_FALSE(index < 0 || index >= siz))
         {
             negateResult = index;
-            CON_ERRPRINTF("%s %s[%d]\n", gvxerrs[GVX_BADINDEX], aGameArrays[id].szLabel, index);;
+            CON_ERRPRINTF("%s %s[%d]\n", gvxerrs[GVX_BADINDEX], aGameArrays[id].szLabel, index);
             return -1;
         }
 
diff --git a/polymer/eduke32/source/gamevars.h b/polymer/eduke32/source/gamevars.h
index 695f342ec..8f42ed1d4 100644
--- a/polymer/eduke32/source/gamevars.h
+++ b/polymer/eduke32/source/gamevars.h
@@ -54,7 +54,7 @@ enum GamevarFlags_t {
 #define PLAYER_VAR_ALIGNMENT (sizeof(intptr_t))
 #define ACTOR_VAR_ALIGNMENT 16
 
-# define MAXGAMEARRAYS (MAXGAMEVARS>>2) // must be lower than MAXGAMEVARS
+# define MAXGAMEARRAYS (MAXGAMEVARS>>2) // must be strictly smaller than MAXGAMEVARS
 # define MAXARRAYLABEL MAXVARLABEL
 
 enum GamearrayFlags_t {
diff --git a/polymer/eduke32/source/m32vars.c b/polymer/eduke32/source/m32vars.c
index a0b04abaa..14fc23283 100644
--- a/polymer/eduke32/source/m32vars.c
+++ b/polymer/eduke32/source/m32vars.c
@@ -40,10 +40,7 @@ static void Gv_Clear(void)
 
     for (; i>=0; i--)
     {
-        if (aGameVars[i].szLabel)
-            Bfree(aGameVars[i].szLabel);
-
-        aGameVars[i].szLabel = NULL;
+        DO_FREE_AND_NULL(aGameVars[i].szLabel);
 
         if ((aGameVars[i].dwFlags & GAMEVAR_USER_MASK) && aGameVars[i].val.plValues)
         {
@@ -59,8 +56,7 @@ static void Gv_Clear(void)
 
         gamearray_t *const gar = &aGameArrays[i];
 
-        Bfree(gar->szLabel);
-        gar->szLabel = NULL;
+        DO_FREE_AND_NULL(gar->szLabel);
 
         if ((gar->dwFlags & GAMEARRAY_NORMAL) && gar->vals)
         {