Merge 0a5c68eaaa into 0fdf1cc386

Source/Android/jni/MainAndroid.cpp

@@ -4,6 +4,7 @@
 #include <condition_variable>
 #include <cstdio>
 #include <cstdlib>
+#include <functional>
 #include <memory>
 #include <mutex>
 #include <optional>
@@ -736,25 +737,23 @@ JNIEXPORT jboolean JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_ConvertD
     return static_cast<bool>(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 =
+    conversion_function = std::bind_front(
-        DiscIO::ConvertToGCZ(blob_reader.get(), in_path, out_path,
+        DiscIO::ConvertToGCZ, platform == DiscIO::Platform::WiiDisc ? 1 : 0, jBlockSize);
-                             platform == DiscIO::Platform::WiiDisc ? 1 : 0, jBlockSize, callback);
     break;
 
   case DiscIO::BlobType::WIA:
   case DiscIO::BlobType::RVZ:
-    success = DiscIO::ConvertToWIAOrRVZ(blob_reader.get(), in_path, out_path,
+    conversion_function =
-                                        format == DiscIO::BlobType::RVZ, compression,
+        std::bind_front(DiscIO::ConvertToWIAOrRVZ, format == DiscIO::BlobType::RVZ, compression,
-                                        jCompressionLevel, jBlockSize, callback);
+                        jCompressionLevel, jBlockSize);
     break;
 
   default:
@@ -762,6 +761,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<jboolean>(success);
 }
 

Source/Core/Common/DirectIOFile.cpp

@@ -68,7 +68,8 @@ 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)
+auto DirectIOFile::Open(const std::string& path, AccessMode access_mode, OpenMode open_mode)
+    -> OpenResult
 {
   ASSERT(!IsOpen());
 
@@ -77,7 +78,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 +103,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())
+  if (!IsOpen() && GetLastError() == ERROR_FILE_EXISTS)
-    WARN_LOG_FMT(COMMON, "CreateFile: {}", Common::GetLastErrorString());
+    return OpenError::AlreadyExists;
 
 #else
 #if defined(ANDROID)
@@ -128,14 +129,14 @@ bool DirectIOFile::Open(const std::string& path, AccessMode access_mode, OpenMod
     if (open_mode == OpenMode::Existing)
     {
       if (access_mode != AccessMode::Read && !*file_exists)
-        return false;
+        return OpenError::Other;
     }
     else
     {
       if (open_mode == OpenMode::Truncate)
         open_mode_str += 't';
       else if (open_mode == OpenMode::Create && *file_exists)
-        return false;
+        return OpenError::AlreadyExists;
 
       // Modes other than `Existing` may create a file, but "r" won't do that automatically.
       if (access_mode == AccessMode::Read && !*file_exists)
@@ -144,7 +145,7 @@ bool DirectIOFile::Open(const std::string& path, AccessMode access_mode, OpenMod
 
     m_fd = OpenAndroidContent(path, open_mode_str);
 
-    return IsOpen();
+    return IsOpen() ? OpenResult{OpenSuccess{}} : OpenError::Other;
   }
 #endif
   int flags = O_RDWR;
@@ -161,10 +162,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()

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,15 @@ enum class OpenMode
   Create,
 };
 
+enum class OpenError
+{
+  AlreadyExists,  // May be returned when using `OpenMode::Create`.
+  InvalidArguments,
+  Other,
+};
+
+enum class OpenSuccess;
+
 // 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 +67,9 @@ public:
   explicit DirectIOFile(const std::string& path, AccessMode access_mode,
                         OpenMode open_mode = OpenMode::Default);
 
-  bool Open(const std::string& path, AccessMode access_mode,
+  using OpenResult = Common::Result<OpenError, OpenSuccess>;
-            OpenMode open_mode = OpenMode::Default);
+  OpenResult Open(const std::string& path, AccessMode access_mode,
+                  OpenMode open_mode = OpenMode::Default);
 
   bool Close();
 

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__)
@@ -1047,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

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);
@@ -270,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

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<BlobReader> CreateBlobReader(const std::string& filename)
   }
 }
 
