Wrapped PhysFS usage in a convenience API

* Most direct PhysFS calls now contained within a single header file.
* File class that automatically closes.
* Files class allows iterating files with range-based for.
* Use std::optional to force error handling where applicable.

10 files changed, 331 insertions, 138 deletions

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index d5d512e8..fffecab7 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -369,8 +369,12 @@ set(SRCS
     utils/copynpaste.cpp
     utils/copynpaste.h
     utils/dtor.h
+    utils/filesystem.h
     utils/gettext.h
     utils/mathutils.h
+    utils/mkdir.cpp
+    utils/mkdir.h
+    utils/mutex.h
     utils/path.cpp
     utils/path.h
     utils/physfsrwops.c
@@ -383,9 +387,6 @@ set(SRCS
     utils/stringutils.h
     utils/time.cpp
     utils/time.h
-    utils/mutex.h
-    utils/mkdir.cpp
-    utils/mkdir.h
     utils/xml.cpp
     utils/xml.h
     utils/zlib.cpp
diff --git a/src/client.cpp b/src/client.cpp
index dc6e9078..9e41976f 100644
--- a/src/client.cpp
+++ b/src/client.cpp
@@ -68,6 +68,7 @@
 #include "resources/userpalette.h"
 #include "resources/settingsmanager.h"
 
+#include "utils/filesystem.h"
 #include "utils/gettext.h"
 #include "utils/mkdir.h"
 #if defined(_WIN32) || defined(__APPLE__)
@@ -76,7 +77,6 @@
 #include "utils/stringutils.h"
 #include "utils/time.h"
 
-#include <physfs.h>
 #include <SDL_image.h>
 
 #ifdef _WIN32
@@ -222,7 +222,7 @@ Client::Client(const Options &options):
 
     ResourceManager *resman = ResourceManager::getInstance();
 
-    if (!resman->setWriteDir(mLocalDataDir))
+    if (!FS::setWriteDir(mLocalDataDir))
     {
         logger->error(strprintf("%s couldn't be set as write directory! "
                                 "Exiting.", mLocalDataDir.c_str()));
@@ -1117,12 +1117,10 @@ bool Client::initUpdatesDir()
     logger->log("Update host: %s", mUpdateHost.c_str());
     logger->log("Updates dir: %s", mUpdatesDir.c_str());
 
-    ResourceManager *resman = ResourceManager::getInstance();
-
     // Verify that the updates directory exists. Create if necessary.
-    if (!resman->isDirectory(mUpdatesDir))
+    if (!FS::isDirectory(mUpdatesDir))
     {
-        if (!resman->mkdir(mUpdatesDir))
+        if (!FS::mkdir(mUpdatesDir))
         {
 #if defined _WIN32
             std::string newDir = mLocalDataDir + "\\" + mUpdatesDir;
diff --git a/src/game.cpp b/src/game.cpp
index c8b295bb..8d3db19f 100644
--- a/src/game.cpp
+++ b/src/game.cpp
@@ -74,16 +74,14 @@
 
 #include "resources/imagewriter.h"
 #include "resources/mapreader.h"
-#include "resources/resourcemanager.h"
 
+#include "utils/filesystem.h"
 #include "utils/gettext.h"
 #include "utils/mkdir.h"
 
 #include <guichan/exception.hpp>
 #include <guichan/focushandler.hpp>
 
-#include <physfs.h>
-
 #include <fstream>
 #include <sstream>
 
@@ -928,8 +926,8 @@ void Game::changeMap(const std::string &mapPath)
 
     std::string fullMap = paths.getValue("maps", "maps/")
                           + mMapName + ".tmx";
-    ResourceManager *resman = ResourceManager::getInstance();
-    if (!resman->exists(fullMap))
+
+    if (!FS::exists(fullMap))
         fullMap += ".gz";
 
     // Attempt to load the new map
diff --git a/src/gui/minimap.cpp b/src/gui/minimap.cpp
index f5206eda..41044431 100644
--- a/src/gui/minimap.cpp
+++ b/src/gui/minimap.cpp
@@ -34,6 +34,7 @@
 #include "resources/resourcemanager.h"
 #include "resources/userpalette.h"
 
+#include "utils/filesystem.h"
 #include "utils/gettext.h"
 
 #include <guichan/font.hpp>
@@ -86,7 +87,7 @@ void Minimap::setMap(Map *map)
 
         std::string minimapName = map->getProperty("minimap");
 
-        if (minimapName.empty() && resman->exists(tempname))
+        if (minimapName.empty() && FS::exists(tempname))
             minimapName = tempname;
 
         if (!minimapName.empty())
diff --git a/src/main.cpp b/src/main.cpp
index b27152e5..21e0264c 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -23,12 +23,12 @@
 
 #include "client.h"
 
+#include "utils/filesystem.h"
 #include "utils/gettext.h"
 #include "utils/xml.h"
 
 #include <getopt.h>
 #include <iostream>
-#include <physfs.h>
 
 #ifdef __MINGW32__
 #include <windows.h>
@@ -257,12 +257,12 @@ int main(int argc, char *argv[])
     initInternationalization();
 
     // Initialize PhysicsFS
-    if (!PHYSFS_init(argv[0])) {
+    if (!FS::init(argv[0])) {
         std::cout << "Error while initializing PhysFS: "
-                << PHYSFS_getErrorByCode(PHYSFS_getLastErrorCode()) << std::endl;
+                << FS::getLastError() << std::endl;
         return 1;
     }
-    atexit((void(*)()) PHYSFS_deinit);
+    atexit((void(*)()) FS::deinit);
 
     XML::init();
 
diff --git a/src/resources/resourcemanager.cpp b/src/resources/resourcemanager.cpp
index ff83f422..80b3ae82 100644
--- a/src/resources/resourcemanager.cpp
+++ b/src/resources/resourcemanager.cpp
@@ -31,11 +31,10 @@
 #include "resources/soundeffect.h"
 #include "resources/spritedef.h"
 
+#include "utils/filesystem.h"
 #include "utils/zlib.h"
 #include "utils/physfsrwops.h"
 
-#include <physfs.h>
-
 #include <SDL_image.h>
 
 #include <cassert>
@@ -144,17 +143,12 @@ void ResourceManager::cleanOrphans()
     mOldestOrphan = oldest;
 }
 
-bool ResourceManager::setWriteDir(const std::string &path)
-{
-    return (bool) PHYSFS_setWriteDir(path.c_str());
-}
-
 bool ResourceManager::addToSearchPath(const std::string &path, bool append)
 {
     logger->log("Adding to PhysicsFS: %s", path.c_str());
-    if (!PHYSFS_mount(path.c_str(), nullptr, append ? 1 : 0))
+    if (!FS::addToSearchPath(path, append))
     {
-        logger->log("Error: %s", PHYSFS_getErrorByCode(PHYSFS_getLastErrorCode()));
+        logger->log("Error: %s", FS::getLastError());
         return false;
     }
     return true;
@@ -165,57 +159,32 @@ void ResourceManager::searchAndAddArchives(const std::string &path,
                                            bool append)
 {
     const char *dirSep = PHYSFS_getDirSeparator();
-    char **list = PHYSFS_enumerateFiles(path.c_str());
 
-    for (char **i = list; *i; i++)
+    for (auto fileName : FS::enumerateFiles(path))
     {
-        size_t len = strlen(*i);
+        const size_t len = strlen(fileName);
 
-        if (len > ext.length() && !ext.compare((*i)+(len - ext.length())))
+        if (len > ext.length() && !ext.compare(fileName + (len - ext.length())))
         {
-            std::string file, realPath, archive;
-
-            file = path + (*i);
-            realPath = std::string(PHYSFS_getRealDir(file.c_str()));
-            archive = realPath + dirSep + file;
-
-            addToSearchPath(archive, append);
+            std::string file = path + fileName;
+            if (auto realDir = FS::getRealDir(file))
+            {
+                std::string archive = std::string(*realDir) + dirSep + file;
+                addToSearchPath(archive, append);
+            }
         }
     }
-
-    PHYSFS_freeList(list);
-}
-
-bool ResourceManager::mkdir(const std::string &path)
-{
-    return (bool) PHYSFS_mkdir(path.c_str());
-}
-
-bool ResourceManager::exists(const std::string &path)
-{
-    return PHYSFS_exists(path.c_str());
-}
-
-bool ResourceManager::isDirectory(const std::string &path)
-{
-    PHYSFS_Stat stat;
-    if (PHYSFS_stat(path.c_str(), &stat) != 0)
-    {
-        return stat.filetype == PHYSFS_FILETYPE_DIRECTORY;
-    }
-    return false;
 }
 
 std::string ResourceManager::getPath(const std::string &file)
 {
-    // get the real path to the file
-    const char* tmp = PHYSFS_getRealDir(file.c_str());
+    // Get the real directory of the file
+    auto realDir = FS::getRealDir(file);
     std::string path;
 
-    // if the file is not in the search path, then its NULL
-    if (tmp)
+    if (realDir)
     {
-        path = std::string(tmp) + "/" + file;
+        path = std::string(*realDir) + "/" + file;
     }
     else
     {
@@ -378,29 +347,40 @@ void *ResourceManager::loadFile(const std::string &filename, int &filesize,
                                 bool inflate)
 {
     // Attempt to open the specified file using PhysicsFS
-    PHYSFS_file *file = PHYSFS_openRead(filename.c_str());
-
-    // If the handler is an invalid pointer indicate failure
-    if (file == nullptr)
+    auto file = FS::openRead(filename);
+    if (!file)
     {
         logger->log("Warning: Failed to load %s: %s",
-                filename.c_str(), PHYSFS_getErrorByCode(PHYSFS_getLastErrorCode()));
+                filename.c_str(), FS::getLastError());
         return nullptr;
     }
 
     // Log the real dir of the file
-    logger->log("Loaded %s/%s", PHYSFS_getRealDir(filename.c_str()),
+    logger->log("Loaded %s/%s", FS::getRealDir(filename).value_or("<null>"),
             filename.c_str());
 
     // Get the size of the file
-    filesize = PHYSFS_fileLength(file);
+    auto maybeFilesize = file.fileLength();
+    if (!maybeFilesize)
+    {
+        logger->log("Error getting file size: %s", FS::getLastError());
+        return nullptr;
+    }
+
+    filesize = *maybeFilesize;
 
     // Allocate memory and load the file
     void *buffer = malloc(filesize);
-    PHYSFS_readBytes(file, buffer, filesize);
+    auto readSize = file.read(buffer, filesize);
+    if (!readSize || *readSize != filesize)
+    {
+        logger->log("Error reading file: %s", FS::getLastError());
+        free(buffer);
+        return nullptr;
+    }
 
     // Close the file and let the user deallocate the memory
-    PHYSFS_close(file);
+    file.close();
 
     if (inflate && filename.find(".gz", filename.length() - 3)
             != std::string::npos)
@@ -424,28 +404,47 @@ void *ResourceManager::loadFile(const std::string &filename, int &filesize,
 
 bool ResourceManager::copyFile(const std::string &src, const std::string &dst)
 {
-    PHYSFS_file *srcFile = PHYSFS_openRead(src.c_str());
+    auto srcFile = FS::openRead(src);
     if (!srcFile)
     {
-        logger->log("Read error: %s", PHYSFS_getErrorByCode(PHYSFS_getLastErrorCode()));
+        logger->log("Read error: %s", FS::getLastError());
         return false;
     }
-    PHYSFS_file *dstFile = PHYSFS_openWrite(dst.c_str());
+    auto dstFile = FS::openWrite(dst);
     if (!dstFile)
     {
-        logger->log("Write error: %s", PHYSFS_getErrorByCode(PHYSFS_getLastErrorCode()));
-        PHYSFS_close(srcFile);
+        logger->log("Write error: %s", FS::getLastError());
         return false;
     }
 
-    int fileSize = PHYSFS_fileLength(srcFile);
-    void *buf = malloc(fileSize);
-    PHYSFS_readBytes(srcFile, buf, fileSize);
-    PHYSFS_writeBytes(dstFile, buf, fileSize);
+    char buffer[1024];
+
+    while (true)
+    {
+        auto len = srcFile.read(buffer, sizeof(buffer));
+        if (!len)
+        {
+            logger->log("Read error: %s", FS::getLastError());
+            return false;
+        }
+
+        if (!dstFile.write(buffer, *len))
+        {
+            logger->log("Write error: %s", FS::getLastError());
+            return false;
+        }
+
+        if (srcFile.eof())
+            break;
+    }
+
+    // Explicit close to flush the file and check for errors
+    if (!dstFile.close())
+    {
+        logger->log("Write error: %s", FS::getLastError());
+        return false;
+    }
 
-    PHYSFS_close(srcFile);
-    PHYSFS_close(dstFile);
-    free(buf);
     return true;
 }
 
diff --git a/src/resources/resourcemanager.h b/src/resources/resourcemanager.h
index 3ce4229b..b72920cf 100644
--- a/src/resources/resourcemanager.h
+++ b/src/resources/resourcemanager.h
@@ -57,14 +57,6 @@ class ResourceManager
         ~ResourceManager();
 
         /**
-         * Sets the write directory.
-         *
-         * @param path The path of the directory to be added.
-         * @return <code>true</code> on success, <code>false</code> otherwise.
-         */
-        bool setWriteDir(const std::string &path);
-
-        /**
          * Adds a directory or archive to the search path. If append is true
          * then the directory is added to the end of the search path, otherwise
          * it is added at the front.
@@ -81,21 +73,6 @@ class ResourceManager
                                   bool append);
 
         /**
-         * Creates a directory in the write path
-         */
-        bool mkdir(const std::string &path);
-
-        /**
-         * Checks whether the given file or directory exists in the search path
-         */
-        bool exists(const std::string &path);
-
-        /**
-         * Checks whether the given path is a directory.
-         */
-        bool isDirectory(const std::string &path);
-
-        /**
          * Returns the real path to a file. Note that this method will always
          * return a path, it does not check whether the file exists.
          *
diff --git a/src/resources/theme.cpp b/src/resources/theme.cpp
index ad686e19..1eb2f87c 100644
--- a/src/resources/theme.cpp
+++ b/src/resources/theme.cpp
@@ -32,11 +32,10 @@
 #include "resources/resourcemanager.h"
 
 #include "utils/dtor.h"
+#include "utils/filesystem.h"
 #include "utils/stringutils.h"
 #include "utils/xml.h"
 
-#include <physfs.h>
-
 #include <algorithm>
 
 static std::string defaultThemePath;
@@ -46,10 +45,9 @@ Theme *Theme::mInstance = nullptr;
 // Set the theme path...
 static void initDefaultThemePath()
 {
-    ResourceManager *resman = ResourceManager::getInstance();
     defaultThemePath = branding.getStringValue("guiThemePath");
 
-    if (defaultThemePath.empty() || !resman->isDirectory(defaultThemePath))
+    if (defaultThemePath.empty() || !FS::isDirectory(defaultThemePath))
         defaultThemePath = "graphics/gui/";
 }
 
@@ -323,7 +321,7 @@ bool Theme::tryThemePath(std::string themePath)
     {
         themePath = defaultThemePath + themePath;
 
-        if (PHYSFS_exists(themePath.c_str()))
+        if (FS::exists(themePath))
         {
             mThemePath = themePath;
             return true;
@@ -359,12 +357,12 @@ std::string Theme::resolveThemePath(const std::string &path)
         file = path;
 
     // Might be a valid path already
-    if (PHYSFS_exists(file.c_str()))
+    if (FS::exists(file))
         return path;
 
     // Try the theme
     file = getThemePath() + "/" + file;
-    if (PHYSFS_exists(file.c_str()))
+    if (FS::exists(file))
         return getThemePath() + "/" + path;
 
     // Backup
diff --git a/src/resources/wallpaper.cpp b/src/resources/wallpaper.cpp
index e8167b6b..2bdcd656 100644
--- a/src/resources/wallpaper.cpp
+++ b/src/resources/wallpaper.cpp
@@ -23,7 +23,7 @@
 
 #include "configuration.h"
 
-#include <physfs.h>
+#include "utils/filesystem.h"
 
 #include <algorithm>
 #include <cstring>
@@ -90,35 +90,24 @@ void Wallpaper::loadWallpapers()
 
     initWallpaperPaths();
 
-    char **fileNames = PHYSFS_enumerateFiles(wallpaperPath.c_str());
-
-    for (char **fileName = fileNames; *fileName; fileName++)
+    for (auto fileName : FS::enumerateFiles(wallpaperPath))
     {
-        int width;
-        int height;
-
         // If the backup file is found, we tell it.
-        if (strncmp(*fileName, wallpaperFile.c_str(), strlen(*fileName)) == 0)
+        if (wallpaperFile == fileName)
             haveBackup = true;
 
         // If the image format is terminated by: "_<width>x<height>.png"
         // It is taken as a potential wallpaper.
-
-        // First, get the base filename of the image:
-        std::string filename = *fileName;
-        filename = filename.substr(0, filename.rfind("_"));
-
-        // Check that the base filename doesn't have any '%' markers.
-        if (filename.find("%") == std::string::npos)
+        if (auto sizeSuffix = strrchr(fileName, '_'))
         {
-            // Then, append the width and height search mask.
-            filename.append("_%dx%d.png");
+            int width;
+            int height;
 
-            if (sscanf(*fileName, filename.c_str(), &width, &height) == 2)
+            if (sscanf(sizeSuffix, "_%dx%d.png", &width, &height) == 2)
             {
                 WallpaperData wp;
                 wp.filename = wallpaperPath;
-                wp.filename.append(*fileName);
+                wp.filename.append(fileName);
                 wp.width = width;
                 wp.height = height;
                 wallpaperData.push_back(wp);
@@ -126,8 +115,6 @@ void Wallpaper::loadWallpapers()
         }
     }
 
-    PHYSFS_freeList(fileNames);
-
     std::sort(wallpaperData.begin(), wallpaperData.end(), wallpaperCompare);
 }
 
diff --git a/src/utils/filesystem.h b/src/utils/filesystem.h
new file mode 100644
index 00000000..68b3d82a
--- /dev/null
+++ b/src/utils/filesystem.h
@@ -0,0 +1,234 @@
+/*
+ *  The Mana Client
+ *  Copyright (C) 2024  The Mana Developers
+ *
+ *  This file is part of The Mana Client.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#pragma once
+
+#include <physfs.h>
+
+#include <optional>
+#include <string>
+
+/**
+ * These functions wrap PHYSFS functions to provide a more user-friendly
+ * interface and to limit the direct use of the PHYSFS API to a single file.
+ */
+namespace FS {
+
+inline bool init(const char *argv0)
+{
+    return PHYSFS_init(argv0) != 0;
+}
+
+inline void deinit()
+{
+    PHYSFS_deinit();
+}
+
+/**
+ * Sets the write directory.
+ *
+ * @param path The path of the directory to be added.
+ * @return <code>true</code> on success, <code>false</code> otherwise.
+ */
+inline bool setWriteDir(const std::string &path)
+{
+    return PHYSFS_setWriteDir(path.c_str()) != 0;
+}
+
+/**
+ * Adds a directory or archive to the search path. If append is true
+ * then the directory is added to the end of the search path, otherwise
+ * it is added at the front.
+ *
+ * @return <code>true</code> on success, <code>false</code> otherwise.
+ */
+inline bool addToSearchPath(const std::string &path, bool append)
+{
+    return PHYSFS_mount(path.c_str(), "/", append ? 1 : 0) != 0;
+}
+
+/**
+ * Checks whether the given file or directory exists in the search path.
+ */
+inline bool exists(const std::string &path)
+{
+    return PHYSFS_exists(path.c_str()) != 0;
+}
+
+inline std::optional<const char *> getRealDir(const std::string &path)
+{
+    auto dir = PHYSFS_getRealDir(path.c_str());
+    return dir ? std::optional<const char *>(dir) : std::nullopt;
+}
+
+/**
+ * Checks whether the given path is a directory.
+ */
+inline bool isDirectory(const std::string &path)
+{
+    PHYSFS_Stat stat;
+    if (PHYSFS_stat(path.c_str(), &stat) != 0)
+    {
+        return stat.filetype == PHYSFS_FILETYPE_DIRECTORY;
+    }
+    return false;
+}
+
+/**
+ * Creates a directory in the write path.
+ */
+inline bool mkdir(const std::string &path)
+{
+    return PHYSFS_mkdir(path.c_str()) != 0;
+}
+
+/**
+ * Helper class to iterate over the files in a directory.
+ * Based on https://stackoverflow.com/a/79051293/355419.
+ */
+class Files
+{
+public:
+    struct End {};
+    friend bool operator!=(const char *const *files, End)
+    { return *files != nullptr; }
+
+    explicit Files(char **files) : mFiles(files) {}
+    ~Files() { PHYSFS_freeList(mFiles); }
+
+    Files(const Files &) = delete;
+    Files &operator=(const Files &) = delete;
+
+    // Relies on C++17 support for begin/end to not have the same return type
+    const char* const *begin() const { return mFiles; }
+    End end() const { return End(); }
+
+private:
+    char **mFiles;
+};
+
+/**
+ * Returns a list of files in the given directory.
+ */
+inline Files enumerateFiles(const std::string &dir)
+{
+    return Files(PHYSFS_enumerateFiles(dir.c_str()));
+}
+
+/**
+ * File wrapper class to provide a more convenient API and automatic closing.
+ */
+class File
+{
+public:
+    explicit File(PHYSFS_file *file)
+        : file(file)
+    {}
+
+    ~File()
+    {
+        if (isOpen())
+            close();
+    }
+
+    bool isOpen() const
+    {
+        return file != nullptr;
+    }
+
+    operator bool() const
+    {
+        return isOpen();
+    }
+
+    bool close()
+    {
+        if (PHYSFS_close(file) != 0)
+        {
+            file = nullptr;
+            return true;
+        }
+        return false;
+    }
+
+    std::optional<size_t> read(void *data, size_t size)
+    {
+        auto len = PHYSFS_readBytes(file, data, size);
+        return len >= 0 ? std::optional<size_t>(len) : std::nullopt;
+    }
+
+    std::optional<size_t> write(const void *data, size_t size)
+    {
+        auto len = PHYSFS_writeBytes(file, data, size);
+        return len >= 0 ? std::optional<size_t>(len) : std::nullopt;
+    }
+
+    bool flush()
+    {
+        return PHYSFS_flush(file) != 0;
+    }
+
+    bool seek(size_t pos)
+    {
+        return PHYSFS_seek(file, pos) != 0;
+    }
+
+    std::optional<size_t> fileLength() const
+    {
+        auto len = PHYSFS_fileLength(file);
+        return len >= 0 ? std::optional<size_t>(len) : std::nullopt;
+    }
+
+    std::optional<size_t> tell() const
+    {
+        auto pos = PHYSFS_tell(file);
+        return pos >= 0 ? std::optional<size_t>(pos) : std::nullopt;
+    }
+
+    bool eof() const
+    {
+        return PHYSFS_eof(file) != 0;
+    }
+
+private:
+    PHYSFS_file *file;
+};
+
+inline File openWrite(const std::string &path)
+{
+    return File(PHYSFS_openWrite(path.c_str()));
+}
+
+inline File openAppend(const std::string &path)
+{
+    return File(PHYSFS_openAppend(path.c_str()));
+}
+
+inline File openRead(const std::string &path)
+{
+    return File(PHYSFS_openRead(path.c_str()));
+}
+
+inline const char *getLastError()
+{
+    return PHYSFS_getErrorByCode(PHYSFS_getLastErrorCode());
+}
+
+} // namespace FS