From 3ebdc1da89c1eea6d69d38339d1ebe06ee34fa1a Mon Sep 17 00:00:00 2001 From: Misa Date: Sun, 18 Apr 2021 10:35:36 -0700 Subject: [PATCH] Transfer param init responsibility to loadFileToMemory So, the codebase was kind of undecided about who is responsible for initializing the parameters passed to FILESYSTEM_loadFileToMemory() - is it the caller? Is it FILESYSTEM_loadFileToMemory()? Sometimes callers would initialize one variable but not the other, and it was always a toss-up whether or not FILESYSTEM_loadFileToMemory() would end up initializing everything in the end. All of this is to say that the game dereferences an uninitialized pointer if it can't load a sound effect. Which is bad. Now, I could either fix that single case, or fix every case. Judging by the title of this commit, you can infer that I decided to fix every case - fixing every case means not just all cases that currently exist (which, as far as I know, is only the sound effect one), but all cases that could exist in the future. So, FILESYSTEM_loadFileToMemory() is now guaranteed to initialize its parameters even if the file fails to be loaded. This is better than passing the responsibility to the caller anyway, because if the caller initialized it, then that would be wasted work if the file succeeds anyway because FILESYSTEM_loadFileToMemory() will overwrite it, and if the file fails to load, well that's when the variables get initialized anyway. --- desktop_version/src/FileSystemUtils.cpp | 15 +++++++++++++-- desktop_version/src/Graphics.cpp | 2 +- desktop_version/src/GraphicsResources.cpp | 4 ++-- desktop_version/src/Screen.cpp | 4 ++-- desktop_version/src/SoundSystem.cpp | 2 +- desktop_version/src/editor.cpp | 2 +- 6 files changed, 20 insertions(+), 9 deletions(-) diff --git a/desktop_version/src/FileSystemUtils.cpp b/desktop_version/src/FileSystemUtils.cpp index f077d38f..8c538a7c 100644 --- a/desktop_version/src/FileSystemUtils.cpp +++ b/desktop_version/src/FileSystemUtils.cpp @@ -484,7 +484,7 @@ void FILESYSTEM_loadFileToMemory( handle = PHYSFS_openRead(name); if (handle == NULL) { - return; + goto fail; } length = PHYSFS_fileLength(handle); if (len != NULL) @@ -518,6 +518,17 @@ void FILESYSTEM_loadFileToMemory( FILESYSTEM_freeMemory(mem); } PHYSFS_close(handle); + return; + +fail: + if (mem != NULL) + { + *mem = NULL; + } + if (len != NULL) + { + *len = 0; + } } void FILESYSTEM_loadAssetToMemory( @@ -645,7 +656,7 @@ bool FILESYSTEM_saveTiXml2Document(const char *name, tinyxml2::XMLDocument& doc) bool FILESYSTEM_loadTiXml2Document(const char *name, tinyxml2::XMLDocument& doc) { /* XMLDocument.LoadFile doesn't account for Unicode paths, PHYSFS does */ - unsigned char *mem = NULL; + unsigned char *mem; FILESYSTEM_loadFileToMemory(name, &mem, NULL, true); if (mem == NULL) { diff --git a/desktop_version/src/Graphics.cpp b/desktop_version/src/Graphics.cpp index 92a41094..f2933c90 100644 --- a/desktop_version/src/Graphics.cpp +++ b/desktop_version/src/Graphics.cpp @@ -361,7 +361,7 @@ void Graphics::Makebfont(void) flipbfont.push_back(TempFlipped); }) - unsigned char* charmap = NULL; + unsigned char* charmap; size_t length; FILESYSTEM_loadAssetToMemory("graphics/font.txt", &charmap, &length, false); if (charmap != NULL) diff --git a/desktop_version/src/GraphicsResources.cpp b/desktop_version/src/GraphicsResources.cpp index 3558e391..acc1fa71 100644 --- a/desktop_version/src/GraphicsResources.cpp +++ b/desktop_version/src/GraphicsResources.cpp @@ -34,8 +34,8 @@ static SDL_Surface* LoadImage(const char *filename, bool noBlend = true, bool no unsigned char *data; unsigned int width, height; - unsigned char *fileIn = NULL; - size_t length = 0; + unsigned char *fileIn; + size_t length; FILESYSTEM_loadAssetToMemory(filename, &fileIn, &length, false); if (noAlpha) { diff --git a/desktop_version/src/Screen.cpp b/desktop_version/src/Screen.cpp index d32c394d..a232634e 100644 --- a/desktop_version/src/Screen.cpp +++ b/desktop_version/src/Screen.cpp @@ -127,8 +127,8 @@ void Screen::GetSettings(ScreenSettings* settings) void Screen::LoadIcon(void) { #ifndef __APPLE__ - unsigned char *fileIn = NULL; - size_t length = 0; + unsigned char *fileIn; + size_t length; unsigned char *data; unsigned int width, height; FILESYSTEM_loadAssetToMemory("VVVVVV.png", &fileIn, &length, false); diff --git a/desktop_version/src/SoundSystem.cpp b/desktop_version/src/SoundSystem.cpp index c0b79fad..e5d0ead8 100644 --- a/desktop_version/src/SoundSystem.cpp +++ b/desktop_version/src/SoundSystem.cpp @@ -30,7 +30,7 @@ MusicTrack::MusicTrack(SDL_RWops *rw) SoundTrack::SoundTrack(const char* fileName) { unsigned char *mem; - size_t length = 0; + size_t length; sound = NULL; diff --git a/desktop_version/src/editor.cpp b/desktop_version/src/editor.cpp index 88a53c63..6b2e0725 100644 --- a/desktop_version/src/editor.cpp +++ b/desktop_version/src/editor.cpp @@ -248,7 +248,7 @@ void editorclass::getDirectoryData(void) } bool editorclass::getLevelMetaData(std::string& _path, LevelMetaData& _data ) { - unsigned char *uMem = NULL; + unsigned char *uMem; FILESYSTEM_loadFileToMemory(_path.c_str(), &uMem, NULL, true); if (uMem == NULL)