Replace all free calls with VVV_free[func]

This replaces all calls to SDL_free with a new macro, VVV_free, that
nulls the pointer afterwards. This mitigates any use-after-frees and
also completely eliminates double-frees. The same is done for any
function to free specific objects such as SDL_FreeSurface, with the
VVV_freefunc macro.

No exceptions for any of these calls, even if the pointer is discarded
or zeroed afterwards anyway. Better safe than sorry.

This is a macro rather than a function that takes in a
pointer-to-pointer because such a function would have type issues that
require casting and that's just not safe.

Even though SDL_free and other SDL functions already check for NULL, the
macro has a NULL check for other functions that don't. For example,
FAudioVoice_DestroyVoice does not check for NULL.

FILESYSTEM_freeMemory has been axed in favor of VVV_free because it
functionally does the same thing except for `unsigned char*` only.

desktop_version/src/Alloc.h

@@ -0,0 +1,19 @@
+#ifndef ALLOCGAME_H
+#define ALLOCGAME_H
+
+/* TODO: VVV_malloc, VVV_realloc, etc. */
+
+#define VVV_freefunc(func, obj) \
+    do \
+    { \
+        if (obj != NULL) \
+        { \
+            func(obj); \
+            obj = NULL; \
+        } \
+    } \
+    while (0)
+
+#define VVV_free(obj) VVV_freefunc(SDL_free, obj)
+
+#endif /* ALLOCGAME_H */

desktop_version/src/BinaryBlob.cpp

@@ -5,6 +5,7 @@
 #include <stdio.h>
 #endif
 
+#include "Alloc.h"
 #include "Exit.h"
 #include "FileSystemUtils.h"
 #include "UtilityClass.h"
@@ -91,7 +92,7 @@ void binaryBlob::clear(void)
     {
         if (m_memblocks[i] != NULL)
         {
-            SDL_free(m_memblocks[i]);
+            VVV_free(m_memblocks[i]);
         }
     }
     SDL_zeroa(m_memblocks);

desktop_version/src/CWrappers.cpp

