From 267e081592d7119cd04ae1d9c7ad569126d23e35 Mon Sep 17 00:00:00 2001 From: Jordan Woyak Date: Fri, 7 Nov 2025 18:37:04 -0600 Subject: [PATCH 1/4] Common/DirectIOFile: Make Open return an OpenError on failure to differentiate between a file already existing vs. some other error. open mode fixup --- Source/Core/Common/DirectIOFile.cpp | 16 +++++++++------- Source/Core/Common/DirectIOFile.h | 16 ++++++++++++++-- Source/UnitTests/Common/FileUtilTest.cpp | 8 +++++++- 3 files changed, 30 insertions(+), 10 deletions(-) diff --git a/Source/Core/Common/DirectIOFile.cpp b/Source/Core/Common/DirectIOFile.cpp index d72e98fa3bb..8bb07885316 100644 --- a/Source/Core/Common/DirectIOFile.cpp +++ b/Source/Core/Common/DirectIOFile.cpp @@ -68,7 +68,7 @@ DirectIOFile::DirectIOFile(const std::string& path, AccessMode access_mode, Open Open(path, access_mode, open_mode); } -bool DirectIOFile::Open(const std::string& path, AccessMode access_mode, OpenMode open_mode) +OpenResult DirectIOFile::Open(const std::string& path, AccessMode access_mode, OpenMode open_mode) { ASSERT(!IsOpen()); @@ -77,7 +77,7 @@ bool DirectIOFile::Open(const std::string& path, AccessMode access_mode, OpenMod // This is not a sensible combination. Fail here to not rely on OS-specific behaviors. if (access_mode == AccessMode::Read && open_mode == OpenMode::Truncate) - return false; + return OpenError::InvalidArguments; #if defined(_WIN32) DWORD desired_access = GENERIC_READ | GENERIC_WRITE; @@ -102,8 +102,8 @@ bool DirectIOFile::Open(const std::string& path, AccessMode access_mode, OpenMod m_handle = CreateFile(UTF8ToTStr(path).c_str(), desired_access, share_mode, nullptr, creation_disposition, FILE_ATTRIBUTE_NORMAL, nullptr); - if (!IsOpen()) - WARN_LOG_FMT(COMMON, "CreateFile: {}", Common::GetLastErrorString()); + if (!IsOpen() && GetLastError() == ERROR_FILE_EXISTS) + return OpenError::AlreadyExists; #else #if defined(ANDROID) @@ -127,7 +127,7 @@ bool DirectIOFile::Open(const std::string& path, AccessMode access_mode, OpenMod if (open_mode == OpenMode::Create) { ASSERT_MSG(COMMON, false, "DirectIOFile doesn't support creating SAF files"); - return false; + return OpenError::InvalidArguments; } m_fd = OpenAndroidContent(path, open_mode_str); @@ -135,7 +135,7 @@ bool DirectIOFile::Open(const std::string& path, AccessMode access_mode, OpenMod if (!IsOpen() && (open_mode == OpenMode::Always || open_mode == OpenMode::Truncate)) ASSERT_MSG(COMMON, Exists(path), "DirectIOFile doesn't support creating SAF files"); - return IsOpen(); + return IsOpen() ? OpenResult{OpenSuccess{}} : OpenError::Other; } #endif int flags = O_RDWR; @@ -152,10 +152,12 @@ bool DirectIOFile::Open(const std::string& path, AccessMode access_mode, OpenMod flags |= O_CREAT; m_fd = open(path.c_str(), flags, 0666); + if (!IsOpen() && errno == EEXIST) + return OpenError::AlreadyExists; #endif - return IsOpen(); + return IsOpen() ? OpenResult{OpenSuccess{}} : OpenError::Other; } bool DirectIOFile::Close() diff --git a/Source/Core/Common/DirectIOFile.h b/Source/Core/Common/DirectIOFile.h index 79a5e68a2c1..9f3a8e5454d 100644 --- a/Source/Core/Common/DirectIOFile.h +++ b/Source/Core/Common/DirectIOFile.h @@ -8,6 +8,7 @@ #include "Common/CommonTypes.h" #include "Common/IOFile.h" +#include "Common/Result.h" namespace File { @@ -39,6 +40,17 @@ enum class OpenMode Create, }; +enum class OpenError +{ + AlreadyExists, // May be returned when using `OpenMode::Create`. + InvalidArguments, + Other, +}; + +enum class OpenSuccess; + +using OpenResult = Common::Result; + // This file wrapper avoids use of the underlying system file position. // It keeps track of its own file position and read/write calls directly use it. // This makes copied handles entirely thread safe. @@ -57,8 +69,8 @@ public: explicit DirectIOFile(const std::string& path, AccessMode access_mode, OpenMode open_mode = OpenMode::Default); - bool Open(const std::string& path, AccessMode access_mode, - OpenMode open_mode = OpenMode::Default); + OpenResult Open(const std::string& path, AccessMode access_mode, + OpenMode open_mode = OpenMode::Default); bool Close(); diff --git a/Source/UnitTests/Common/FileUtilTest.cpp b/Source/UnitTests/Common/FileUtilTest.cpp index bfdc9128f6b..ba5ed1b5b25 100644 --- a/Source/UnitTests/Common/FileUtilTest.cpp +++ b/Source/UnitTests/Common/FileUtilTest.cpp @@ -194,7 +194,13 @@ TEST_F(FileUtilTest, DirectIOFile) // Note: Double Open() currently ASSERTs. It's not obvious if that should succeed or fail. EXPECT_TRUE(file.Close()); - EXPECT_FALSE(file.Open(m_file_path, File::AccessMode::Write, File::OpenMode::Create)); + { + // OpenMode::Create properly returns AlreadyExists. + const auto open_result = + file.Open(m_file_path, File::AccessMode::Write, File::OpenMode::Create); + EXPECT_TRUE(!open_result.Succeeded() && + (open_result.Error() == File::OpenError::AlreadyExists)); + } EXPECT_TRUE(file.Open(m_file_path, File::AccessMode::Write, File::OpenMode::Existing)); EXPECT_TRUE(file.Close()); From c84f78f05846b57705c447f4323cdf0c19d93c0a Mon Sep 17 00:00:00 2001 From: Jordan Woyak Date: Fri, 7 Nov 2025 21:25:42 -0600 Subject: [PATCH 2/4] Common/FileUtil: Add CreateTempFileForAtomicWrite function. --- Source/Core/Common/FileUtil.cpp | 34 ++++++++++++++++++++++-- Source/Core/Common/FileUtil.h | 4 +++ Source/UnitTests/Common/FileUtilTest.cpp | 11 ++++++++ 3 files changed, 47 insertions(+), 2 deletions(-) diff --git a/Source/Core/Common/FileUtil.cpp b/Source/Core/Common/FileUtil.cpp index bcfd74256d9..d6c7599ced7 100644 --- a/Source/Core/Common/FileUtil.cpp +++ b/Source/Core/Common/FileUtil.cpp @@ -25,6 +25,7 @@ #include "Common/CommonFuncs.h" #include "Common/CommonPaths.h" #include "Common/CommonTypes.h" +#include "Common/DirectIOFile.h" #ifdef __APPLE__ #include "Common/DynamicLibrary.h" #endif @@ -668,13 +669,42 @@ std::string CreateTempDir() #endif } -std::string GetTempFilenameForAtomicWrite(std::string path) +static auto TryToGetAbsolutePath(std::string path) { std::error_code error; auto absolute_path = fs::absolute(StringToPath(path), error); if (!error) path = PathToString(absolute_path); - return std::move(path) + ".xxx"; + return path; +} + +std::string GetTempFilenameForAtomicWrite(std::string path) +{ + return TryToGetAbsolutePath(std::move(path)) + ".xxx"; +} + +std::string CreateTempFileForAtomicWrite(std::string path) +{ + path = TryToGetAbsolutePath(std::move(path)); + while (true) + { + DirectIOFile file; + + // e.g. "/dir/file.txt" -> "/dir/file.txt.189234789.tmp" + const auto timestamp = Clock::now().time_since_epoch().count(); + std::string tmp_path = fmt::format("{}.{}.tmp", path, timestamp); + + const auto open_result = file.Open(tmp_path, AccessMode::Write, OpenMode::Create); + if (open_result.Succeeded()) + return tmp_path; + + // In the very unlikely case that the file already exists, we will try again. + if (open_result.Error() == File::OpenError::AlreadyExists) + continue; + + // Failure. + return {}; + } } #if defined(__APPLE__) diff --git a/Source/Core/Common/FileUtil.h b/Source/Core/Common/FileUtil.h index 37548acfed0..585614b6f90 100644 --- a/Source/Core/Common/FileUtil.h +++ b/Source/Core/Common/FileUtil.h @@ -223,6 +223,10 @@ std::string CreateTempDir(); // Get a filename that can hopefully be atomically renamed to the given path. std::string GetTempFilenameForAtomicWrite(std::string path); +// Creates and returns the path to a newly created temporary file next to the given path. +// Returns an empty string on error, generally caused by lack of write permissions. +std::string CreateTempFileForAtomicWrite(std::string path); + // Gets a set user directory path // Don't call prior to setting the base user directory const std::string& GetUserPath(unsigned int dir_index); diff --git a/Source/UnitTests/Common/FileUtilTest.cpp b/Source/UnitTests/Common/FileUtilTest.cpp index ba5ed1b5b25..ce90b2729e1 100644 --- a/Source/UnitTests/Common/FileUtilTest.cpp +++ b/Source/UnitTests/Common/FileUtilTest.cpp @@ -154,6 +154,17 @@ TEST_F(FileUtilTest, CreateFullPath) EXPECT_TRUE(File::IsFile(p3file)); } +TEST_F(FileUtilTest, CreateTempFileForAtomicWrite) +{ + EXPECT_TRUE(File::Exists(File::CreateTempFileForAtomicWrite(m_file_path))); + +#if defined(_WIN32) + EXPECT_FALSE(File::Exists(File::CreateTempFileForAtomicWrite("C:/con/cant_write_here.txt"))); +#else + EXPECT_FALSE(File::Exists(File::CreateTempFileForAtomicWrite("/dev/null/cant_write_here.txt"))); +#endif +} + TEST_F(FileUtilTest, DirectIOFile) { static constexpr std::array u8_test_data = {42, 7, 99}; From a1dd36fbbeb5f1481be0048ead17a43cb674b140 Mon Sep 17 00:00:00 2001 From: Jordan Woyak Date: Fri, 7 Nov 2025 21:25:53 -0600 Subject: [PATCH 3/4] Common/FileUtil: Add AtomicWriteHelper class. --- Source/Core/Common/FileUtil.cpp | 30 ++++++++++++++++++++++++++++++ Source/Core/Common/FileUtil.h | 30 ++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/Source/Core/Common/FileUtil.cpp b/Source/Core/Common/FileUtil.cpp index d6c7599ced7..dbddeb5783f 100644 --- a/Source/Core/Common/FileUtil.cpp +++ b/Source/Core/Common/FileUtil.cpp @@ -1077,4 +1077,34 @@ bool ReadFileToString(const std::string& filename, std::string& str) return file.ReadArray(str.data(), str.size()); } +AtomicWriteHelper::AtomicWriteHelper(DirectIOFile* file, std::string path) + : m_path{std::move(path)}, m_temp_path{File::CreateTempFileForAtomicWrite(m_path)}, + m_file{*file} + +{ + m_file.Open(m_temp_path, File::AccessMode::Write); +} + +AtomicWriteHelper::~AtomicWriteHelper() +{ + Delete(m_file, m_temp_path); + m_file.Close(); +} + +const std::string& AtomicWriteHelper::GetTempPath() const +{ + return m_temp_path; +} + +bool AtomicWriteHelper::Finalize() +{ + if (Rename(m_file, m_temp_path, m_path)) + { + m_file.Close(); + return true; + } + + return false; +} + } // namespace File diff --git a/Source/Core/Common/FileUtil.h b/Source/Core/Common/FileUtil.h index 585614b6f90..f7bccaa0bc3 100644 --- a/Source/Core/Common/FileUtil.h +++ b/Source/Core/Common/FileUtil.h @@ -274,4 +274,34 @@ void OpenFStream(T& fstream, const std::string& filename, std::ios_base::openmod #endif } +class DirectIOFile; + +// This class opens a temporary file next to the given path. +// Do all your writing to the file, then use Finalize() to rename and close the temporay file. +// Letting the helper go out of scope while the file is open will instead delete the file. +class AtomicWriteHelper +{ +public: + explicit AtomicWriteHelper(DirectIOFile* file, std::string path); + + // Moves the temporay file to the target path then closes the file. + // Failure to rename leaves the file open and returns false. + bool Finalize(); + + // If the file is open during destruction, it will be deleted. + ~AtomicWriteHelper(); + + const std::string& GetTempPath() const; + + AtomicWriteHelper(const AtomicWriteHelper&) = delete; + AtomicWriteHelper& operator=(AtomicWriteHelper&&) = delete; + AtomicWriteHelper(AtomicWriteHelper&&) = delete; + AtomicWriteHelper& operator=(const AtomicWriteHelper&) = delete; + +private: + const std::string m_path; + const std::string m_temp_path; + File::DirectIOFile& m_file; +}; + } // namespace File From da5d226c1d85005952a9a95bf14cc100550784f3 Mon Sep 17 00:00:00 2001 From: Jordan Woyak Date: Fri, 7 Nov 2025 21:22:12 -0600 Subject: [PATCH 4/4] DiscIO: Make disc conversion use AtomicWriteHelper. Consolidate duplicate error messaging. --- Source/Android/jni/MainAndroid.cpp | 19 ++--- Source/Core/DiscIO/Blob.cpp | 79 +++++++++++++++++++ Source/Core/DiscIO/Blob.h | 32 +++++--- Source/Core/DiscIO/CompressedBlob.cpp | 49 +++--------- Source/Core/DiscIO/FileBlob.cpp | 88 ++++++++------------- Source/Core/DiscIO/FileBlob.h | 4 +- Source/Core/DiscIO/WIABlob.cpp | 48 +++-------- Source/Core/DolphinQt/ConvertDialog.cpp | 92 ++++++++++------------ Source/Core/DolphinTool/ConvertCommand.cpp | 25 +++--- 9 files changed, 221 insertions(+), 215 deletions(-) diff --git a/Source/Android/jni/MainAndroid.cpp b/Source/Android/jni/MainAndroid.cpp index 7c78f49045b..4fe32622dc1 100644 --- a/Source/Android/jni/MainAndroid.cpp +++ b/Source/Android/jni/MainAndroid.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -739,25 +740,23 @@ JNIEXPORT jboolean JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_ConvertD return static_cast(result); }; - bool success = false; - + DiscIO::ConversionFunction conversion_function; switch (format) { case DiscIO::BlobType::PLAIN: - success = DiscIO::ConvertToPlain(blob_reader.get(), in_path, out_path, callback); + conversion_function = DiscIO::ConvertToPlain; break; case DiscIO::BlobType::GCZ: - success = - DiscIO::ConvertToGCZ(blob_reader.get(), in_path, out_path, - platform == DiscIO::Platform::WiiDisc ? 1 : 0, jBlockSize, callback); + conversion_function = std::bind_front( + DiscIO::ConvertToGCZ, platform == DiscIO::Platform::WiiDisc ? 1 : 0, jBlockSize); break; case DiscIO::BlobType::WIA: case DiscIO::BlobType::RVZ: - success = DiscIO::ConvertToWIAOrRVZ(blob_reader.get(), in_path, out_path, - format == DiscIO::BlobType::RVZ, compression, - jCompressionLevel, jBlockSize, callback); + conversion_function = + std::bind_front(DiscIO::ConvertToWIAOrRVZ, format == DiscIO::BlobType::RVZ, compression, + jCompressionLevel, jBlockSize); break; default: @@ -765,6 +764,8 @@ JNIEXPORT jboolean JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_ConvertD break; } + const bool success = + DiscIO::ConvertBlob(conversion_function, std::move(blob_reader), in_path, out_path, callback); return static_cast(success); } diff --git a/Source/Core/DiscIO/Blob.cpp b/Source/Core/DiscIO/Blob.cpp index aad8b60b5e5..b869019ab7f 100644 --- a/Source/Core/DiscIO/Blob.cpp +++ b/Source/Core/DiscIO/Blob.cpp @@ -11,6 +11,7 @@ #include "Common/BitUtils.h" #include "Common/CommonTypes.h" #include "Common/DirectIOFile.h" +#include "Common/FileUtil.h" #include "Common/MsgHandler.h" #include "DiscIO/CISOBlob.h" @@ -251,4 +252,82 @@ std::unique_ptr CreateBlobReader(const std::string& filename) } } +bool ConvertBlob(const ConversionFunction& conversion_function, std::unique_ptr infile, + std::string_view infile_path, const std::string& outfile_path, + const CompressCB& callback) +{ + File::DirectIOFile outfile; + + // Writing to and renaming a temporary file won't work with Android SAF. +#if !defined(ANDROID) + // Since we write to a temporary file it probably makes sense to first delete the destination. + // Users may be doing conversions with limited storage space. + if (!File::Delete(outfile_path)) + { + PanicAlertFmtT( + "Failed to delete existing file \"{0}\".\n" + "Check that you have permissions to write the target folder and that the media can " + "be written.", + outfile_path); + return false; + } + + File::AtomicWriteHelper atomic_write_helper{&outfile, outfile_path}; + if (!outfile.IsOpen()) + { + PanicAlertFmtT( + "Failed to create temporary file for \"{0}\".\n" + "Check that you have permissions to write the target folder and that the media can " + "be written.", + outfile_path); + return false; + } + const auto& temp_path = atomic_write_helper.GetTempPath(); +#else + outfile.Open(outfile_path, File::AccessMode::Write); + if (!outfile.IsOpen()) + { + PanicAlertFmtT( + "Failed to open the output file \"{0}\".\n" + "Check that you have permissions to write the target folder and that the media can " + "be written.", + outfile_path); + return false; + } + const auto& temp_path = outfile_path; +#endif + + const auto result = conversion_function(std::move(infile), outfile, callback); + switch (result) + { + case ConversionResultCode::ReadFailed: + PanicAlertFmtT("Failed to read from the input file \"{0}\".", infile_path); + return false; + + case ConversionResultCode::WriteFailed: + PanicAlertFmtT("Failed to write the output file \"{0}\".\n" + "Check that you have enough space available on the target drive.", + temp_path); + return false; + + case ConversionResultCode::InternalError: + case ConversionResultCode::Canceled: + default: + return false; + + case ConversionResultCode::Success: + break; + } + +#if !defined(ANDROID) + if (!atomic_write_helper.Finalize()) + { + PanicAlertFmtT("Failed to rename file from \"{0}\" to \"{1}\".", temp_path, outfile_path); + return false; + } +#endif + + return true; +} + } // namespace DiscIO diff --git a/Source/Core/DiscIO/Blob.h b/Source/Core/DiscIO/Blob.h index 841a827b976..9358193ac98 100644 --- a/Source/Core/DiscIO/Blob.h +++ b/Source/Core/DiscIO/Blob.h @@ -23,6 +23,11 @@ #include "Common/CommonTypes.h" #include "Common/Swap.h" +namespace File +{ +class DirectIOFile; +} + namespace DiscIO { enum class WIARVZCompressionType : u32; @@ -194,14 +199,23 @@ std::unique_ptr CreateBlobReader(const std::string& filename); using CompressCB = std::function; -bool ConvertToGCZ(BlobReader* infile, const std::string& infile_path, - const std::string& outfile_path, u32 sub_type, int sector_size, - const CompressCB& callback); -bool ConvertToPlain(BlobReader* infile, const std::string& infile_path, - const std::string& outfile_path, const CompressCB& callback); -bool ConvertToWIAOrRVZ(BlobReader* infile, const std::string& infile_path, - const std::string& outfile_path, bool rvz, - WIARVZCompressionType compression_type, int compression_level, - int chunk_size, const CompressCB& callback); +enum class ConversionResultCode; + +using ConversionFunction = std::function infile, File::DirectIOFile& outfile, const CompressCB& callback)>; + +// Handles common functionality like opening the output file and displaying error messages. +bool ConvertBlob(const ConversionFunction& conversion_function, std::unique_ptr infile, + std::string_view infile_path, const std::string& outfile_path, + const CompressCB& callback); + +ConversionResultCode ConvertToGCZ(u32 sub_type, int block_size, std::unique_ptr infile, + File::DirectIOFile& outfile, const CompressCB& callback); +ConversionResultCode ConvertToPlain(std::unique_ptr infile, File::DirectIOFile& outfile, + const CompressCB& callback); +ConversionResultCode ConvertToWIAOrRVZ(bool rvz, WIARVZCompressionType compression_type, + int compression_level, int chunk_size, + std::unique_ptr infile, + File::DirectIOFile& outfile, const CompressCB& callback); } // namespace DiscIO diff --git a/Source/Core/DiscIO/CompressedBlob.cpp b/Source/Core/DiscIO/CompressedBlob.cpp index 7f86ee23881..26832a0f202 100644 --- a/Source/Core/DiscIO/CompressedBlob.cpp +++ b/Source/Core/DiscIO/CompressedBlob.cpp @@ -25,13 +25,12 @@ #include "Common/Logging/Log.h" #include "Common/MsgHandler.h" #include "DiscIO/Blob.h" -#include "DiscIO/DiscScrubber.h" #include "DiscIO/MultithreadedCompressor.h" #include "DiscIO/Volume.h" namespace DiscIO { -bool IsGCZBlob(File::DirectIOFile& file); +static bool IsGCZBlob(File::DirectIOFile& file); CompressedBlobReader::CompressedBlobReader(File::DirectIOFile file, const std::string& filename) : m_file(std::move(file)), m_file_name(filename) @@ -270,23 +269,11 @@ static ConversionResultCode Output(OutputParameters parameters, File::DirectIOFi return ConversionResultCode::Success; } -bool ConvertToGCZ(BlobReader* infile, const std::string& infile_path, - const std::string& outfile_path, u32 sub_type, int block_size, - const CompressCB& callback) +ConversionResultCode ConvertToGCZ(u32 sub_type, int block_size, std::unique_ptr infile, + File::DirectIOFile& outfile, const CompressCB& callback) { ASSERT(infile->GetDataSizeType() == DataSizeType::Accurate); - File::DirectIOFile outfile(outfile_path, File::AccessMode::Write); - if (!outfile.IsOpen()) - { - PanicAlertFmtT( - "Failed to open the output file \"{0}\".\n" - "Check that you have permissions to write the target folder and that the media can " - "be written.", - outfile_path); - return false; - } - callback(Common::GetStringT("Files opened, ready to compress."), 0); CompressedBlobHeader header; @@ -352,35 +339,21 @@ bool ConvertToGCZ(BlobReader* infile, const std::string& infile_path, header.compressed_data_size = position; const ConversionResultCode result = compressor.GetStatus(); - - if (result != ConversionResultCode::Success) - { - // Remove the incomplete output file. - outfile.Close(); - File::Delete(outfile_path); - } - else + if (result == ConversionResultCode::Success) { // Okay, go back and fill in headers outfile.Seek(0, File::SeekOrigin::Begin); - outfile.Write(Common::AsU8Span(header)); - outfile.Write(Common::AsU8Span(offsets)); - outfile.Write(Common::AsU8Span(hashes)); + const auto headers_written = outfile.Write(Common::AsU8Span(header)) && + outfile.Write(Common::AsU8Span(offsets)) && + outfile.Write(Common::AsU8Span(hashes)); + + if (!headers_written) + return ConversionResultCode::WriteFailed; callback(Common::GetStringT("Done compressing disc image."), 1.0f); } - if (result == ConversionResultCode::ReadFailed) - PanicAlertFmtT("Failed to read from the input file \"{0}\".", infile_path); - - if (result == ConversionResultCode::WriteFailed) - { - PanicAlertFmtT("Failed to write the output file \"{0}\".\n" - "Check that you have enough space available on the target drive.", - outfile_path); - } - - return result == ConversionResultCode::Success; + return result; } bool IsGCZBlob(File::DirectIOFile& file) diff --git a/Source/Core/DiscIO/FileBlob.cpp b/Source/Core/DiscIO/FileBlob.cpp index 3a30e2a2552..a98da819f75 100644 --- a/Source/Core/DiscIO/FileBlob.cpp +++ b/Source/Core/DiscIO/FileBlob.cpp @@ -5,19 +5,21 @@ #include #include -#include #include -#include +#include "Common/Align.h" #include "Common/Assert.h" +#include "Common/Buffer.h" #include "Common/FileUtil.h" #include "Common/MsgHandler.h" +#include "DiscIO/MultithreadedCompressor.h" + namespace DiscIO { -PlainFileReader::PlainFileReader(File::DirectIOFile file) : m_file(std::move(file)) +PlainFileReader::PlainFileReader(File::DirectIOFile file) + : m_file(std::move(file)), m_size{m_file.GetSize()} { - m_size = m_file.GetSize(); } std::unique_ptr PlainFileReader::Create(File::DirectIOFile file) @@ -38,77 +40,53 @@ bool PlainFileReader::Read(u64 offset, u64 nbytes, u8* out_ptr) return m_file.OffsetRead(offset, out_ptr, nbytes); } -bool ConvertToPlain(BlobReader* infile, const std::string& infile_path, - const std::string& outfile_path, const CompressCB& callback) +ConversionResultCode ConvertToPlain(std::unique_ptr infile, File::DirectIOFile& outfile, + const CompressCB& callback) { ASSERT(infile->GetDataSizeType() == DataSizeType::Accurate); - File::DirectIOFile outfile(outfile_path, File::AccessMode::Write); - if (!outfile.IsOpen()) - { - PanicAlertFmtT( - "Failed to open the output file \"{0}\".\n" - "Check that you have permissions to write the target folder and that the media can " - "be written.", - outfile_path); - return false; - } - - constexpr size_t DESIRED_BUFFER_SIZE = 0x80000; + constexpr size_t MINIMUM_BUFFER_SIZE = 0x80000; u64 buffer_size = infile->GetBlockSize(); if (buffer_size == 0) { - buffer_size = DESIRED_BUFFER_SIZE; + buffer_size = MINIMUM_BUFFER_SIZE; } else { - while (buffer_size < DESIRED_BUFFER_SIZE) + while (buffer_size < MINIMUM_BUFFER_SIZE) buffer_size *= 2; } - std::vector buffer(buffer_size); - const u64 num_buffers = (infile->GetDataSize() + buffer_size - 1) / buffer_size; - int progress_monitor = std::max(1, num_buffers / 100); - bool success = true; + const u64 total_size = infile->GetDataSize(); - for (u64 i = 0; i < num_buffers; i++) + // Avoid fragmentation. + if (!Resize(outfile, total_size)) + return ConversionResultCode::WriteFailed; + + Common::UniqueBuffer buffer(buffer_size); + + const u64 progress_interval = Common::AlignUp(std::max(total_size / 100, 1), buffer_size); + for (u64 read_pos = 0; read_pos != total_size;) { - if (i % progress_monitor == 0) + if (read_pos % progress_interval == 0) { const bool was_cancelled = - !callback(Common::GetStringT("Unpacking"), (float)i / (float)num_buffers); + !callback(Common::GetStringT("Unpacking"), float(read_pos) / float(total_size)); if (was_cancelled) - { - success = false; - break; - } - } - const u64 inpos = i * buffer_size; - const u64 sz = std::min(buffer_size, infile->GetDataSize() - inpos); - if (!infile->Read(inpos, sz, buffer.data())) - { - PanicAlertFmtT("Failed to read from the input file \"{0}\".", infile_path); - success = false; - break; - } - if (!outfile.Write(buffer.data(), sz)) - { - PanicAlertFmtT("Failed to write the output file \"{0}\".\n" - "Check that you have enough space available on the target drive.", - outfile_path); - success = false; - break; + return ConversionResultCode::Canceled; } + + const u64 read_size = std::min(buffer_size, total_size - read_pos); + if (!infile->Read(read_pos, read_size, buffer.data())) + return ConversionResultCode::ReadFailed; + + if (!outfile.Write(buffer.data(), read_size)) + return ConversionResultCode::WriteFailed; + + read_pos += read_size; } - if (!success) - { - // Remove the incomplete output file. - outfile.Close(); - File::Delete(outfile_path); - } - - return success; + return ConversionResultCode::Success; } } // namespace DiscIO diff --git a/Source/Core/DiscIO/FileBlob.h b/Source/Core/DiscIO/FileBlob.h index 3e3a495343a..c4c85f16f15 100644 --- a/Source/Core/DiscIO/FileBlob.h +++ b/Source/Core/DiscIO/FileBlob.h @@ -32,10 +32,10 @@ public: bool Read(u64 offset, u64 nbytes, u8* out_ptr) override; private: - PlainFileReader(File::DirectIOFile file); + explicit PlainFileReader(File::DirectIOFile file); File::DirectIOFile m_file; - u64 m_size; + const u64 m_size; }; } // namespace DiscIO diff --git a/Source/Core/DiscIO/WIABlob.cpp b/Source/Core/DiscIO/WIABlob.cpp index 743eade500f..3bcb3606add 100644 --- a/Source/Core/DiscIO/WIABlob.cpp +++ b/Source/Core/DiscIO/WIABlob.cpp @@ -2038,47 +2038,17 @@ WIARVZFileReader::Convert(BlobReader* infile, const VolumeDisc* infile_volu return ConversionResultCode::Success; } -bool ConvertToWIAOrRVZ(BlobReader* infile, const std::string& infile_path, - const std::string& outfile_path, bool rvz, - WIARVZCompressionType compression_type, int compression_level, - int chunk_size, const CompressCB& callback) +ConversionResultCode ConvertToWIAOrRVZ(bool rvz, WIARVZCompressionType compression_type, + int compression_level, int chunk_size, + std::unique_ptr infile, + File::DirectIOFile& outfile, const CompressCB& callback) { - File::DirectIOFile outfile(outfile_path, File::AccessMode::Write); - if (!outfile.IsOpen()) - { - PanicAlertFmtT( - "Failed to open the output file \"{0}\".\n" - "Check that you have permissions to write the target folder and that the media can " - "be written.", - outfile_path); - return false; - } + auto* const infile_ptr = infile.get(); + std::unique_ptr infile_volume = CreateDisc(std::move(infile)); - std::unique_ptr infile_volume = CreateDisc(infile_path); - - const auto convert = rvz ? RVZFileReader::Convert : WIAFileReader::Convert; - const ConversionResultCode result = - convert(infile, infile_volume.get(), &outfile, compression_type, compression_level, - chunk_size, callback); - - if (result == ConversionResultCode::ReadFailed) - PanicAlertFmtT("Failed to read from the input file \"{0}\".", infile_path); - - if (result == ConversionResultCode::WriteFailed) - { - PanicAlertFmtT("Failed to write the output file \"{0}\".\n" - "Check that you have enough space available on the target drive.", - outfile_path); - } - - if (result != ConversionResultCode::Success) - { - // Remove the incomplete output file - outfile.Close(); - File::Delete(outfile_path); - } - - return result == ConversionResultCode::Success; + return std::invoke(rvz ? RVZFileReader::Convert : WIAFileReader::Convert, infile_ptr, + infile_volume.get(), &outfile, compression_type, compression_level, chunk_size, + callback); } template class WIARVZFileReader; diff --git a/Source/Core/DolphinQt/ConvertDialog.cpp b/Source/Core/DolphinQt/ConvertDialog.cpp index 28ca0f9be79..cac264fb406 100644 --- a/Source/Core/DolphinQt/ConvertDialog.cpp +++ b/Source/Core/DolphinQt/ConvertDialog.cpp @@ -460,63 +460,57 @@ void ConvertDialog::Convert() tr("Failed to open the input file \"%1\".").arg(QString::fromStdString(original_path))); return; } - else + + const auto callback = [&progress_dialog](const std::string& text [[maybe_unused]], + float percent) { + progress_dialog.SetValue(int(percent * 100)); + return !progress_dialog.WasCanceled(); + }; + + DiscIO::ConversionFunction conversion_function; + switch (format) { - const auto callback = [&progress_dialog](const std::string& text, float percent) { - progress_dialog.SetValue(percent * 100); - return !progress_dialog.WasCanceled(); - }; + case DiscIO::BlobType::PLAIN: + conversion_function = DiscIO::ConvertToPlain; - std::future success; + break; - switch (format) - { - case DiscIO::BlobType::PLAIN: - success = std::async(std::launch::async, [&] { - const bool good = DiscIO::ConvertToPlain(blob_reader.get(), original_path, - dst_path.toStdString(), callback); - progress_dialog.Reset(); - return good; - }); - break; + case DiscIO::BlobType::GCZ: + conversion_function = + std::bind_front(DiscIO::ConvertToGCZ, + file->GetPlatform() == DiscIO::Platform::WiiDisc ? 1 : 0, block_size); - case DiscIO::BlobType::GCZ: - success = std::async(std::launch::async, [&] { - const bool good = DiscIO::ConvertToGCZ( - blob_reader.get(), original_path, dst_path.toStdString(), - file->GetPlatform() == DiscIO::Platform::WiiDisc ? 1 : 0, block_size, callback); - progress_dialog.Reset(); - return good; - }); - break; + break; - case DiscIO::BlobType::WIA: - case DiscIO::BlobType::RVZ: - success = std::async(std::launch::async, [&] { - const bool good = - DiscIO::ConvertToWIAOrRVZ(blob_reader.get(), original_path, dst_path.toStdString(), - format == DiscIO::BlobType::RVZ, compression, - compression_level, block_size, callback); - progress_dialog.Reset(); - return good; - }); - break; + case DiscIO::BlobType::WIA: + case DiscIO::BlobType::RVZ: + conversion_function = + std::bind_front(DiscIO::ConvertToWIAOrRVZ, format == DiscIO::BlobType::RVZ, compression, + compression_level, block_size); - default: - ASSERT(false); - break; - } + break; - progress_dialog.GetRaw()->exec(); - if (!success.get()) - { - ModalMessageBox::critical(this, tr("Error"), - tr("Dolphin failed to complete the requested action.")); - return; - } - - success_count++; + default: + break; } + + auto success = std::async(std::launch::async, [&] { + const auto good = conversion_function && + DiscIO::ConvertBlob(conversion_function, std::move(blob_reader), + original_path, dst_path.toStdString(), callback); + progress_dialog.Reset(); + return good; + }); + + progress_dialog.GetRaw()->exec(); + if (!success.get()) + { + ModalMessageBox::critical(this, tr("Error"), + tr("Dolphin failed to complete the requested action.")); + return; + } + + ++success_count; } ModalMessageBox::information(this, tr("Success"), diff --git a/Source/Core/DolphinTool/ConvertCommand.cpp b/Source/Core/DolphinTool/ConvertCommand.cpp index 210e608d2ac..cae760df5c6 100644 --- a/Source/Core/DolphinTool/ConvertCommand.cpp +++ b/Source/Core/DolphinTool/ConvertCommand.cpp @@ -4,6 +4,7 @@ #include "DolphinTool/ConvertCommand.h" #include +#include #include #include #include @@ -19,7 +20,6 @@ #include "DiscIO/DiscUtils.h" #include "DiscIO/ScrubbedBlob.h" #include "DiscIO/Volume.h" -#include "DiscIO/VolumeDisc.h" #include "DiscIO/WIABlob.h" #include "UICommon/UICommon.h" @@ -296,17 +296,12 @@ int ConvertCommand(const std::vector& args) } } - // Perform the conversion - const auto NOOP_STATUS_CALLBACK = [](const std::string& text, float percent) { return true; }; - - bool success = false; - + DiscIO::ConversionFunction conversion_function; switch (format) { case DiscIO::BlobType::PLAIN: { - success = DiscIO::ConvertToPlain(blob_reader.get(), input_file_path, output_file_path, - NOOP_STATUS_CALLBACK); + conversion_function = DiscIO::ConvertToPlain; break; } @@ -320,18 +315,16 @@ int ConvertCommand(const std::vector& args) else if (volume->GetVolumeType() == DiscIO::Platform::WiiDisc) sub_type = 1; } - success = DiscIO::ConvertToGCZ(blob_reader.get(), input_file_path, output_file_path, sub_type, - block_size_o.value(), NOOP_STATUS_CALLBACK); + conversion_function = std::bind_front(DiscIO::ConvertToGCZ, sub_type, block_size_o.value()); break; } case DiscIO::BlobType::WIA: case DiscIO::BlobType::RVZ: { - success = DiscIO::ConvertToWIAOrRVZ(blob_reader.get(), input_file_path, output_file_path, - format == DiscIO::BlobType::RVZ, compression_o.value(), - compression_level_o.value(), block_size_o.value(), - NOOP_STATUS_CALLBACK); + conversion_function = + std::bind_front(DiscIO::ConvertToWIAOrRVZ, format == DiscIO::BlobType::RVZ, + compression_o.value(), compression_level_o.value(), block_size_o.value()); break; } @@ -342,6 +335,10 @@ int ConvertCommand(const std::vector& args) } } + // Perform the conversion + const auto NOOP_STATUS_CALLBACK = [](auto&&...) { return true; }; + const bool success = DiscIO::ConvertBlob(conversion_function, std::move(blob_reader), + input_file_path, output_file_path, NOOP_STATUS_CALLBACK); if (!success) { fmt::print(std::cerr, "Error: Conversion failed\n");