From 663b62e63f4c33352bb1ebd4a9ad2632265fd305 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=CE=A3=CF=84=CE=AD=CF=86=CE=B1=CE=BD=CE=BF=CF=82=20=22Coor?= =?UTF-8?q?nio/8924th=22=20=CE=92=CE=BB=CE=B1=CF=83=CF=84=CF=8C=CF=82?= <8924th@gmail.com> Date: Fri, 6 Dec 2024 04:25:43 +0200 Subject: [PATCH] another massive overhaul to file IO, better protect against errors and unexpected situations, improve general flexibility of methods, update relevant emulator code, resulting in considerably more robust file IO and cleaner code :D --- src/Assistants/HomeDirManager.cpp | 4 +- src/Assistants/SimpleFileIO.hpp | 121 ++++++++++--------- src/Systems/CHIP8/Chip8_CoreInterface.cpp | 134 ++++++++++------------ src/Systems/CHIP8/Chip8_CoreInterface.hpp | 15 ++- src/Systems/CHIP8/Cores/MEGACHIP.cpp | 6 +- src/Systems/CHIP8/Cores/SCHIP_LEGACY.cpp | 6 +- src/Systems/CHIP8/Cores/SCHIP_MODERN.cpp | 6 +- src/Systems/CHIP8/Cores/XOCHIP.cpp | 6 +- src/Systems/CHIP8/Cores/XOCHIP.hpp | 2 +- 9 files changed, 150 insertions(+), 150 deletions(-) diff --git a/src/Assistants/HomeDirManager.cpp b/src/Assistants/HomeDirManager.cpp index 74e2e95..6dcd1e5 100644 --- a/src/Assistants/HomeDirManager.cpp +++ b/src/Assistants/HomeDirManager.cpp @@ -102,8 +102,8 @@ bool HomeDirManager::validateGameFile(const Path gamePath) noexcept { if (checkGame( std::data(mFileData), std::size(mFileData), - gamePath.extension().string(), tempSHA1) - ) { + gamePath.extension().string(), tempSHA1 + )) { mFilePath = gamePath; mFileSHA1 = tempSHA1; return true; diff --git a/src/Assistants/SimpleFileIO.hpp b/src/Assistants/SimpleFileIO.hpp index 301e1ba..48541c8 100644 --- a/src/Assistants/SimpleFileIO.hpp +++ b/src/Assistants/SimpleFileIO.hpp @@ -100,81 +100,88 @@ namespace fs { /*==================================================================*/ [[maybe_unused]] -inline auto readFileData(const Path& filePath) noexcept --> std::expected, std::error_code> -{ - std::vector fileData{}; - - // get file's last write time stamp before we begin - auto fileModStampBegin{ fs::last_write_time(filePath) }; - if (!fileModStampBegin) { - return std::unexpected(std::move(fileModStampBegin.error())); - } - - std::ifstream ifs(filePath, std::ios::binary); - - // check if the stream was unable to access the file - if (!ifs) { - return std::unexpected(std::make_error_code(std::errc::permission_denied)); - } - - // ensure we could copy all the data into the vector +inline auto readFileData( + const Path& filePath, const usz dataReadSize = 0, + const std::streamoff dataReadOffset = 0 +) noexcept -> std::expected, std::error_code> { try { - fileData.assign(std::istreambuf_iterator(ifs), {}); - } catch (const std::exception&) { - return std::unexpected(std::make_error_code(std::errc::not_enough_memory)); - } - - // check if we failed in reading the file properly - if (!ifs.good()) { + std::vector fileData{}; + + auto fileModStampBegin{ fs::last_write_time(filePath) }; + if (!fileModStampBegin) { return std::unexpected(std::move(fileModStampBegin.error())); } + + std::ifstream inFile(filePath, std::ios::binary); + if (!inFile) { return std::unexpected(std::make_error_code(std::errc::permission_denied)); } + + inFile.seekg(static_cast(dataReadOffset)); + if (!inFile) { return std::unexpected(std::make_error_code(std::errc::invalid_argument)); } + + if (dataReadSize) { + fileData.resize(dataReadSize); + inFile.read(fileData.data(), dataReadSize); + } else { + try { + fileData.assign(std::istreambuf_iterator(inFile), {}); + } catch (const std::exception&) { + return std::unexpected(std::make_error_code(std::errc::not_enough_memory)); + } + } + + if (!inFile.good()) { throw std::exception{}; } + + auto fileModStampEnd{ fs::last_write_time(filePath) }; + if (!fileModStampEnd) { return std::unexpected(std::move(fileModStampEnd.error())); } + + if (fileModStampBegin.value() != fileModStampEnd.value()) { + return std::unexpected(std::make_error_code(std::errc::interrupted)); + } else { return fileData; } + } + catch (const std::exception&) { return std::unexpected(std::make_error_code(std::errc::io_error)); } - - // get file's last write time stamp after our read ended - const auto fileModStampEnd{ fs::last_write_time(filePath) }; - if (!fileModStampEnd) { - return std::unexpected(std::move(fileModStampEnd.error())); - } - - // check if both timestamps are still the same - if (fileModStampBegin.value() != fileModStampEnd.value()) { - return std::unexpected(std::make_error_code(std::errc::interrupted)); - } - - return fileData; } /*==================================================================*/ template [[maybe_unused]] -inline auto writeFileData(const Path& filePath, const T* fileData, const usz fileSize) noexcept --> std::expected -{ - std::ofstream ofs(filePath, std::ios::binary); - - // check if the stream was unable to access the file - if (!ofs) { - return std::unexpected(std::make_error_code(std::errc::permission_denied)); - } - - // check if we failed in writing the file properly +inline auto writeFileData( + const Path& filePath, const T* fileData, const usz dataWriteSize, + const std::streamoff dataWriteOffset = 0 +) noexcept -> std::expected { try { - ofs.write(reinterpret_cast(fileData), fileSize * sizeof(T)); - if (!ofs.good()) { throw std::exception{}; } else { return {}; } - } catch (const std::exception&) { + std::ofstream outFile(filePath, std::ios::binary | std::ios::out); + if (!outFile) { return std::unexpected(std::make_error_code(std::errc::permission_denied)); } + + outFile.seekp(static_cast(dataWriteOffset)); + if (!outFile) { return std::unexpected(std::make_error_code(std::errc::invalid_argument)); } + + outFile.write(reinterpret_cast(fileData), dataWriteSize * sizeof(T)); + if (!outFile.good()) { throw std::exception{}; } else { return {}; } + } + catch (const std::exception&) { return std::unexpected(std::make_error_code(std::errc::io_error)); } } template [[maybe_unused]] -inline auto writeFileData(const Path& filePath, const T& fileData) noexcept { - return writeFileData(filePath, std::data(fileData), std::size(fileData)); +inline auto writeFileData( + const Path& filePath, const T& fileData, const usz dataWriteSize = 0, + const std::streamoff dataWriteOffset = 0 +) noexcept { + return writeFileData( + filePath, std::data(fileData), dataWriteSize + ? dataWriteSize : std::size(fileData), + dataWriteOffset + ); } template [[maybe_unused]] -inline auto writeFileData(const Path& filePath, const T(&fileData)[N]) noexcept { - return writeFileData(filePath, fileData, N); +inline auto writeFileData( + const Path& filePath, const T(&fileData)[N], + const std::streamoff dataWriteOffset = 0 +) noexcept { + return writeFileData(filePath, fileData, N, dataWriteOffset); } diff --git a/src/Systems/CHIP8/Chip8_CoreInterface.cpp b/src/Systems/CHIP8/Chip8_CoreInterface.cpp index 97ac6e4..8dd33ba 100644 --- a/src/Systems/CHIP8/Chip8_CoreInterface.cpp +++ b/src/Systems/CHIP8/Chip8_CoreInterface.cpp @@ -20,9 +20,11 @@ Chip8_CoreInterface::Chip8_CoreInterface() noexcept : ASB{ std::make_unique(SDL_AUDIO_S8, 1, 48'000, STREAM::COUNT) } { sSavestatePath = HDM->addSystemDir("savestate", "CHIP8"); - if (!sSavestatePath) { setCoreState(EmuState::FATAL); } + if (sSavestatePath) { *sSavestatePath /= HDM->getFileSHA1(); } sPermaRegsPath = HDM->addSystemDir("permaRegs", "CHIP8"); - if (!sPermaRegsPath) { setCoreState(EmuState::FATAL); } + if (sPermaRegsPath) { *sPermaRegsPath /= HDM->getFileSHA1(); } + + if (!checkFileValidity(sPermaRegsPath)) { sPermaRegsPath = nullptr; } ASB->resumeStreams(); loadPresetBinds(); @@ -224,103 +226,93 @@ void Chip8_CoreInterface::triggerInterrupt(const Interrupt type) noexcept { mActiveCPF = -std::abs(mActiveCPF); } -void Chip8_CoreInterface::triggerCritError(const Str& msg) noexcept { - blog.newEntry(BLOG::WARN, msg); - triggerInterrupt(Interrupt::ERROR); -} - /*==================================================================*/ -bool Chip8_CoreInterface::setPermaRegs(const u32 X) noexcept { - const auto path{ *sPermaRegsPath / HDM->getFileSHA1() }; +bool Chip8_CoreInterface::checkFileValidity(const Path* filePath) noexcept { + if (!filePath) { return false; } - const auto fileExists{ fs::is_regular_file(path) }; + const auto fileExists{ fs::exists(*filePath) }; if (!fileExists) { - blog.newEntry(BLOG::ERROR, "Path is ineligible: \"{}\" [{}]", - path.string(), fileExists.error().message() + blog.newEntry(BLOG::ERROR, "\"{}\" [{}]", + filePath->string(), fileExists.error().message() ); return false; } if (fileExists.value()) { - auto regsData{ readFileData(path) }; - - if (!regsData) { - blog.newEntry(BLOG::ERROR, "File IO error: \"{}\" [{}]", - path.string(), regsData.error().message() - ); - return false; - } - if (regsData.value().size() > mRegisterV.size()) { - blog.newEntry(BLOG::ERROR, "File is too large: \"{}\" [{} bytes]", - path.string(), regsData.value().size() + const auto fileNormal{ fs::is_regular_file(*filePath) }; + if (!fileNormal) { + blog.newEntry(BLOG::ERROR, "\"{}\" [{}]", + filePath->string(), fileExists.error().message() ); return false; } - regsData.value().resize(mRegisterV.size()); - std::copy_n(mRegisterV.begin(), X, regsData.value().begin()); + if (fileNormal.value()) { return true; } + else { + const auto fileRemove{ fs::remove(*filePath) }; + if (!fileRemove) { + blog.newEntry(BLOG::ERROR, "\"{}\" [{}]", + filePath->string(), fileExists.error().message() + ); + return false; + } - auto fileWritten{ writeFileData(path, regsData.value()) }; - if (!fileWritten) { - blog.newEntry(BLOG::ERROR, "File IO error: \"{}\" [{}]", - path.string(), fileWritten.error().message() - ); - return false; - } else { - return true; + if (fileRemove.value()) { return true; } + else { + blog.newEntry(BLOG::WARN, "{}: \"{}\"", + "Cannot remove irregular file", filePath->string() + ); + return false; + } } } else { - std::error_code error_code; - char regsData[sizeof(mRegisterV)]{}; - std::copy_n(mRegisterV.begin(), X, regsData); - - auto fileWritten{ writeFileData(path, regsData) }; - if (!fileWritten) { - blog.newEntry(BLOG::ERROR, "File IO error: \"{}\" [{}]", - path.string(), fileWritten.error().message() + const char blankRegs[sPermRegsV.size()]{}; + const auto fileWritten{ writeFileData(*filePath, blankRegs) }; + if (fileWritten) { return true; } + else { + blog.newEntry(BLOG::WARN, "{}: \"{}\"", + "Cannot write new file", filePath->string() ); return false; - } else { - return true; } } } -bool Chip8_CoreInterface::getPermaRegs(const u32 X) noexcept { - const auto path{ *sPermaRegsPath / HDM->getFileSHA1() }; - - const auto fileExists{ fs::is_regular_file(path) }; - if (!fileExists) { - blog.newEntry(BLOG::ERROR, "Path is ineligible: \"{}\" [{}]", - path.string(), fileExists.error().message() +void Chip8_CoreInterface::setFilePermaRegs(const u32 X) noexcept { + auto fileData{ writeFileData(*sPermaRegsPath, mRegisterV, X) }; + if (!fileData) { + blog.newEntry(BLOG::ERROR, "File IO error: \"{}\" [{}]", + sPermaRegsPath->string(), fileData.error().message() ); - return false; } +} - if (fileExists) { - auto regsData{ readFileData(path) }; +void Chip8_CoreInterface::getFilePermaRegs(const u32 X) noexcept { + auto fileData{ readFileData(*sPermaRegsPath, X) }; + if (!fileData) { + blog.newEntry(BLOG::ERROR, "File IO error: \"{}\" [{}]", + sPermaRegsPath->string(), fileData.error().message() + ); + } else { + std::copy_n(fileData.value().begin(), X, sPermRegsV.begin()); + } +} - if (!regsData) { - blog.newEntry(BLOG::ERROR, "File IO error: \"{}\" [{}]", - path.string(), regsData.error().message() - ); - return false; - } - if (regsData.value().size() > mRegisterV.size()) { - blog.newEntry(BLOG::ERROR, "File is too large: \"{}\" [{} bytes]", - path.string(), regsData.value().size() - ); - return false; - } +void Chip8_CoreInterface::setPermaRegs(const u32 X) noexcept { + if (sPermaRegsPath) { + if (checkFileValidity(sPermaRegsPath)) { setFilePermaRegs(X); } + else { sPermaRegsPath = nullptr; } + } + std::copy_n(mRegisterV.begin(), X, sPermRegsV.begin()); +} - regsData.value().resize(mRegisterV.size()); - std::copy_n(regsData.value().begin(), X, mRegisterV.begin()); - return true; - } else { - std::fill_n(mRegisterV.begin(), X, u8{}); - return true; +void Chip8_CoreInterface::getPermaRegs(const u32 X) noexcept { + if (sPermaRegsPath) { + if (checkFileValidity(sPermaRegsPath)) { getFilePermaRegs(X); } + else { sPermaRegsPath = nullptr; } } + std::copy_n(sPermRegsV.begin(), X, mRegisterV.begin()); } /*==================================================================*/ diff --git a/src/Systems/CHIP8/Chip8_CoreInterface.hpp b/src/Systems/CHIP8/Chip8_CoreInterface.hpp index 2663fbe..5a0f802 100644 --- a/src/Systems/CHIP8/Chip8_CoreInterface.hpp +++ b/src/Systems/CHIP8/Chip8_CoreInterface.hpp @@ -154,6 +154,10 @@ class Chip8_CoreInterface : public EmuInterface { u32 mStackTop{}; u8* mInputReg{}; + inline static + std::array + sPermRegsV{}; + std::array mRegisterV{}; @@ -165,10 +169,15 @@ class Chip8_CoreInterface : public EmuInterface { void instructionError(const u32 HI, const u32 LO); void triggerInterrupt(const Interrupt type) noexcept; - void triggerCritError(const Str& msg) noexcept; - bool setPermaRegs(const u32 X) noexcept; - bool getPermaRegs(const u32 X) noexcept; +private: + bool checkFileValidity(const Path* filePath) noexcept; + void setFilePermaRegs(const u32 X) noexcept; + void getFilePermaRegs(const u32 X) noexcept; + +protected: + void setPermaRegs(const u32 X) noexcept; + void getPermaRegs(const u32 X) noexcept; void copyGameToMemory(void* dest) noexcept; void copyFontToMemory(void* dest, const usz size) noexcept; diff --git a/src/Systems/CHIP8/Cores/MEGACHIP.cpp b/src/Systems/CHIP8/Cores/MEGACHIP.cpp index fb7e7e5..ce451d4 100644 --- a/src/Systems/CHIP8/Cores/MEGACHIP.cpp +++ b/src/Systems/CHIP8/Cores/MEGACHIP.cpp @@ -1036,12 +1036,10 @@ void MEGACHIP::scrollBuffersRT() { { mRegisterV[idx] = readMemoryI(idx); } } void MEGACHIP::instruction_FN75(const s32 N) noexcept { - if (!setPermaRegs(std::min(N, 7) + 1)) [[unlikely]] - { triggerCritError("Error :: Failed writing persistent registers!"); } + setPermaRegs(std::min(N, 7) + 1); } void MEGACHIP::instruction_FN85(const s32 N) noexcept { - if (!getPermaRegs(std::min(N, 7) + 1)) [[unlikely]] - { triggerCritError("Error :: Failed reading persistent registers!"); } + getPermaRegs(std::min(N, 7) + 1); } #pragma endregion diff --git a/src/Systems/CHIP8/Cores/SCHIP_LEGACY.cpp b/src/Systems/CHIP8/Cores/SCHIP_LEGACY.cpp index 734eeb0..9683fa2 100644 --- a/src/Systems/CHIP8/Cores/SCHIP_LEGACY.cpp +++ b/src/Systems/CHIP8/Cores/SCHIP_LEGACY.cpp @@ -642,12 +642,10 @@ void SCHIP_LEGACY::scrollDisplayRT() { { mRegisterI = mRegisterI + N & 0xFFF; } } void SCHIP_LEGACY::instruction_FN75(const s32 N) noexcept { - if (!setPermaRegs(std::min(N, 7) + 1)) [[unlikely]] - { triggerCritError("Error :: Failed writing persistent registers!"); } + setPermaRegs(std::min(N, 7) + 1); } void SCHIP_LEGACY::instruction_FN85(const s32 N) noexcept { - if (!getPermaRegs(std::min(N, 7) + 1)) [[unlikely]] - { triggerCritError("Error :: Failed reading persistent registers!"); } + getPermaRegs(std::min(N, 7) + 1); } #pragma endregion diff --git a/src/Systems/CHIP8/Cores/SCHIP_MODERN.cpp b/src/Systems/CHIP8/Cores/SCHIP_MODERN.cpp index 0f147be..9740a14 100644 --- a/src/Systems/CHIP8/Cores/SCHIP_MODERN.cpp +++ b/src/Systems/CHIP8/Cores/SCHIP_MODERN.cpp @@ -616,12 +616,10 @@ void SCHIP_MODERN::scrollDisplayRT() { { mRegisterI = mRegisterI + N + 1 & 0xFFF; } } void SCHIP_MODERN::instruction_FN75(const s32 N) noexcept { - if (!setPermaRegs(N + 1)) [[unlikely]] - { triggerCritError("Error :: Failed writing persistent registers!"); } + setPermaRegs(N + 1); } void SCHIP_MODERN::instruction_FN85(const s32 N) noexcept { - if (!getPermaRegs(N + 1)) [[unlikely]] - { triggerCritError("Error :: Failed reading persistent registers!"); } + getPermaRegs(N + 1); } #pragma endregion diff --git a/src/Systems/CHIP8/Cores/XOCHIP.cpp b/src/Systems/CHIP8/Cores/XOCHIP.cpp index 079d176..cc1ef11 100644 --- a/src/Systems/CHIP8/Cores/XOCHIP.cpp +++ b/src/Systems/CHIP8/Cores/XOCHIP.cpp @@ -772,12 +772,10 @@ void XOCHIP::scrollDisplayRT() { { mRegisterI = mRegisterI + N + 1 & 0xFFFF; } } void XOCHIP::instruction_FN75(const s32 N) noexcept { - if (!setPermaRegs(N + 1)) [[unlikely]] - { triggerCritError("Error :: Failed writing persistent registers!"); } + setPermaRegs(N + 1); } void XOCHIP::instruction_FN85(const s32 N) noexcept { - if (!getPermaRegs(N + 1)) [[unlikely]] - { triggerCritError("Error :: Failed reading persistent registers!"); } + getPermaRegs(N + 1); } #pragma endregion diff --git a/src/Systems/CHIP8/Cores/XOCHIP.hpp b/src/Systems/CHIP8/Cores/XOCHIP.hpp index 4b953ad..6dbe5dc 100644 --- a/src/Systems/CHIP8/Cores/XOCHIP.hpp +++ b/src/Systems/CHIP8/Cores/XOCHIP.hpp @@ -13,7 +13,7 @@ /*==================================================================*/ class XOCHIP final : public Chip8_CoreInterface { - static constexpr u32 cTotalMemory{ ::CalcBytes(16, KiB) }; + static constexpr u32 cTotalMemory{ ::CalcBytes(64, KiB) }; static constexpr u32 cSafezoneOOB{ 32 }; static constexpr u32 cGameLoadPos{ 512 }; static constexpr u32 cStartOffset{ 512 };