@@ -5,7 +5,7 @@
 extern "C" char* HELP_number_words(int _t)
 {
     /* C wrapper for UtilityClass::number_words.
-     * Caller must SDL_free. */
+     * Caller must VVV_free. */
 
     std::string str = help.number_words(_t);
 

desktop_version/src/CustomLevels.cpp

@@ -9,6 +9,7 @@
 #include <tinyxml2.h>
 #include <utf8/unchecked.h>
 
+#include "Alloc.h"
 #include "Constants.h"
 #include "Editor.h"
 #include "Enums.h"
@@ -267,7 +268,7 @@ bool customlevelclass::getLevelMetaDataAndPlaytestArgs(const std::string& _path,
 
     std::string buf((char*) uMem);
 
-    FILESYSTEM_freeMemory(&uMem);
+    VVV_free(uMem);
 
     if (find_metadata(buf) == "")
     {

desktop_version/src/FileSystemUtils.cpp

@@ -4,6 +4,7 @@
 #include <stdio.h>
 #include <tinyxml2.h>
 
+#include "Alloc.h"
 #include "BinaryBlob.h"
 #include "Exit.h"
 #include "Graphics.h"
@@ -201,13 +202,8 @@ void FILESYSTEM_deinit(void)
     {
         PHYSFS_deinit();
     }
-    if (stdin_buffer != NULL)
+    VVV_free(stdin_buffer);
-    {
+    VVV_free(basePath);
-        SDL_free(stdin_buffer);
-        stdin_buffer = NULL;
-    }
-    SDL_free(basePath);
-    basePath = NULL;
     isInit = false;
 }
 
@@ -505,8 +501,6 @@ bool FILESYSTEM_isAssetMounted(const char* filename)
     return SDL_strcmp(assetDir, realDir) == 0;
 }
 
-void FILESYSTEM_freeMemory(unsigned char **mem);
-
 static void load_stdin(void)
 {
     size_t pos = 0;
@@ -636,7 +630,7 @@ void FILESYSTEM_loadFileToMemory(
     success = PHYSFS_readBytes(handle, *mem, length);
     if (success == -1)
     {
-        FILESYSTEM_freeMemory(mem);
+        VVV_free(*mem);
     }
     PHYSFS_close(handle);
     return;
@@ -665,12 +659,6 @@ void FILESYSTEM_loadAssetToMemory(
     FILESYSTEM_loadFileToMemory(path, mem, len, addnull);
 }
 
-void FILESYSTEM_freeMemory(unsigned char **mem)
-{
-    SDL_free(*mem);
-    *mem = NULL;
-}
-
 bool FILESYSTEM_loadBinaryBlob(binaryBlob* blob, const char* filename)
 {
     PHYSFS_sint64 size;
@@ -817,7 +805,7 @@ bool FILESYSTEM_loadTiXml2Document(const char *name, tinyxml2::XMLDocument& doc)
         return false;
     }
     doc.Parse((const char*) mem);
-    FILESYSTEM_freeMemory(&mem);
+    VVV_free(mem);
     return true;
 }
 

desktop_version/src/FileSystemUtils.h

@@ -32,7 +32,6 @@ void FILESYSTEM_loadAssetToMemory(
     size_t* len,
     const bool addnull
 );
-void FILESYSTEM_freeMemory(unsigned char **mem);
 
 bool FILESYSTEM_loadBinaryBlob(binaryBlob* blob, const char* filename);
 

desktop_version/src/Graphics.cpp

@@ -4,6 +4,7 @@
 #include <SDL.h>
 #include <utf8/unchecked.h>
 
+#include "Alloc.h"
 #include "Constants.h"
 #include "CustomLevels.h"
 #include "Entity.h"
@@ -158,7 +159,7 @@ void Graphics::destroy(void)
     #define CLEAR_ARRAY(name) \
         for (size_t i = 0; i < name.size(); i += 1) \
         { \
-            SDL_FreeSurface(name[i]); \
+            VVV_freefunc(SDL_FreeSurface, name[i]); \
         } \
         name.clear();
 
@@ -228,22 +229,20 @@ void Graphics::create_buffers(const SDL_PixelFormat* fmt)
 
 void Graphics::destroy_buffers(void)
 {
-#define FREE_SURFACE(SURFACE) \
+#define FREE_SURFACE(SURFACE) VVV_freefunc(SDL_FreeSurface, SURFACE)
-    SDL_FreeSurface(SURFACE); \
-    SURFACE = NULL;
 
-    FREE_SURFACE(backBuffer)
+    FREE_SURFACE(backBuffer);
-    FREE_SURFACE(footerbuffer)
+    FREE_SURFACE(footerbuffer);
-    FREE_SURFACE(ghostbuffer)
+    FREE_SURFACE(ghostbuffer);
-    FREE_SURFACE(foregroundBuffer)
+    FREE_SURFACE(foregroundBuffer);
-    FREE_SURFACE(menubuffer)
+    FREE_SURFACE(menubuffer);
-    FREE_SURFACE(warpbuffer)
+    FREE_SURFACE(warpbuffer);
-    FREE_SURFACE(warpbuffer_lerp)
+    FREE_SURFACE(warpbuffer_lerp);
-    FREE_SURFACE(towerbg.buffer)
+    FREE_SURFACE(towerbg.buffer);
-    FREE_SURFACE(towerbg.buffer_lerp)
+    FREE_SURFACE(towerbg.buffer_lerp);
-    FREE_SURFACE(titlebg.buffer)
+    FREE_SURFACE(titlebg.buffer);
-    FREE_SURFACE(titlebg.buffer_lerp)
+    FREE_SURFACE(titlebg.buffer_lerp);
-    FREE_SURFACE(tempBuffer)
+    FREE_SURFACE(tempBuffer);
 
 #undef FREE_SURFACE
 }
@@ -347,8 +346,7 @@ void Graphics::updatetitlecolours(void)
             } \
         } \
         \
-        SDL_FreeSurface(grphx.im_##tilesheet); \
+        VVV_freefunc(SDL_FreeSurface, grphx.im_##tilesheet); \
-        grphx.im_##tilesheet = NULL; \
     }
 
 #define PROCESS_TILESHEET(tilesheet, tile_square, extra_code) \
@@ -376,7 +374,7 @@ bool Graphics::Makebfont(void)
             font_positions[codepoint] = pos;
             ++pos;
         }