+bool ConvertBlob(const ConversionFunction& conversion_function, std::unique_ptr<BlobReader> 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

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<BlobReader> CreateBlobReader(const std::string& filename);
 
 using CompressCB = std::function<bool(const std::string& text, float percent)>;
 
-bool ConvertToGCZ(BlobReader* infile, const std::string& infile_path,
+enum class ConversionResultCode;
-                  const std::string& outfile_path, u32 sub_type, int sector_size,
+
-                  const CompressCB& callback);
+using ConversionFunction = std::function<ConversionResultCode(
-bool ConvertToPlain(BlobReader* infile, const std::string& infile_path,
+    std::unique_ptr<BlobReader> infile, File::DirectIOFile& outfile, const CompressCB& callback)>;
-                    const std::string& outfile_path, const CompressCB& callback);
+
-bool ConvertToWIAOrRVZ(BlobReader* infile, const std::string& infile_path,
+// Handles common functionality like opening the output file and displaying error messages.
-                       const std::string& outfile_path, bool rvz,
+bool ConvertBlob(const ConversionFunction& conversion_function, std::unique_ptr<BlobReader> infile,
-                       WIARVZCompressionType compression_type, int compression_level,
+                 std::string_view infile_path, const std::string& outfile_path,
-                       int chunk_size, const CompressCB& callback);
+                 const CompressCB& callback);
+
+ConversionResultCode ConvertToGCZ(u32 sub_type, int block_size, std::unique_ptr<BlobReader> infile,
+                                  File::DirectIOFile& outfile, const CompressCB& callback);
+ConversionResultCode ConvertToPlain(std::unique_ptr<BlobReader> infile, File::DirectIOFile& outfile,
+                                    const CompressCB& callback);
+ConversionResultCode ConvertToWIAOrRVZ(bool rvz, WIARVZCompressionType compression_type,
+                                       int compression_level, int chunk_size,
+                                       std::unique_ptr<BlobReader> infile,
+                                       File::DirectIOFile& outfile, const CompressCB& callback);
 
 }  // namespace DiscIO

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,
+ConversionResultCode ConvertToGCZ(u32 sub_type, int block_size, std::unique_ptr<BlobReader> infile,
-                  const std::string& outfile_path, u32 sub_type, int block_size,
+                                  File::DirectIOFile& outfile, const CompressCB& callback)
-                  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)
-  if (result != ConversionResultCode::Success)
-  {
-    // Remove the incomplete output file.
-    outfile.Close();
-    File::Delete(outfile_path);
-  }
-  else
   {
     // Okay, go back and fill in headers
     outfile.Seek(0, File::SeekOrigin::Begin);
-    outfile.Write(Common::AsU8Span(header));
+    const auto headers_written = outfile.Write(Common::AsU8Span(header)) &&
-    outfile.Write(Common::AsU8Span(offsets));
+                                 outfile.Write(Common::AsU8Span(offsets)) &&
-    outfile.Write(Common::AsU8Span(hashes));
+                                 outfile.Write(Common::AsU8Span(hashes));
+
+    if (!headers_written)
+      return ConversionResultCode::WriteFailed;
 
     callback(Common::GetStringT("Done compressing disc image."), 1.0f);
   }
 
-  if (result == ConversionResultCode::ReadFailed)
+  return result;
-    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;
 }
 
 bool IsGCZBlob(File::DirectIOFile& file)

Source/Core/DiscIO/FileBlob.cpp

@@ -5,19 +5,21 @@
 
 #include <algorithm>
 #include <memory>
-#include <string>
 #include <utility>
-#include <vector>
 
+#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> 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,
+ConversionResultCode ConvertToPlain(std::unique_ptr<BlobReader> infile, File::DirectIOFile& outfile,
-                    const std::string& outfile_path, const CompressCB& callback)
+                                    const CompressCB& callback)
 {
   ASSERT(infile->GetDataSizeType() == DataSizeType::Accurate);
 
-  File::DirectIOFile outfile(outfile_path, File::AccessMode::Write);
+  constexpr size_t MINIMUM_BUFFER_SIZE = 0x80000;
-  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;
   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<u8> buffer(buffer_size);
+  const u64 total_size = infile->GetDataSize();
-  const u64 num_buffers = (infile->GetDataSize() + buffer_size - 1) / buffer_size;
-  int progress_monitor = std::max<int>(1, num_buffers / 100);
-  bool success = true;
 
-  for (u64 i = 0; i < num_buffers; i++)
+  // Avoid fragmentation.
+  if (!Resize(outfile, total_size))
+    return ConversionResultCode::WriteFailed;
+
+  Common::UniqueBuffer<u8> buffer(buffer_size);
+
+  const u64 progress_interval = Common::AlignUp(std::max<u64>(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)
-      {
+        return ConversionResultCode::Canceled;
-        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;
     }
+
+    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)
+  return ConversionResultCode::Success;
-  {
-    // Remove the incomplete output file.
-    outfile.Close();
-    File::Delete(outfile_path);
-  }
-
-  return success;
 }
 
 }  // namespace DiscIO

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

Source/Core/DiscIO/WIABlob.cpp

@@ -2038,47 +2038,17 @@ WIARVZFileReader<RVZ>::Convert(BlobReader* infile, const VolumeDisc* infile_volu
   return ConversionResultCode::Success;
 }
 
