From 2d1ad2d326bdd6c5cb9f49cdcb883892af423cd7 Mon Sep 17 00:00:00 2001 From: Will Toohey Date: Fri, 26 Jun 2026 19:46:40 +1000 Subject: [PATCH] Modernize: we can use std::mutex again~ And hey, found a reentrancy bug that the AI made writing ARC support (huge surprise...) --- src/imagefs.cpp | 26 +++++++----------- src/log.cpp | 11 +++----- src/modpath_handler.cpp | 6 ++--- src/modpath_handler.h | 2 +- src/ramfs_demangler.cpp | 27 ++++++------------- src/winxp_mutex.hpp | 58 ----------------------------------------- 6 files changed, 26 insertions(+), 104 deletions(-) delete mode 100644 src/winxp_mutex.hpp diff --git a/src/imagefs.cpp b/src/imagefs.cpp index c863f4e..06ec1c6 100644 --- a/src/imagefs.cpp +++ b/src/imagefs.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include "3rd_party/lodepng.h" @@ -17,7 +18,6 @@ #include "modpath_handler.h" #include "texture_packer.h" #include "utils.hpp" -#include "winxp_mutex.hpp" using std::string; @@ -51,10 +51,10 @@ typedef struct afp { // ifs_textures["data/graphics/ver04/logo.ifs/tex/4f754d4f424f092637a49a5527ece9bb"] will be "konami" static std::map, CaseInsensitiveCompare> ifs_textures; -static CriticalSectionLock ifs_textures_mtx; +static std::mutex ifs_textures_mtx; static std::map, CaseInsensitiveCompare> afp_md5_names; -static CriticalSectionLock afp_md5_names_mtx; +static std::mutex afp_md5_names_mtx; void rapidxml_dump_to_file(const string& out, const rapidxml::xml_document<> &xml) { @@ -182,9 +182,8 @@ bool add_images_to_list(string_set &extra_pngs, rapidxml::xml_node<> *texturelis image_info.height = texture->height; auto md5_path = ifs_path + "/tex/" + image_info.name_md5; - ifs_textures_mtx.lock(); + std::lock_guard lock(ifs_textures_mtx); ifs_textures[md5_path] = std::make_shared(std::move(image_info)); - ifs_textures_mtx.unlock(); } } @@ -293,9 +292,8 @@ void parse_texturelist(HookFile &file) { extra_pngs.erase(image_info.name); auto md5_path = ifs_path + "/tex/" + image_info.name_md5; - ifs_textures_mtx.lock(); + std::lock_guard lock(ifs_textures_mtx); ifs_textures[md5_path] = std::make_shared(std::move(image_info)); - ifs_textures_mtx.unlock(); } } @@ -474,7 +472,7 @@ void parse_afplist(HookFile &file) { // log_info("AFP %s -> %s", md5_path.c_str(), (ifs_mod_path + folder + file).c_str()); }; - afp_md5_names_mtx.lock(); + std::lock_guard lock(afp_md5_names_mtx); add_mapping("/afp/", name->value()); add_mapping("/afp/bsi/", name->value()); @@ -485,24 +483,21 @@ void parse_afplist(HookFile &file) { while(ss >> index) { add_mapping("/geo/", std::string(name->value()) + "_shape" + index); } - - afp_md5_names_mtx.unlock(); } log_verbose("Mapped %d AFP filenames", mapped); } std::optional>> lookup_png_from_md5(HookFile &file) { - ifs_textures_mtx.lock(); + std::unique_lock lock(ifs_textures_mtx); auto tex_search = ifs_textures.find(file.norm_path); if (tex_search == ifs_textures.end()) { - ifs_textures_mtx.unlock(); return std::nullopt; } //log_misc("Mapped file %s is found!", norm_path.c_str()); auto tex = tex_search->second; - ifs_textures_mtx.unlock(); // is it safe to unlock this early? Time will tell... + lock.unlock(); // is it safe to unlock this early? Time will tell... // remove the /tex/, it's nicer to navigate auto png_path = find_first_modfile(tex->ifs_mod_path + "/" + tex->name + ".png"); @@ -540,16 +535,15 @@ void handle_texture(HookFile &file) { } std::optional lookup_afp_from_md5(HookFile &file) { - afp_md5_names_mtx.lock(); + std::unique_lock lock(afp_md5_names_mtx); auto afp_search = afp_md5_names.find(file.norm_path); if (afp_search == afp_md5_names.end()) { - afp_md5_names_mtx.unlock(); return std::nullopt; } //log_misc("Mapped file %s is found!", norm_path.c_str()); auto afp = afp_search->second; - afp_md5_names_mtx.unlock(); // is it safe to unlock this early? Time will tell... + lock.unlock(); // is it safe to unlock this early? Time will tell... return find_first_modfile(afp->mod_path); } diff --git a/src/log.cpp b/src/log.cpp index 2c6b74d..3d160ef 100644 --- a/src/log.cpp +++ b/src/log.cpp @@ -1,8 +1,8 @@ +#include #include #include "config.hpp" #include "log.hpp" -#include "winxp_mutex.hpp" #define SUPPRESS_PRINTF @@ -13,7 +13,7 @@ void stdout_log(char level, const char *fmt, va_list args) { } static void log_to_file(char level, const char* fmt, va_list args) { - static CriticalSectionLock log_mutex; + static std::mutex log_mutex; static FILE* logfile = NULL; static bool tried_to_open = false; #ifndef SUPPRESS_PRINTF @@ -21,7 +21,7 @@ static void log_to_file(char level, const char* fmt, va_list args) { #endif // don't reopen every time: slow as shit if (!tried_to_open) { - log_mutex.lock(); + std::lock_guard lock(log_mutex); if (!logfile) { // default to ifs_hook.log because we need *something* in the case @@ -30,10 +30,9 @@ static void log_to_file(char level, const char* fmt, va_list args) { logfile = fopen(path, "w"); } tried_to_open = true; - log_mutex.unlock(); } if (logfile) { - log_mutex.lock(); + std::lock_guard lock(log_mutex); fprintf(logfile, "%c:", level); vfprintf(logfile, fmt, args); @@ -41,8 +40,6 @@ static void log_to_file(char level, const char* fmt, va_list args) { if(config.developer_mode || level == 'F') fflush(logfile); - - log_mutex.unlock(); } } diff --git a/src/modpath_handler.cpp b/src/modpath_handler.cpp index 60fe414..e8a0ee9 100644 --- a/src/modpath_handler.cpp +++ b/src/modpath_handler.cpp @@ -9,7 +9,6 @@ #include "log.hpp" #include "utils.hpp" #include "avs.h" -#include "winxp_mutex.hpp" using std::nullopt; @@ -98,9 +97,10 @@ void modpath_debug_add_folder(const string &folder) { game_folders.push_back(folder + "/"); } -optional normalise_path(const string &_path) { +optional normalise_path(const string &_path, bool demangle) { auto path = _path; - ramfs_demangler_demangle_if_possible(path); + if (demangle) + ramfs_demangler_demangle_if_possible(path); auto data_pos = string_find_icase(path, "data/"); auto other_pos = string::npos; diff --git a/src/modpath_handler.h b/src/modpath_handler.h index 91c02da..78c46fa 100644 --- a/src/modpath_handler.h +++ b/src/modpath_handler.h @@ -19,7 +19,7 @@ void modpath_debug_add_folder(const string &folder); void cache_mods(void); vector available_mods(); // mutates source string to be all lowercase -optional normalise_path(const string &path); +optional normalise_path(const string &path, bool demangle = true); optional find_first_modfile(const string &norm_path); optional find_first_modfolder(const string &norm_path); vector find_all_modfile(const string &norm_path); diff --git a/src/ramfs_demangler.cpp b/src/ramfs_demangler.cpp index 688a357..adb10ce 100644 --- a/src/ramfs_demangler.cpp +++ b/src/ramfs_demangler.cpp @@ -33,6 +33,7 @@ #include #include #include +#include #include #include @@ -42,7 +43,6 @@ #include "log.hpp" #include "modpath_handler.h" #include "utils.hpp" -#include "winxp_mutex.hpp" using namespace std; @@ -63,7 +63,7 @@ static tsl::htrie_map mangling_map; // Basename ("foo.ifs") -> demangled inner path ("arc/.../foo_arc/.../foo.ifs") static unordered_map arc_inner_by_basename; -static CriticalSectionLock mangling_mtx; +static std::mutex mangling_mtx; // since we call this from a function that is already taking the lock static void ramfs_demangler_demangle_if_possible_nolock(std::string& raw_path); @@ -75,7 +75,7 @@ void ramfs_demangler_on_fs_open(const std::string& path, AVS_FILE open_result) { return; } - mangling_mtx.lock(); + std::lock_guard lock(mangling_mtx); auto existing_info = cleanup_map.find(path); if (existing_info != cleanup_map.end()) { @@ -104,12 +104,10 @@ void ramfs_demangler_on_fs_open(const std::string& path, AVS_FILE open_result) { }; cleanup_map[path] = cleanup; open_file_map[open_result] = path; - - mangling_mtx.unlock(); } void ramfs_demangler_register_arc_inner_ifs(const std::string& basename, const std::string& demangled_path) { - mangling_mtx.lock(); + std::lock_guard lock(mangling_mtx); auto existing = arc_inner_by_basename.find(basename); if (existing != arc_inner_by_basename.end() && existing->second != demangled_path) { log_warning("arc demangle: basename collision for '%s' (%s vs %s), later one wins", @@ -117,11 +115,10 @@ void ramfs_demangler_register_arc_inner_ifs(const std::string& basename, const s } arc_inner_by_basename[basename] = demangled_path; log_verbose("arc inner basename '%s' -> %s", basename.c_str(), demangled_path.c_str()); - mangling_mtx.unlock(); } void ramfs_demangler_on_fs_read(AVS_FILE context, void* dest) { - mangling_mtx.lock(); + std::lock_guard lock(mangling_mtx); auto find = open_file_map.find(context); if (find != open_file_map.end()) { @@ -135,25 +132,21 @@ void ramfs_demangler_on_fs_read(AVS_FILE context, void* dest) { cleanup->second.buffer = dest; } } - - mangling_mtx.unlock(); } void ramfs_demangler_on_fs_mount(const char* mountpoint, const char* fsroot, const char* fstype, const char* flags) { - mangling_mtx.lock(); + std::lock_guard lock(mangling_mtx); if (!strcmp(fstype, "ramfs")) { void* buffer; if (!flags) { log_verbose("ramfs has no flags?"); - mangling_mtx.unlock(); return; } const char* baseptr = strstr(flags, "base="); if (!baseptr) { log_verbose("ramfs has no base pointer?"); - mangling_mtx.unlock(); return; } @@ -234,26 +227,22 @@ void ramfs_demangler_on_fs_mount(const char* mountpoint, const char* fsroot, con // the result is something normalise_path can use. string root = (string)fsroot; ramfs_demangler_demangle_if_possible_nolock(root); - if (normalise_path(root)) { + if (normalise_path(root, /* demangle */ false)) { log_verbose("imagefs mount mapped to %s", root.c_str()); mangling_map[mountpoint] = root; } } } - - mangling_mtx.unlock(); } void ramfs_demangler_demangle_if_possible(std::string& raw_path) { - mangling_mtx.lock(); + std::lock_guard lock(mangling_mtx); auto search = mangling_map.longest_prefix(raw_path); if (search != mangling_map.end()) { // log_verbose("can demangle %s to %s", search.key().c_str(), search->c_str()); string_replace(raw_path, search.key().c_str(), search->c_str()); } - - mangling_mtx.unlock(); } static void ramfs_demangler_demangle_if_possible_nolock(std::string& raw_path) { diff --git a/src/winxp_mutex.hpp b/src/winxp_mutex.hpp deleted file mode 100644 index e2c67bd..0000000 --- a/src/winxp_mutex.hpp +++ /dev/null @@ -1,58 +0,0 @@ -#pragma once - -#include - -// This class is a lightweight replacement for std::mutex on Windows platforms. -// std::mutex does not work on Windows XP SP2 with the latest VC++ libraries, -// because it utilizes the Concurrency Runtime that is only supported on Windows -// XP SP3 and above. - -// mon addition: avoid std::lock_guard. It uses thread local storage and is just, in general, pain to compile properly. -// This sucks, because RAII is awesome. - -class CriticalSectionLock { -public: - CriticalSectionLock() { InitializeCriticalSection(&critical_section_); } - ~CriticalSectionLock() { DeleteCriticalSection(&critical_section_); } - void lock() { EnterCriticalSection(&critical_section_); } - void unlock() { LeaveCriticalSection(&critical_section_); } - -private: - CRITICAL_SECTION critical_section_; -}; - -/** -Copyright 2008 Google Inc. All rights reserved. - -Redistribution and use in source and binary forms, with or without -modification, are permitted provided that the following conditions are -met: - - * Redistributions of source code must retain the above copyright -notice, this list of conditions and the following disclaimer. - * Redistributions in binary form must reproduce the above -copyright notice, this list of conditions and the following disclaimer -in the documentation and/or other materials provided with the -distribution. - * Neither the name of Google Inc. nor the names of its -contributors may be used to endorse or promote products derived from -this software without specific prior written permission. - -THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR -A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT -OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, -SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT -LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, -DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY -THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT -(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - -Code generated by the Protocol Buffer compiler is owned by the owner -of the input file used when generating it. This code is not -standalone and requires a support library to be linked with it. This -support library is itself covered by the above license. - -**/