-        FILESYSTEM_freeMemory(&charmap);
+        VVV_free(charmap);
     }
     else
     {
@@ -502,7 +500,7 @@ static void print_char(
 
     if (scale > 1)
     {
-        SDL_FreeSurface(surface);
+        VVV_freefunc(SDL_FreeSurface, surface);
     }
 }
 
@@ -2179,7 +2177,7 @@ void Graphics::drawentity(const int i, const int yoff)
         setRect(drawRect, xp, yp - yoff, sprites_rect.x * 6, sprites_rect.y * 6);
         SDL_Surface* TempSurface = ScaleSurface( spritesvec[obj.entities[i].drawframe], 6 * sprites_rect.w,6* sprites_rect.h );
         BlitSurfaceColoured(TempSurface, NULL , backBuffer,  &drawRect, ct );
-        SDL_FreeSurface(TempSurface);
+        VVV_freefunc(SDL_FreeSurface, TempSurface);
 
 
 

desktop_version/src/GraphicsResources.cpp

@@ -1,5 +1,6 @@
 #include "GraphicsResources.h"
 
+#include "Alloc.h"
 #include "FileSystemUtils.h"
 #include "Vlogging.h"
 
@@ -37,7 +38,7 @@ SDL_Surface* LoadImage(const char *filename)
         return NULL;
     }
     error = lodepng_decode32(&data, &width, &height, fileIn, length);
-    FILESYSTEM_freeMemory(&fileIn);
+    VVV_free(fileIn);
 
     if (error != 0)
     {
@@ -61,14 +62,14 @@ SDL_Surface* LoadImage(const char *filename)
             SDL_PIXELFORMAT_ARGB8888,
             0
         );
-        SDL_FreeSurface( loadedImage );
+        VVV_freefunc(SDL_FreeSurface, loadedImage);
-        SDL_free(data);
+        VVV_free(data);
         SDL_SetSurfaceBlendMode(optimizedImage, SDL_BLENDMODE_BLEND);
         return optimizedImage;
     }
     else
     {
-        SDL_free(data);
+        VVV_free(data);
         vlog_error("Image not found: %s", filename);
         SDL_assert(0 && "Image not found! See stderr.");
         return NULL;
@@ -104,10 +105,7 @@ void GraphicsResources::init(void)
 
 void GraphicsResources::destroy(void)
 {
-#define CLEAR(img) \
+#define CLEAR(img) VVV_freefunc(SDL_FreeSurface, img)
-    SDL_FreeSurface(img); \
-    img = NULL;
-
     CLEAR(im_tiles);
     CLEAR(im_tiles2);
     CLEAR(im_tiles3);

desktop_version/src/GraphicsUtil.cpp

@@ -4,6 +4,7 @@
 #include <stddef.h>
 #include <stdlib.h>
 
+#include "Alloc.h"
 #include "Graphics.h"
 #include "Maths.h"
 
@@ -226,7 +227,7 @@ void BlitSurfaceColoured(
     }
 
     SDL_BlitSurface(tempsurface, _srcRect, _dest, tempRect);
-    SDL_FreeSurface(tempsurface);
+    VVV_freefunc(SDL_FreeSurface, tempsurface);
 }
 
 void BlitSurfaceTinted(
@@ -287,7 +288,7 @@ void BlitSurfaceTinted(
     }
 
     SDL_BlitSurface(tempsurface, _srcRect, _dest, tempRect);
-    SDL_FreeSurface(tempsurface);
+    VVV_freefunc(SDL_FreeSurface, tempsurface);
 }
 
 
@@ -489,7 +490,7 @@ void ScrollSurface( SDL_Surface* _src, int _pX, int _pY )
     //Cleanup temp surface
     if (part1)
     {
-        SDL_FreeSurface(part1);
+        VVV_freefunc(SDL_FreeSurface, part1);
     }
 
 }

desktop_version/src/KeyPoll.cpp

@@ -4,6 +4,7 @@
 #include <string.h>
 #include <utf8/unchecked.h>
 
+#include "Alloc.h"
 #include "Exit.h"
 #include "Game.h"
 #include "GlitchrunnerMode.h"