-bool ConvertToWIAOrRVZ(BlobReader* infile, const std::string& infile_path,
+ConversionResultCode ConvertToWIAOrRVZ(bool rvz, WIARVZCompressionType compression_type,
-                       const std::string& outfile_path, bool rvz,
+                                       int compression_level, int chunk_size,
-                       WIARVZCompressionType compression_type, int compression_level,
+                                       std::unique_ptr<BlobReader> infile,
-                       int chunk_size, const CompressCB& callback)
+                                       File::DirectIOFile& outfile, const CompressCB& callback)
 {
-  File::DirectIOFile outfile(outfile_path, File::AccessMode::Write);
+  auto* const infile_ptr = infile.get();
-  if (!outfile.IsOpen())
+  std::unique_ptr<VolumeDisc> infile_volume = CreateDisc(std::move(infile));
-  {
-    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;
-  }
 
-  std::unique_ptr<VolumeDisc> infile_volume = CreateDisc(infile_path);
+  return std::invoke(rvz ? RVZFileReader::Convert : WIAFileReader::Convert, infile_ptr,
-
+                     infile_volume.get(), &outfile, compression_type, compression_level, chunk_size,
-  const auto convert = rvz ? RVZFileReader::Convert : WIAFileReader::Convert;
+                     callback);
-  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;
 }
 
 template class WIARVZFileReader<false>;

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) {
+    case DiscIO::BlobType::PLAIN:
-        progress_dialog.SetValue(percent * 100);
+      conversion_function = DiscIO::ConvertToPlain;
-        return !progress_dialog.WasCanceled();
-      };
 
-      std::future<bool> success;
+      break;
 
-      switch (format)
+    case DiscIO::BlobType::GCZ:
-      {
+      conversion_function =
-      case DiscIO::BlobType::PLAIN:
+          std::bind_front(DiscIO::ConvertToGCZ,
-        success = std::async(std::launch::async, [&] {
+                          file->GetPlatform() == DiscIO::Platform::WiiDisc ? 1 : 0, block_size);
-          const bool good = DiscIO::ConvertToPlain(blob_reader.get(), original_path,
-                                                   dst_path.toStdString(), callback);
-          progress_dialog.Reset();
-          return good;
-        });
-        break;
 
-      case DiscIO::BlobType::GCZ:
+      break;
-        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;
 
-      case DiscIO::BlobType::WIA:
+    case DiscIO::BlobType::WIA:
-      case DiscIO::BlobType::RVZ:
+    case DiscIO::BlobType::RVZ:
-        success = std::async(std::launch::async, [&] {
+      conversion_function =
-          const bool good =
+          std::bind_front(DiscIO::ConvertToWIAOrRVZ, format == DiscIO::BlobType::RVZ, compression,
-              DiscIO::ConvertToWIAOrRVZ(blob_reader.get(), original_path, dst_path.toStdString(),
+                          compression_level, block_size);
-                                        format == DiscIO::BlobType::RVZ, compression,
-                                        compression_level, block_size, callback);
-          progress_dialog.Reset();
-          return good;
-        });
-        break;
 
-      default:
+      break;
-        ASSERT(false);
-        break;
-      }
 
-      progress_dialog.GetRaw()->exec();
+    default:
-      if (!success.get())
+      break;
-      {
-        ModalMessageBox::critical(this, tr("Error"),
-                                  tr("Dolphin failed to complete the requested action."));
-        return;
-      }
-
-      success_count++;
     }
+
+    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"),

Source/Core/DolphinTool/ConvertCommand.cpp

@@ -4,6 +4,7 @@
 #include "DolphinTool/ConvertCommand.h"
 
 #include <cstdlib>
+#include <functional>
 #include <iostream>
 #include <limits>
 #include <optional>
@@ -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<std::string>& args)
     }
   }
 
-  // Perform the conversion
+  DiscIO::ConversionFunction conversion_function;
-  const auto NOOP_STATUS_CALLBACK = [](const std::string& text, float percent) { return true; };
-
-  bool success = false;
-
   switch (format)
   {
   case DiscIO::BlobType::PLAIN:
   {
-    success = DiscIO::ConvertToPlain(blob_reader.get(), input_file_path, output_file_path,
+    conversion_function = DiscIO::ConvertToPlain;
-                                     NOOP_STATUS_CALLBACK);
     break;
   }
 
@@ -320,18 +315,16 @@ int ConvertCommand(const std::vector<std::string>& 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,
+    conversion_function = std::bind_front(DiscIO::ConvertToGCZ, sub_type, block_size_o.value());
-                                   block_size_o.value(), NOOP_STATUS_CALLBACK);
     break;
   }
 
   case DiscIO::BlobType::WIA:
   case DiscIO::BlobType::RVZ:
   {
-    success = DiscIO::ConvertToWIAOrRVZ(blob_reader.get(), input_file_path, output_file_path,
+    conversion_function =
-                                        format == DiscIO::BlobType::RVZ, compression_o.value(),
+        std::bind_front(DiscIO::ConvertToWIAOrRVZ, format == DiscIO::BlobType::RVZ,
-                                        compression_level_o.value(), block_size_o.value(),
+                        compression_o.value(), compression_level_o.value(), block_size_o.value());
-                                        NOOP_STATUS_CALLBACK);
     break;
   }
 
@@ -342,6 +335,10 @@ int ConvertCommand(const std::vector<std::string>& 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");

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, 3> u8_test_data = {42, 7, 99};
@@ -194,7 +205,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());