Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ChdFileReader: Use core_file instead of modifing chd_open_file #12075

Merged
merged 1 commit into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion 3rdparty/libchdr/include/libchdr/chd.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 1 addition & 7 deletions 3rdparty/libchdr/src/libchdr_chd.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/*-------------------------------------------------
Expand Down
82 changes: 78 additions & 4 deletions pcsx2/CDVD/ChdFileReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,74 @@ static constexpr u32 MAX_PARENTS = 32; // Surely someone wouldn't be insane enou
static std::vector<std::pair<std::string, chd_header>> s_chd_hash_cache; // <filename, header>
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<ChdCoreFileWrapper*>(file->argp);
}

void SetFileOwner(bool isOwner)
{
m_free_file = isOwner;
}

private:
static u64 FSize(core_file* file)
{
return static_cast<u64>(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()
Expand All @@ -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;
}
Expand Down Expand Up @@ -140,16 +211,19 @@ 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)));
Error::SetString(error, chd_error_string(err));
return nullptr;
}

// fp now owned by libchdr
// core_wrapper should manage fp.
core_wrapper->SetFileOwner(true);
fp.release();
return chd;
}
Expand Down
Loading