From a30c3f71995c3487a89bcd6c0ee80c6fe031296d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20M=C3=BCller?= <robytemueller@gmail.com>
Date: Fri, 3 Jan 2025 12:19:43 +0100
Subject: [PATCH] Refactor `IStorage::OpenFile` function

Add assertion to ensure the file opening flags match the storage type.

Reduce indentation to improve readability.

Ensure result buffer is initialized with path also when opening fails, so the path can be logged in error messages.
---
 src/engine/shared/storage.cpp | 51 +++++++++++++++++------------------
 1 file changed, 24 insertions(+), 27 deletions(-)

diff --git a/src/engine/shared/storage.cpp b/src/engine/shared/storage.cpp
index 4425b0c0af5..989f27ac3d8 100644
--- a/src/engine/shared/storage.cpp
+++ b/src/engine/shared/storage.cpp
@@ -452,64 +452,61 @@ class CStorage : public IStorage
 			Type = fs_is_relative_path(pPath) ? TYPE_ALL : TYPE_ABSOLUTE;
 	}
 
-	IOHANDLE OpenFile(const char *pFilename, int Flags, int Type, char *pBuffer = 0, int BufferSize = 0) override
+	IOHANDLE OpenFile(const char *pFilename, int Flags, int Type, char *pBuffer = nullptr, int BufferSize = 0) override
 	{
 		TranslateType(Type, pFilename);
 
+		dbg_assert((Flags & IOFLAG_WRITE) == 0 || Type == TYPE_SAVE || Type == TYPE_ABSOLUTE, "IOFLAG_WRITE only usable with TYPE_SAVE and TYPE_ABSOLUTE");
+
 		char aBuffer[IO_MAX_PATH_LENGTH];
 		if(!pBuffer)
 		{
 			pBuffer = aBuffer;
 			BufferSize = sizeof(aBuffer);
 		}
+		pBuffer[0] = '\0';
 
 		if(Type == TYPE_ABSOLUTE)
 		{
 			return io_open(GetPath(TYPE_ABSOLUTE, pFilename, pBuffer, BufferSize), Flags);
 		}
+
 		if(str_startswith(pFilename, "mapres/../skins/"))
 		{
 			pFilename = pFilename + 10; // just start from skins/
 		}
-		if(pFilename[0] == '/' || pFilename[0] == '\\' || str_find(pFilename, "../") != NULL || str_find(pFilename, "..\\") != NULL
+		if(pFilename[0] == '/' || pFilename[0] == '\\' || str_find(pFilename, "../") != nullptr || str_find(pFilename, "..\\") != nullptr
 #ifdef CONF_FAMILY_WINDOWS
 			|| (pFilename[0] && pFilename[1] == ':')
 #endif
 		)
 		{
 			// don't escape base directory
+			return nullptr;
 		}
-		else if(Flags & IOFLAG_WRITE)
-		{
-			return io_open(GetPath(TYPE_SAVE, pFilename, pBuffer, BufferSize), Flags);
-		}
-		else
+		else if(Type == TYPE_ALL)
 		{
-			if(Type == TYPE_ALL)
-			{
-				// check all available directories
-				for(int i = TYPE_SAVE; i < m_NumPaths; ++i)
-				{
-					IOHANDLE Handle = io_open(GetPath(i, pFilename, pBuffer, BufferSize), Flags);
-					if(Handle)
-						return Handle;
-				}
-			}
-			else if(Type >= TYPE_SAVE && Type < m_NumPaths)
+			// check all available directories
+			for(int i = TYPE_SAVE; i < m_NumPaths; ++i)
 			{
-				// check wanted directory
-				IOHANDLE Handle = io_open(GetPath(Type, pFilename, pBuffer, BufferSize), Flags);
+				IOHANDLE Handle = io_open(GetPath(i, pFilename, pBuffer, BufferSize), Flags);
 				if(Handle)
+				{
 					return Handle;
+				}
 			}
-			else
-			{
-				dbg_assert(false, "Type invalid");
-			}
+			return nullptr;
+		}
+		else if(Type >= TYPE_SAVE && Type < m_NumPaths)
+		{
+			// check wanted directory
+			return io_open(GetPath(Type, pFilename, pBuffer, BufferSize), Flags);
+		}
+		else
+		{
+			dbg_assert(false, "Type invalid");
+			return nullptr;
 		}
-
-		pBuffer[0] = 0;
-		return 0;
 	}
 
 	template<typename F>