Slight fixups with FATStorage (#1934)

* Reload the SD card for `CartSD` and all subclasses

* Make `ROMManager::LoadDLDISDCard` delegate to `GetDLDISDCardArgs`

* Add a method overload for `CartSD::SetSDCard`

* Initialize new SD card images with the correct size

* Sync the old card to the host (if applicable) when move-assigning a new one

* Only sync the old card to the host if it's not read-only

* Remove static state in `FATStorage`

- Replace `FF_ReadStorage` and `FF_WriteStorage` with lambda functions
- Keep open and use the single `File` handle throughout the `FATStorage`'s life

src/FATStorage.cpp

@@ -48,8 +48,6 @@ FATStorage::FATStorage(FATStorageArgs&& args) noexcept :
     SourceDir(std::move(args.SourceDir))
 {
     Load(FilePath, FileSize, SourceDir);
-
-    File = Platform::OpenLocalFile(FilePath, FileMode::ReadWriteExisting);
 }
 
 FATStorage::FATStorage(FATStorage&& other) noexcept
@@ -71,7 +69,10 @@ FATStorage& FATStorage::operator=(FATStorage&& other) noexcept
     if (this != &other)
     {
         if (File)
+        { // Sync this file's contents to the host (if applicable) before closing it
+            if (!ReadOnly) Save();
             CloseFile(File);
+        }
 
         FilePath = std::move(other.FilePath);
         IndexPath = std::move(other.IndexPath);
@@ -83,6 +84,7 @@ FATStorage& FATStorage::operator=(FATStorage&& other) noexcept
         FileIndex = std::move(other.FileIndex);
 
         other.File = nullptr;
+        other.SourceDir = std::nullopt;
     }
 
     return *this;
@@ -99,11 +101,8 @@ FATStorage::~FATStorage()
 bool FATStorage::InjectFile(const std::string& path, u8* data, u32 len)
 {
     if (!File) return false;
-    if (FF_File) return false;
 
-    FF_File = File;
+    ff_disk_open(FF_ReadStorage(), FF_WriteStorage(), (LBA_t)(FileSize>>9));
-    FF_FileSize = FileSize;
-    ff_disk_open(FF_ReadStorage, FF_WriteStorage, (LBA_t)(FileSize>>9));
 
     FRESULT res;
     FATFS fs;
@@ -112,7 +111,6 @@ bool FATStorage::InjectFile(const std::string& path, u8* data, u32 len)
     if (res != FR_OK)
     {
         ff_disk_close();
-        FF_File = nullptr;
         return false;
     }
 
@@ -124,7 +122,6 @@ bool FATStorage::InjectFile(const std::string& path, u8* data, u32 len)
     {
         f_unmount("0:");
         ff_disk_close();
-        FF_File = nullptr;
         return false;
     }
 
@@ -134,18 +131,14 @@ bool FATStorage::InjectFile(const std::string& path, u8* data, u32 len)
 
     f_unmount("0:");
     ff_disk_close();
-    FF_File = nullptr;
     return nwrite==len;
 }
 
 u32 FATStorage::ReadFile(const std::string& path, u32 start, u32 len, u8* data)
 {
     if (!File) return false;
-    if (FF_File) return false;
 
-    FF_File = File;
+    ff_disk_open(FF_ReadStorage(), FF_WriteStorage(), (LBA_t)(FileSize>>9));
-    FF_FileSize = FileSize;
-    ff_disk_open(FF_ReadStorage, FF_WriteStorage, (LBA_t)(FileSize>>9));
 
     FRESULT res;
     FATFS fs;
@@ -154,7 +147,6 @@ u32 FATStorage::ReadFile(const std::string& path, u32 start, u32 len, u8* data)
     if (res != FR_OK)
     {
         ff_disk_close();
-        FF_File = nullptr;
         return false;
     }
 
@@ -166,7 +158,6 @@ u32 FATStorage::ReadFile(const std::string& path, u32 start, u32 len, u8* data)
     {
         f_unmount("0:");
         ff_disk_close();
-        FF_File = nullptr;
         return false;
     }
 
@@ -177,7 +168,6 @@ u32 FATStorage::ReadFile(const std::string& path, u32 start, u32 len, u8* data)
 
     f_unmount("0:");
     ff_disk_close();
-    FF_File = nullptr;
     return nread;
 }
 
@@ -197,18 +187,18 @@ u64 FATStorage::GetSectorCount() const
     return FileSize / 0x200;
 }
 