@@ -183,7 +184,7 @@ void KeyPoll::Poll(void)
                     if (text != NULL)
                     {
                         keybuffer += text;
-                        SDL_free(text);
+                        VVV_free(text);
                     }
                 }
             }

desktop_version/src/Music.cpp

@@ -5,6 +5,7 @@
 #include <FAudio.h>
 #include <physfsrwops.h>
 
+#include "Alloc.h"
 #include "BinaryBlob.h"
 #include "FileSystemUtils.h"
 #include "Game.h"
@@ -21,7 +22,7 @@
 
 #define malloc SDL_malloc
 #define realloc SDL_realloc
-#define free SDL_free
+#define free VVV_free
 #ifdef memset /* Thanks, Apple! */
 #undef memset
 #endif
@@ -121,12 +122,12 @@ public:
         format.cbSize = 0;
         valid = true;
 end:
-        FILESYSTEM_freeMemory(&mem);
+        VVV_free(mem);
     }
 
     void Dispose()
     {
-        SDL_free(wav_buffer);
+        VVV_free(wav_buffer);
     }
 
     void Play()
@@ -146,7 +147,7 @@ end:
                 FAudioVoice_GetVoiceDetails(voices[i], &details);
                 if (details.InputChannels != format.nChannels)
                 {
-                    FAudioVoice_DestroyVoice(voices[i]);
+                    VVV_freefunc(FAudioVoice_DestroyVoice, voices[i]);
                     FAudio_CreateSourceVoice(faudioctx, &voices[i], &format, 0, 2.0f, NULL, NULL, NULL);
                 }
                 const FAudioBuffer faudio_buffer = {
@@ -221,10 +222,9 @@ end:
         {
             for (int i = 0; i < VVV_MAX_CHANNELS; i++)
             {
-                FAudioVoice_DestroyVoice(voices[i]);
+                VVV_freefunc(FAudioVoice_DestroyVoice, voices[i]);
             }
-            SDL_free(voices);
+            VVV_free(voices);
-            voices = NULL;
         }
     }
 
@@ -263,8 +263,7 @@ public:
         if (vorbis == NULL)
         {
             vlog_error("Unable to create Vorbis handle, error %d", err);
-            SDL_free(read_buf);
+            VVV_free(read_buf);
-            read_buf = NULL;
             goto end;
         }
         vorbis_info = stb_vorbis_get_info(vorbis);
@@ -295,13 +294,12 @@ end:
     void Dispose()
     {
         stb_vorbis_close(vorbis);
-        SDL_free(read_buf);
+        VVV_free(read_buf);
-        SDL_free(decoded_buf_playing);
+        VVV_free(decoded_buf_playing);
-        SDL_free(decoded_buf_reserve);
+        VVV_free(decoded_buf_reserve);
         if (!IsHalted())
         {
-            FAudioVoice_DestroyVoice(musicVoice);
+            VVV_freefunc(FAudioVoice_DestroyVoice, musicVoice);
-            musicVoice = NULL;
         }
     }
 
@@ -358,8 +356,7 @@ end:
         if (!IsHalted())
         {
             FAudioSourceVoice_FlushSourceBuffers(musicVoice);
-            FAudioVoice_DestroyVoice(musicVoice);
+            VVV_freefunc(FAudioVoice_DestroyVoice, musicVoice);
-            musicVoice = NULL;
             paused = true;
         }
     }
@@ -535,11 +532,11 @@ end:
                 t->loopbegin = 0;
                 t->looplength = 0;
                 loopend = 0;
-                SDL_free(param);
+                VVV_free(param);
                 break;
             }
 
-            SDL_free(param);
+            VVV_free(param);
         }
         if (loopend != 0)
         {
@@ -792,14 +789,8 @@ void musicclass::destroy(void)
 
     pppppp_blob.clear();
     mmmmmm_blob.clear();
-    if (masteringvoice != NULL)
+    VVV_freefunc(FAudioVoice_DestroyVoice, masteringvoice);
-    {
+    VVV_freefunc(FAudio_Release, faudioctx);
-        FAudioVoice_DestroyVoice(masteringvoice);
-    }
-    if (faudioctx != NULL)
-    {
-        FAudio_Release(faudioctx);
-    }
 }
 
 void musicclass::play(int t)

