From ceb3e10c65f23eeaad76a493926b23e16e377b8a Mon Sep 17 00:00:00 2001 From: TheLastRar Date: Mon, 9 Dec 2024 16:41:47 +0000 Subject: [PATCH] ChdFileReader: Use core_file instead of modifing chd_open_file --- 3rdparty/libchdr/include/libchdr/chd.h | 1 - 3rdparty/libchdr/src/libchdr_chd.c | 8 +-- pcsx2/CDVD/ChdFileReader.cpp | 82 ++++++++++++++++++++++++-- 3 files changed, 79 insertions(+), 12 deletions(-) diff --git a/3rdparty/libchdr/include/libchdr/chd.h b/3rdparty/libchdr/include/libchdr/chd.h index 506476136b252..d1e7a81dd16bc 100644 --- a/3rdparty/libchdr/include/libchdr/chd.h +++ b/3rdparty/libchdr/include/libchdr/chd.h @@ -259,7 +259,6 @@ extern "C" { /* CHD open values */ #define CHD_OPEN_READ 1 #define CHD_OPEN_READWRITE 2 -#define CHD_OPEN_TRANSFER_FILE 4 /* Freeing of the FILE* is now libchdr's responsibility if open was successful */ /* error types */ enum _chd_error diff --git a/3rdparty/libchdr/src/libchdr_chd.c b/3rdparty/libchdr/src/libchdr_chd.c index 30c7ecfc795cc..cac9720a172e1 100644 --- a/3rdparty/libchdr/src/libchdr_chd.c +++ b/3rdparty/libchdr/src/libchdr_chd.c @@ -1737,13 +1737,7 @@ CHD_EXPORT chd_error chd_open_file(FILE *file, int mode, chd_file *parent, chd_f stream->fclose = core_stdio_fclose_nonowner; stream->fseek = core_stdio_fseek; - chd_error err = chd_open_core_file(stream, mode, parent, chd); - if (err != CHDERR_NONE) - return err; - - // swap out the fclose so that we close it on chd clost - stream->fclose = core_stdio_fclose; - return CHDERR_NONE; + return chd_open_core_file(stream, mode, parent, chd); } /*------------------------------------------------- diff --git a/pcsx2/CDVD/ChdFileReader.cpp b/pcsx2/CDVD/ChdFileReader.cpp index 8cdb7e4193ad0..35d78c07dfe52 100644 --- a/pcsx2/CDVD/ChdFileReader.cpp +++ b/pcsx2/CDVD/ChdFileReader.cpp @@ -20,6 +20,74 @@ static constexpr u32 MAX_PARENTS = 32; // Surely someone wouldn't be insane enou static std::vector> s_chd_hash_cache; // static std::recursive_mutex s_chd_hash_cache_mutex; +// Provides an implementation of core_file which allows us to control if the underlying FILE handle is freed. +// The lifetime of ChdCoreFileWrapper will be equal to that of the relevant chd_file, +// ChdCoreFileWrapper will also get destroyed if chd_open_core_file fails. +class ChdCoreFileWrapper +{ + DeclareNoncopyableObject(ChdCoreFileWrapper); + +private: + core_file m_core; + std::FILE* m_file; + bool m_free_file = false; + +public: + ChdCoreFileWrapper(std::FILE* file) + : m_file{file} + { + m_core.argp = this; + m_core.fsize = FSize; + m_core.fread = FRead; + m_core.fclose = FClose; + m_core.fseek = FSeek; + } + + ~ChdCoreFileWrapper() + { + if (m_free_file) + std::fclose(m_file); + } + + core_file* GetCoreFile() + { + return &m_core; + } + + static ChdCoreFileWrapper* FromCoreFile(core_file* file) + { + return reinterpret_cast(file->argp); + } + + void SetFileOwner(bool isOwner) + { + m_free_file = isOwner; + } + +private: + static u64 FSize(core_file* file) + { + return static_cast(FileSystem::FSize64(FromCoreFile(file)->m_file)); + } + + static size_t FRead(void* buffer, size_t elmSize, size_t elmCount, core_file* file) + { + return std::fread(buffer, elmSize, elmCount, FromCoreFile(file)->m_file); + } + + static int FClose(core_file* file) + { + // Destructor handles freeing the FILE handle. + delete FromCoreFile(file); + return 0; + } + + static int FSeek(core_file* file, int64_t offset, int whence) + { + return FileSystem::FSeek64(FromCoreFile(file)->m_file, offset, whence); + } +}; + ChdFileReader::ChdFileReader() = default; ChdFileReader::~ChdFileReader() @@ -30,10 +98,13 @@ ChdFileReader::~ChdFileReader() static chd_file* OpenCHD(const std::string& filename, FileSystem::ManagedCFilePtr fp, Error* error, u32 recursion_level) { chd_file* chd; - chd_error err = chd_open_file(fp.get(), CHD_OPEN_READ | CHD_OPEN_TRANSFER_FILE, nullptr, &chd); + ChdCoreFileWrapper* core_wrapper = new ChdCoreFileWrapper(fp.get()); + // libchdr will take ownership of core_wrapper, and will close/free it on failure. + chd_error err = chd_open_core_file(core_wrapper->GetCoreFile(), CHD_OPEN_READ, nullptr, &chd); if (err == CHDERR_NONE) { - // fp is now managed by libchdr + // core_wrapper should manage fp. + core_wrapper->SetFileOwner(true); fp.release(); return chd; } @@ -140,8 +211,10 @@ static chd_file* OpenCHD(const std::string& filename, FileSystem::ManagedCFilePt return nullptr; } + // Our last core file wrapper got freed, so make a new one. + core_wrapper = new ChdCoreFileWrapper(fp.get()); // Now try re-opening with the parent. - err = chd_open_file(fp.get(), CHD_OPEN_READ | CHD_OPEN_TRANSFER_FILE, parent_chd, &chd); + err = chd_open_core_file(core_wrapper->GetCoreFile(), CHD_OPEN_READ, parent_chd, &chd); if (err != CHDERR_NONE) { Console.Error(fmt::format("Failed to open CHD '{}': {}", filename, chd_error_string(err))); @@ -149,7 +222,8 @@ static chd_file* OpenCHD(const std::string& filename, FileSystem::ManagedCFilePt return nullptr; } - // fp now owned by libchdr + // core_wrapper should manage fp. + core_wrapper->SetFileOwner(true); fp.release(); return chd; }