-
+ff_disk_read_cb FATStorage::FF_ReadStorage() const noexcept
-FileHandle* FATStorage::FF_File;
-u64 FATStorage::FF_FileSize;
-
-UINT FATStorage::FF_ReadStorage(BYTE* buf, LBA_t sector, UINT num)
 {
-    return ReadSectorsInternal(FF_File, FF_FileSize, sector, num, buf);
+    return [this](BYTE* buf, LBA_t sector, UINT num) {
+        return ReadSectorsInternal(File, FileSize, sector, num, buf);
+    };
 }
 
-UINT FATStorage::FF_WriteStorage(const BYTE* buf, LBA_t sector, UINT num)
+ff_disk_write_cb FATStorage::FF_WriteStorage() const noexcept
 {
-    return WriteSectorsInternal(FF_File, FF_FileSize, sector, num, buf);
+    return [this](const BYTE* buf, LBA_t sector, UINT num) {
+        return WriteSectorsInternal(File, FileSize, sector, num, buf);
+    };
 }
 
 
@@ -1030,8 +1020,8 @@ bool FATStorage::Load(const std::string& filename, u64 size, const std::optional
     //   with a minimum 128MB extra, otherwise size is defaulted to 512MB
 
     bool isnew = !Platform::LocalFileExists(filename);
-    FF_File = Platform::OpenLocalFile(filename, static_cast<FileMode>(FileMode::ReadWrite | FileMode::Preserve));
+    File = Platform::OpenLocalFile(filename, static_cast<FileMode>(FileMode::ReadWrite | FileMode::Preserve));
-    if (!FF_File)
+    if (!File)
         return false;
 
     IndexPath = FilePath + ".idx";
@@ -1047,7 +1037,7 @@ bool FATStorage::Load(const std::string& filename, u64 size, const std::optional
 
         if (FileSize == 0)
         {
-            FileSize = FileLength(FF_File);
+            FileSize = FileLength(File);
         }
     }
 
@@ -1061,8 +1051,7 @@ bool FATStorage::Load(const std::string& filename, u64 size, const std::optional
     }
     else
     {
-        FF_FileSize = FileSize;
+        ff_disk_open(FF_ReadStorage(), FF_WriteStorage(), (LBA_t)(FileSize>>9));
-        ff_disk_open(FF_ReadStorage, FF_WriteStorage, (LBA_t)(FF_FileSize>>9));
 
         res = f_mount(&fs, "0:", 1);
         if (res != FR_OK)
@@ -1098,9 +1087,8 @@ bool FATStorage::Load(const std::string& filename, u64 size, const std::optional
                 FileSize = 0x20000000ULL; // 512MB
         }
 
-        FF_FileSize = FileSize;
         ff_disk_close();
-        ff_disk_open(FF_ReadStorage, FF_WriteStorage, (LBA_t)(FF_FileSize>>9));
+        ff_disk_open(FF_ReadStorage(), FF_WriteStorage(), (LBA_t)(FileSize>>9));
 
         DirIndex.clear();
         FileIndex.clear();
@@ -1137,8 +1125,6 @@ bool FATStorage::Load(const std::string& filename, u64 size, const std::optional
     f_unmount("0:");
 
     ff_disk_close();
-    CloseFile(FF_File);
-    FF_File = nullptr;
 
     return true;
 }
@@ -1150,14 +1136,7 @@ bool FATStorage::Save()
         return true; // Not an error.
     }
 
-    FF_File = Platform::OpenLocalFile(FilePath, FileMode::ReadWriteExisting);
+    ff_disk_open(FF_ReadStorage(), FF_WriteStorage(), (LBA_t)(FileSize>>9));
-    if (!FF_File)
-    {
-        return false;
-    }
-
-    FF_FileSize = FileSize;
-    ff_disk_open(FF_ReadStorage, FF_WriteStorage, (LBA_t)(FileSize>>9));
 
     FRESULT res;
     FATFS fs;
@@ -1166,8 +1145,6 @@ bool FATStorage::Save()
     if (res != FR_OK)
     {
         ff_disk_close();
-        CloseFile(FF_File);
-        FF_File = nullptr;
         return false;
     }
 
@@ -1178,8 +1155,6 @@ bool FATStorage::Save()
     f_unmount("0:");
 
     ff_disk_close();
-    CloseFile(FF_File);
-    FF_File = nullptr;
 
     return true;
 }

src/FATStorage.h

@@ -28,6 +28,7 @@
 #include "Platform.h"
 #include "types.h"
 #include "fatfs/ff.h"