desktop_version/src/Screen.cpp

@@ -3,6 +3,7 @@
 
 #include <SDL.h>
 
+#include "Alloc.h"
 #include "Constants.h"
 #include "FileSystemUtils.h"
 #include "Game.h"
@@ -81,17 +82,11 @@ void Screen::init(const struct ScreenSettings* settings)
 
 void Screen::destroy(void)
 {
-#define X(CLEANUP, POINTER) \
-    CLEANUP(POINTER); \
-    POINTER = NULL;
-
     /* Order matters! */
-    X(SDL_DestroyTexture, m_screenTexture);
+    VVV_freefunc(SDL_DestroyTexture, m_screenTexture);
-    X(SDL_FreeSurface, m_screen);
+    VVV_freefunc(SDL_FreeSurface, m_screen);
-    X(SDL_DestroyRenderer, m_renderer);
+    VVV_freefunc(SDL_DestroyRenderer, m_renderer);
-    X(SDL_DestroyWindow, m_window);
+    VVV_freefunc(SDL_DestroyWindow, m_window);
-
-#undef X
 }
 
 void Screen::GetSettings(struct ScreenSettings* settings)
@@ -126,7 +121,7 @@ void Screen::LoadIcon(void)
         return;
     }
     SDL_SetWindowIcon(m_window, icon);
-    SDL_FreeSurface(icon);
+    VVV_freefunc(SDL_FreeSurface, icon);
 }
 #endif /* __APPLE__ */
 
@@ -279,7 +274,7 @@ void Screen::UpdateScreen(SDL_Surface* buffer, SDL_Rect* rect )
 
     if(badSignalEffect)
     {
-        SDL_FreeSurface(buffer);
+        VVV_freefunc(SDL_FreeSurface, buffer);
     }
 
 }

desktop_version/src/ThirdPartyDeps.c

@@ -1,5 +1,7 @@
 #include <SDL_stdinc.h>
 
+#include "Alloc.h"
+
 /* Handle third-party dependencies' needs here */
 
 void* lodepng_malloc(size_t size)
@@ -14,5 +16,5 @@ void* lodepng_realloc(void* ptr, size_t new_size)
 
 void lodepng_free(void* ptr)
 {
-    SDL_free(ptr);
+    VVV_free(ptr);
 }

desktop_version/src/VFormat.c

@@ -4,6 +4,7 @@
 #include <SDL.h>
 #include <stdbool.h>
 
+#include "Alloc.h"
 #include "CWrappers.h"
 
 
@@ -64,8 +65,8 @@ static inline void call_with_upper(format_callback callback, void* userdata, con
         /* Never mind the capitalization then! Better than nothing. */
         callback(userdata, string, bytes);
 
-        SDL_free(utf32);
+        VVV_free(utf32);
-        SDL_free(utf8);
+        VVV_free(utf8);
         return;
     }
 
@@ -78,8 +79,8 @@ static inline void call_with_upper(format_callback callback, void* userdata, con
 
     callback(userdata, utf8, SDL_strlen(utf8));
 
-    SDL_free(utf32);
+    VVV_free(utf32);
-    SDL_free(utf8);
+    VVV_free(utf8);
 }
 
 
@@ -263,7 +264,7 @@ void vformat_cb_valist(
                             {
                                 callback(userdata, number, SDL_strlen(number));
                             }
-                            SDL_free(number);
+                            VVV_free(number);
                         }
                         else
                         {
@@ -458,7 +459,7 @@ char* vformat_alloc(
 )
 {
     /* Format to an automatically allocated and resized buffer.
-     * Caller must SDL_free. */
+     * Caller must VVV_free. */
 
     va_list args;
     va_start(args, args_index);

desktop_version/src/VFormat.h

@@ -5,7 +5,7 @@
  *  - vformat_cb       Calls a user-supplied callback function for each part of
  *                      the resulting string.
  *  - vformat_buf      Fills a user-supplied buffer with the result.
- *  - vformat_alloc    Allocates a buffer with the result (caller must SDL_free).
+ *  - vformat_alloc    Allocates a buffer with the result (caller must VVV_free).
  *
  * All include the following parameters:
  *  - format_string     The string which needs placeholders to be filled in