+#include "FATIO.h"
 
 namespace melonDS
 {
@@ -39,6 +40,8 @@ namespace melonDS
 struct FATStorageArgs
 {
     std::string Filename;
+
+    /// Size of the desired SD card in bytes, or 0 for auto-detect.
     u64 Size;
     bool ReadOnly;
     std::optional<std::string> SourceDir;
@@ -74,10 +77,8 @@ private:
     Platform::FileHandle* File;
     u64 FileSize;
 
-    static Platform::FileHandle* FF_File;
+    [[nodiscard]] ff_disk_read_cb FF_ReadStorage() const noexcept;
-    static u64 FF_FileSize;
+    [[nodiscard]] ff_disk_write_cb FF_WriteStorage() const noexcept;
-    static UINT FF_ReadStorage(BYTE* buf, LBA_t sector, UINT num);
-    static UINT FF_WriteStorage(const BYTE* buf, LBA_t sector, UINT num);
 
     static u32 ReadSectorsInternal(Platform::FileHandle* file, u64 filelen, u32 start, u32 num, u8* data);
     static u32 WriteSectorsInternal(Platform::FileHandle* file, u64 filelen, u32 start, u32 num, const u8* data);

src/NDSCart.h

@@ -260,6 +260,22 @@ public:
         // it just leaves behind an optional with a moved-from value
     }
 
+    void SetSDCard(std::optional<FATStorageArgs>&& args) noexcept
+    {
+        // Close the open SD card (if any) so that its contents are flushed to disk.
+        // Also, if args refers to the same image file that SD is currently using,
+        // this will ensure that we don't have two open read-write handles
+        // to the same file.
+        SD = std::nullopt;
+
+        if (args)
+            SD = FATStorage(std::move(*args));
+
+        args = std::nullopt;
+        // moving from an optional doesn't set it to nullopt,
+        // it just leaves behind an optional with a moved-from value
+    }
+
 protected:
     void ApplyDLDIPatchAt(u8* binary, u32 dldioffset, const u8* patch, u32 patchlen, bool readonly) const;
     void ApplyDLDIPatch(const u8* patch, u32 patchlen, bool readonly);

src/frontend/qt_sdl/EmuThread.cpp

@@ -196,12 +196,11 @@ bool EmuThread::UpdateConsole(UpdateConsoleNDSArgs&& ndsargs, UpdateConsoleGBAAr
         ndsargs = {};
     }
 
-    if (nextndscart && nextndscart->Type() == NDSCart::Homebrew)
+    if (auto* cartsd = dynamic_cast<NDSCart::CartSD*>(nextndscart.get()))
     {
-        // Load DLDISDCard will return nullopt if the SD card is disabled;
+        // LoadDLDISDCard will return nullopt if the SD card is disabled;
         // SetSDCard will accept nullopt, which means no SD card
-        auto& homebrew = static_cast<NDSCart::CartHomebrew&>(*nextndscart);
+        cartsd->SetSDCard(ROMManager::GetDLDISDCardArgs());
-        homebrew.SetSDCard(ROMManager::LoadDLDISDCard());
     }
 
     std::unique_ptr<GBACart::CartCommon> nextgbacart;

src/frontend/qt_sdl/ROMManager.cpp

@@ -759,7 +759,12 @@ std::optional<DSi_NAND::NANDImage> LoadNAND(const std::array<u8, DSiBIOSSize>& a
     return nandImage;
 }
 
-constexpr u64 imgsizes[] = {0, 256, 512, 1024, 2048, 4096};
+constexpr u64 MB(u64 i)
+{
+    return i * 1024 * 1024;
+}
+
+constexpr u64 imgsizes[] = {0, MB(256), MB(512), MB(1024), MB(2048), MB(4096)};
 std::optional<FATStorageArgs> GetDSiSDCardArgs() noexcept
 {
     if (!Config::DSiSDEnable)
@@ -804,12 +809,7 @@ std::optional<FATStorage> LoadDLDISDCard() noexcept
     if (!Config::DLDIEnable)
         return std::nullopt;
 
-    return FATStorage(
+    return FATStorage(*GetDLDISDCardArgs());
-        Config::DLDISDPath,
-        imgsizes[Config::DLDISize],
-        Config::DLDIReadOnly,
-        Config::DLDIFolderSync ? std::make_optional(Config::DLDIFolderPath) : std::nullopt
-    );
 }
 
 void EnableCheats(NDS& nds, bool enable)