Refactor the core's handling of firmware and BIOS images to rely less on the file system (#1826)

* Introduce firmware-related structs

* Fix some indents

* Move the generated firmware identifier to a constant

* Document the WifiAccessPoint constructors

* Add some constants

* Remove a stray comment

* Implement Firmware::UserData

* Add Firmware::Mask

* Document Firmware::Buffer

* Add a Firmware constructor that uses a FileHandle

* Set the default username in UserData

* Update the UserData checksum

* Forgot to include Platform.h

* Remove some redundant assignments in the default Firmware constructor

* const-ify CRC16

* Replace the plain Firmware buffer with a Firmware object

- Remove some functions that were reimplemented in the Firmware constructors

* Fix some crashes due to undefined behavior

* Fix the user data initialization

- Initialize both user data objects to default
- Set both user data objects to the same touch screen calibration

* Follow the DS logic in determining which user data section is current

* Remove an unneeded include

* Remove another unneeded include

* Initialize FirmwareMask in Firmware::Firmware

* Use the DEFAULT_SSID constant

* Add SPI_Firmware::InstallFirmware and SPI_Firmware::RemoveFirmware

* Move a logging call to after the file is written

* Add a SaveManager for the firmware

* Touch up the SPI_Firmware::Firmware declaration

* Move all firmware loading and customization to the frontend

* Call Platform::WriteFirmware when it's time to write the firmware back to disk

* Fix some missing stuff

* Remove the Get* functions from SPI_Firmware in favor of GetFirmware()

* Implement SPI_Firmware::DeInit in terms of RemoveFirmware

* Add Firmware::UpdateChecksums

* Fix an incorrect length

* Update all checksums in the firmware after setting touch screen calibration data

* Use the Firmware object's Position methods

* Remove register fields from the Firmware object

* Install the firmware before seeing if direct boot is necessary

* Install the firmware before calling NDS::Reset in LoadROM

* Slight cleanup in ROMManager

* Fix the default access point name

* Shorten the various getters in Firmware

* Add qualifiers for certain uses of firmware types

- GCC can get picky if -fpermissive isn't defined

* Add an InstallFirmware overload that takes a unique_ptr

* Log when firmware is added or removed

* Don't clear the firmware in SPI_Firmware::Init

- The original code didn't, it just set the pointer to nullptr

* Fix a typo

* Write back the full firmware if it's not generated

* Move the FreeBIOS to an external file

* Load wfcsettings.bin into the correct part of the generated firmware blob

* Load BIOS files in the frontend, not in the core

* Fix logging the firmware ID

* Add some utility functions

* Mark Firmware's constructors as explicit

* Remove obsolete ConfigEntry values

* Include <locale> explicitly in ROMManager

* Fix up some includes

* Add Firmware::IsBootable()

* Add a newline to a log entry

- Whoops

* Log the number of bytes written out in SaveManager

* Mark FirmwareHeader's constructor as explicit

* Clean up GenerateDefaultFirmware and LoadFirmwareFromFile

- Now they return a pair instead of two by-ref values

* Refactor SaveManager a little bit

- Manage its buffers as unique_ptrs to mitigate leaks
- Reallocate the internal buffer if SetPath is asked to reload the file (and the new length is different)

* Remove some stray parens

* Fix some firmware-related bugs I introduced

- Firmware settings are now properly saved to disk (beforehand I misunderstood when the firmware blob was written)
- Firmware is no longer overwritten by contents of wfcsettings.bin

* Slight cleanup

6 files changed, 408 insertions, 26 deletions

diff --git a/src/frontend/qt_sdl/Platform.cpp b/src/frontend/qt_sdl/Platform.cpp
index 5263377..0574d5d 100644
--- a/src/frontend/qt_sdl/Platform.cpp
+++ b/src/frontend/qt_sdl/Platform.cpp
@@ -39,6 +39,7 @@
 #include "LAN_PCap.h"
 #include "LocalMP.h"
 #include "OSD.h"
+#include "SPI_Firmware.h"
 
 #ifdef __WIN32__
 #define fseek _fseeki64
@@ -249,13 +250,6 @@ std::string GetConfigString(ConfigEntry entry)
 {
     switch (entry)
     {
-    case BIOS9Path: return Config::BIOS9Path;
-    case BIOS7Path: return Config::BIOS7Path;
-    case FirmwarePath: return Config::FirmwarePath;
-
-    case DSi_BIOS9Path: return Config::DSiBIOS9Path;
-    case DSi_BIOS7Path: return Config::DSiBIOS7Path;
-    case DSi_FirmwarePath: return Config::DSiFirmwarePath;
     case DSi_NANDPath: return Config::DSiNANDPath;
 
     case DLDI_ImagePath: return Config::DLDISDPath;
@@ -584,7 +578,36 @@ void WriteGBASave(const u8* savedata, u32 savelen, u32 writeoffset, u32 writelen
         ROMManager::GBASave->RequestFlush(savedata, savelen, writeoffset, writelen);
 }
 
+void WriteFirmware(const SPI_Firmware::Firmware& firmware, u32 writeoffset, u32 writelen)
+{
+    if (!ROMManager::FirmwareSave)
+        return;
+
+    if (firmware.Header().Identifier != SPI_Firmware::GENERATED_FIRMWARE_IDENTIFIER)
+    { // If this is not the default built-in firmware...
+        // ...then write the whole thing back.
+        ROMManager::FirmwareSave->RequestFlush(firmware.Buffer(), firmware.Length(), writeoffset, writelen);
+    }
+    else
+    {
+        u32 eapstart = firmware.ExtendedAccessPointOffset();
+        u32 eapend = eapstart + sizeof(firmware.ExtendedAccessPoints());
 
+        u32 apstart = firmware.WifiAccessPointOffset();
+        u32 apend = apstart + sizeof(firmware.AccessPoints());
+
+        // assert that the extended access points come just before the regular ones
+        assert(eapend == apstart);
+
+        if (eapstart <= writeoffset && writeoffset < apend)
+        { // If we're writing to the access points...
+            const u8* buffer = firmware.ExtendedAccessPointPosition();
+            u32 length = sizeof(firmware.ExtendedAccessPoints()) + sizeof(firmware.AccessPoints());
+            ROMManager::FirmwareSave->RequestFlush(buffer, length, writeoffset - eapstart, writelen);
+        }
+    }
+
+}
 
 bool MP_Init()
 {
diff --git a/src/frontend/qt_sdl/ROMManager.cpp b/src/frontend/qt_sdl/ROMManager.cpp
index a2a5fca..03723b5 100644
--- a/src/frontend/qt_sdl/ROMManager.cpp
+++ b/src/frontend/qt_sdl/ROMManager.cpp
@@ -16,9 +16,14 @@
     with melonDS. If not, see http://www.gnu.org/licenses/.
 */
 
+#include <assert.h>
 #include <stdio.h>
 #include <string.h>
 
+#include <codecvt>
+#include <locale>
+#include <memory>
+#include <tuple>
 #include <string>
 #include <utility>
 #include <fstream>
@@ -35,7 +40,13 @@
 #include "DSi.h"
 #include "SPI.h"
 #include "DSi_I2C.h"
+#include "FreeBIOS.h"
 
+using std::make_unique;
+using std::pair;
+using std::string;
+using std::tie;
+using std::unique_ptr;
 using namespace Platform;
 
 namespace ROMManager
@@ -53,6 +64,7 @@ std::string BaseGBAAssetName = "";
 
 SaveManager* NDSSave = nullptr;
 SaveManager* GBASave = nullptr;
+std::unique_ptr<SaveManager> FirmwareSave = nullptr;
 
 std::unique_ptr<Savestate> BackupState = nullptr;
 bool SavestateLoaded = false;
@@ -462,6 +474,94 @@ void LoadCheats()
     AREngine::SetCodeFile(CheatsOn ? CheatFile : nullptr);
 }
 
+void LoadBIOSFiles()
+{
+    if (Config::ExternalBIOSEnable)
+    {
+        if (FileHandle* f = Platform::OpenLocalFile(Config::BIOS9Path, FileMode::Read))
+        {
+            FileRewind(f);
+            FileRead(NDS::ARM9BIOS, sizeof(NDS::ARM9BIOS), 1, f);
+
+            Log(LogLevel::Info, "ARM9 BIOS loaded from %s\n", Config::BIOS9Path.c_str());
+            Platform::CloseFile(f);
+        }
+        else
+        {
+            Log(LogLevel::Warn, "ARM9 BIOS not found\n");
+
+            for (int i = 0; i < 16; i++)
+                ((u32*)NDS::ARM9BIOS)[i] = 0xE7FFDEFF;
+        }
+
+        if (FileHandle* f = Platform::OpenLocalFile(Config::BIOS7Path, FileMode::Read))
+        {
+            FileRead(NDS::ARM7BIOS, sizeof(NDS::ARM7BIOS), 1, f);
+
+            Log(LogLevel::Info, "ARM7 BIOS loaded from\n", Config::BIOS7Path.c_str());
+            Platform::CloseFile(f);
+        }
+        else
+        {
+            Log(LogLevel::Warn, "ARM7 BIOS not found\n");
+
+            for (int i = 0; i < 16; i++)
+                ((u32*)NDS::ARM7BIOS)[i] = 0xE7FFDEFF;
+        }
+    }
+    else
+    {
+        Log(LogLevel::Info, "Using built-in ARM7 and ARM9 BIOSes\n");
+        memcpy(NDS::ARM9BIOS, bios_arm9_bin, sizeof(bios_arm9_bin));
+        memcpy(NDS::ARM7BIOS, bios_arm7_bin, sizeof(bios_arm7_bin));
+    }
+
+    if (Config::ConsoleType == 1)
+    {
+        if (FileHandle* f = Platform::OpenLocalFile(Config::DSiBIOS9Path, FileMode::Read))
+        {
+            FileRead(DSi::ARM9iBIOS, sizeof(DSi::ARM9iBIOS), 1, f);
+
+            Log(LogLevel::Info, "ARM9i BIOS loaded from %s\n", Config::DSiBIOS9Path.c_str());
+            Platform::CloseFile(f);
+        }
+        else
+        {
+            Log(LogLevel::Warn, "ARM9i BIOS not found\n");
+
+            for (int i = 0; i < 16; i++)
+                ((u32*)DSi::ARM9iBIOS)[i] = 0xE7FFDEFF;
+        }
+
+        if (FileHandle* f = Platform::OpenLocalFile(Config::DSiBIOS7Path, FileMode::Read))
+        {
+        // TODO: check if the first 32 bytes are crapoed
+            FileRead(DSi::ARM7iBIOS, sizeof(DSi::ARM7iBIOS), 1, f);
+
+            Log(LogLevel::Info, "ARM7i BIOS loaded from %s\n", Config::DSiBIOS7Path.c_str());
+            CloseFile(f);
+        }
+        else
+        {
+            Log(LogLevel::Warn, "ARM7i BIOS not found\n");
+
+            for (int i = 0; i < 16; i++)
+                ((u32*)DSi::ARM7iBIOS)[i] = 0xE7FFDEFF;
+        }
+
+        if (!Config::DSiFullBIOSBoot)
+        {
+            // herp
+            *(u32*)&DSi::ARM9iBIOS[0] = 0xEAFFFFFE;
+            *(u32*)&DSi::ARM7iBIOS[0] = 0xEAFFFFFE;
+
+            // TODO!!!!
+            // hax the upper 32K out of the goddamn DSi
+            // done that :)  -pcy
+        }
+    }
+}
+
 void EnableCheats(bool enable)
 {
     CheatsOn = enable;
@@ -492,6 +592,7 @@ void Reset()
 {
     NDS::SetConsoleType(Config::ConsoleType);
     if (Config::ConsoleType == 1) EjectGBACart();
+    LoadBIOSFiles();
     NDS::Reset();
     SetBatteryLevels();
 
@@ -513,6 +614,28 @@ void Reset()
             GBASave->SetPath(newsave, false);
     }
 
+    if (FirmwareSave)
+    {
+        std::string oldsave = FirmwareSave->GetPath();
+        string newsave;
+        if (Config::ExternalBIOSEnable)
+        {
+            if (Config::ConsoleType == 1)
+                newsave = Config::DSiFirmwarePath + Platform::InstanceFileSuffix();
+            else
+                newsave = Config::FirmwarePath + Platform::InstanceFileSuffix();
+        }
+        else
+        {
+            newsave = Config::WifiSettingsPath + Platform::InstanceFileSuffix();
+        }
+
+        if (oldsave != newsave)
+        { // If the player toggled the ConsoleType or ExternalBIOSEnable...
+            FirmwareSave->SetPath(newsave, true);
+        }
+    }
+
     if (!BaseROMName.empty())
     {
         if (Config::DirectBoot || NDS::NeedsDirectBoot())
@@ -527,6 +650,11 @@ bool LoadBIOS()
 {
     NDS::SetConsoleType(Config::ConsoleType);
 
+    LoadBIOSFiles();
+
+    if (!InstallFirmware())
+        return false;
+
     if (NDS::NeedsDirectBoot())
         return false;
 
@@ -638,6 +766,222 @@ void ClearBackupState()
     }
 }
 
+// We want both the firmware object and the path that was used to load it,
+// since we'll need to give it to the save manager later
+pair<unique_ptr<SPI_Firmware::Firmware>, string> LoadFirmwareFromFile()
+{
+    string loadedpath;
+    unique_ptr<SPI_Firmware::Firmware> firmware = nullptr;
+    string firmwarepath = Config::ConsoleType == 0 ? Config::FirmwarePath : Config::DSiFirmwarePath;
+
+    Log(LogLevel::Debug, "SPI firmware: loading from file %s\n", firmwarepath.c_str());
+
+    string firmwareinstancepath = firmwarepath + Platform::InstanceFileSuffix();
+
+    loadedpath = firmwareinstancepath;
+    FileHandle* f = Platform::OpenLocalFile(firmwareinstancepath, FileMode::Read);
+    if (!f)
+    {
+        loadedpath = firmwarepath;
+        f = Platform::OpenLocalFile(firmwarepath, FileMode::Read);
+    }
+
+    if (f)
+    {
+        firmware = make_unique<SPI_Firmware::Firmware>(f);
+        if (!firmware->Buffer())
+        {
+            Log(LogLevel::Warn, "Couldn't read firmware file!\n");
+            firmware = nullptr;
+            loadedpath = "";
+        }
+
+        CloseFile(f);
+    }
+
+    return std::make_pair(std::move(firmware), loadedpath);
+}
+
+pair<unique_ptr<SPI_Firmware::Firmware>, string> GenerateDefaultFirmware()
+{
+    using namespace SPI_Firmware;
+    // Construct the default firmware...
+    string settingspath;
+    std::unique_ptr<Firmware> firmware = std::make_unique<Firmware>(Config::ConsoleType);
+    assert(firmware->Buffer() != nullptr);
+
+    // Try to open the instanced Wi-fi settings, falling back to the regular Wi-fi settings if they don't exist.
+    // We don't need to save the whole firmware, just the part that may actually change.
+    std::string wfcsettingspath = Platform::GetConfigString(ConfigEntry::WifiSettingsPath);
+    settingspath = wfcsettingspath + Platform::InstanceFileSuffix();
+    FileHandle* f = Platform::OpenLocalFile(settingspath, FileMode::Read);
+    if (!f)
+    {
+        settingspath = wfcsettingspath;
+        f = Platform::OpenLocalFile(settingspath, FileMode::Read);
+    }
+
+    // If using generated firmware, we keep the wi-fi settings on the host disk separately.
+    // Wi-fi access point data includes Nintendo WFC settings,
+    // and if we didn't keep them then the player would have to reset them in each session.
+    if (f)
+    { // If we have Wi-fi settings to load...
+        constexpr unsigned TOTAL_WFC_SETTINGS_SIZE = 3 * (sizeof(WifiAccessPoint) + sizeof(ExtendedWifiAccessPoint));
+
+        // The access point and extended access point segments might
+        // be in different locations depending on the firmware revision,
+        // but our generated firmware always keeps them next to each other.
+        // (Extended access points first, then regular ones.)
+
+        if (!FileRead(firmware->ExtendedAccessPointPosition(), TOTAL_WFC_SETTINGS_SIZE, 1, f))
+        { // If we couldn't read the Wi-fi settings from this file...
+            Platform::Log(Platform::LogLevel::Warn, "Failed to read Wi-fi settings from \"%s\"; using defaults instead\n", wfcsettingspath.c_str());
+
+            firmware->AccessPoints() = {
+                WifiAccessPoint(Config::ConsoleType),
+                WifiAccessPoint(),
+                WifiAccessPoint(),
+            };
+
+            firmware->ExtendedAccessPoints() = {
+                ExtendedWifiAccessPoint(),
+                ExtendedWifiAccessPoint(),
+                ExtendedWifiAccessPoint(),
+            };
+        }
+
+        firmware->UpdateChecksums();
+
+        CloseFile(f);
+    }
+
+    // If we don't have Wi-fi settings to load,
+    // then the defaults will have already been populated by the constructor.
+    return std::make_pair(std::move(firmware), std::move(wfcsettingspath));
+}
+
+void LoadUserSettingsFromConfig(SPI_Firmware::Firmware& firmware)
+{
+    using namespace SPI_Firmware;
+    UserData& currentData = firmware.EffectiveUserData();
+
+    // setting up username
+    std::string orig_username = Platform::GetConfigString(Platform::Firm_Username);
+    if (!orig_username.empty())
+    { // If the frontend defines a username, take it. If not, leave the existing one.
+        std::u16string username = std::wstring_convert<std::codecvt_utf8_utf16<char16_t>, char16_t>{}.from_bytes(orig_username);
+        size_t usernameLength = std::min(username.length(), (size_t) 10);
+        currentData.NameLength = usernameLength;
+        memcpy(currentData.Nickname, username.data(), usernameLength * sizeof(char16_t));
+    }
+
+    auto language = static_cast<Language>(Platform::GetConfigInt(Platform::Firm_Language));
+    if (language != Language::Reserved)
+    { // If the frontend specifies a language (rather than using the existing value)...
+        currentData.Settings &= ~Language::Reserved; // ..clear the existing language...
+        currentData.Settings |= language; // ...and set the new one.
+    }
+
+    // setting up color
+    u8 favoritecolor = Platform::GetConfigInt(Platform::Firm_Color);
+    if (favoritecolor != 0xFF)
+    {
+        currentData.FavoriteColor = favoritecolor;
+    }
+
+    u8 birthmonth = Platform::GetConfigInt(Platform::Firm_BirthdayMonth);
+    if (birthmonth != 0)
+    { // If the frontend specifies a birth month (rather than using the existing value)...
+        currentData.BirthdayMonth = birthmonth;
+    }
+
+    u8 birthday = Platform::GetConfigInt(Platform::Firm_BirthdayDay);
+    if (birthday != 0)
+    { // If the frontend specifies a birthday (rather than using the existing value)...
+        currentData.BirthdayDay = birthday;
+    }
+
+    // setup message
+    std::string orig_message = Platform::GetConfigString(Platform::Firm_Message);
+    if (!orig_message.empty())
+    {
+        std::u16string message = std::wstring_convert<std::codecvt_utf8_utf16<char16_t>, char16_t>{}.from_bytes(orig_message);
+        size_t messageLength = std::min(message.length(), (size_t) 26);
+        currentData.MessageLength = messageLength;
+        memcpy(currentData.Message, message.data(), messageLength * sizeof(char16_t));
+    }
+
+    MacAddress mac;
+    bool rep = false;
+    auto& header = firmware.Header();
+
+    memcpy(&mac, header.MacAddress.data(), sizeof(MacAddress));
+
+
+    MacAddress configuredMac;
+    rep = Platform::GetConfigArray(Platform::Firm_MAC, &configuredMac);
+    rep &= (configuredMac != MacAddress());
+
+    if (rep)
+    {
+        mac = configuredMac;
+    }
+
+    int inst = Platform::InstanceID();
+    if (inst > 0)
+    {
+        rep = true;
+        mac[3] += inst;
+        mac[4] += inst*0x44;
+        mac[5] += inst*0x10;
+    }
+
+    if (rep)
+    {
+        mac[0] &= 0xFC; // ensure the MAC isn't a broadcast MAC
+        header.MacAddress = mac;
+        header.UpdateChecksum();
+    }
+
+    firmware.UpdateChecksums();
+}
+
+bool InstallFirmware()
+{
+    using namespace SPI_Firmware;
+    FirmwareSave.reset();
+    unique_ptr<Firmware> firmware;
+    string firmwarepath;
+    bool generated = false;
+
+    if (Config::ExternalBIOSEnable)
+    { // If we want to try loading a firmware dump...
+
+        tie(firmware, firmwarepath) = LoadFirmwareFromFile();
+        if (!firmware)
+        { // Try to load the configured firmware dump. If that fails...
+            Log(LogLevel::Warn, "Firmware not found! Generating default firmware.\n");
+        }
+    }
+
+    if (!firmware)
+    { // If we haven't yet loaded firmware (either because the load failed or we want to use the default...)
+        tie(firmware, firmwarepath) = GenerateDefaultFirmware();
+    }
+
+    if (!firmware)
+        return false;
+
+    if (Config::FirmwareOverrideSettings)
+    {
+        LoadUserSettingsFromConfig(*firmware);
+    }
+
+    FirmwareSave = std::make_unique<SaveManager>(firmwarepath);
+
+    return InstallFirmware(std::move(firmware));
+}
+
 bool LoadROM(QStringList filepath, bool reset)
 {
     if (filepath.empty()) return false;
@@ -733,10 +1077,16 @@ bool LoadROM(QStringList filepath, bool reset)
     BaseROMName = romname;
     BaseAssetName = romname.substr(0, romname.rfind('.'));
 
+    if (!InstallFirmware())
+    {
+        return false;
+    }
+
     if (reset)
     {
         NDS::SetConsoleType(Config::ConsoleType);
         NDS::EjectCart();
+        LoadBIOSFiles();
         NDS::Reset();
         SetBatteryLevels();
     }
diff --git a/src/frontend/qt_sdl/ROMManager.h b/src/frontend/qt_sdl/ROMManager.h
index 8e199db..5faef1a 100644
--- a/src/frontend/qt_sdl/ROMManager.h
+++ b/src/frontend/qt_sdl/ROMManager.h
@@ -24,6 +24,7 @@
 #include "AREngine.h"
 
 #include <string>
+#include <memory>
 #include <vector>
 
 namespace ROMManager
@@ -31,12 +32,14 @@ namespace ROMManager
 
 extern SaveManager* NDSSave;
 extern SaveManager* GBASave;
+extern std::unique_ptr<SaveManager> FirmwareSave;
 
 QString VerifySetup();
 void Reset();
 bool LoadBIOS();
 void ClearBackupState();
 
+bool InstallFirmware();
 bool LoadROM(QStringList filepath, bool reset);
 void EjectCart();
 bool CartInserted();
diff --git a/src/frontend/qt_sdl/SaveManager.cpp b/src/frontend/qt_sdl/SaveManager.cpp
index 034b48f..ee55091 100644
--- a/src/frontend/qt_sdl/SaveManager.cpp
+++ b/src/frontend/qt_sdl/SaveManager.cpp
@@ -58,11 +58,11 @@ SaveManager::~SaveManager()
         FlushSecondaryBuffer();
     }
 
-    if (SecondaryBuffer) delete[] SecondaryBuffer;
+    SecondaryBuffer = nullptr;
 
     delete SecondaryBufferLock;
 
-    if (Buffer) delete[] Buffer;
+    Buffer = nullptr;
 }
 
 std::string SaveManager::GetPath()
@@ -75,11 +75,17 @@ void SaveManager::SetPath(const std::string& path, bool reload)
     Path = path;
 
     if (reload)
-    {
-        FileHandle* f = Platform::OpenFile(Path, FileMode::Read);
-        if (f)
+    { // If we should load whatever file is at the new path...
+
+        if (FileHandle* f = Platform::OpenFile(Path, FileMode::Read))
         {
-            FileRead(Buffer, 1, Length, f);
+            if (u32 length = Platform::FileLength(f); length != Length)
+            { // If the new file is a different size, we need to re-allocate the buffer.
+                Length = length;
+                Buffer = std::make_unique<u8[]>(Length);
+            }
+
+            FileRead(Buffer.get(), 1, Length, f);
             CloseFile(f);
         }
     }
@@ -91,12 +97,10 @@ void SaveManager::RequestFlush(const u8* savedata, u32 savelen, u32 writeoffset,
 {
     if (Length != savelen)
     {
-        if (Buffer) delete[] Buffer;
-
         Length = savelen;
-        Buffer = new u8[Length];
+        Buffer = std::make_unique<u8[]>(Length);
 
-        memcpy(Buffer, savedata, Length);
+        memcpy(Buffer.get(), savedata, Length);
     }
     else
     {
@@ -127,13 +131,11 @@ void SaveManager::CheckFlush()
 
     if (SecondaryBufferLength != Length)
     {
-        if (SecondaryBuffer) delete[] SecondaryBuffer;
-
         SecondaryBufferLength = Length;
-        SecondaryBuffer = new u8[SecondaryBufferLength];
+        SecondaryBuffer = std::make_unique<u8[]>(SecondaryBufferLength);
     }
 
-    memcpy(SecondaryBuffer, Buffer, Length);
+    memcpy(SecondaryBuffer.get(), Buffer.get(), Length);
 
     FlushRequested = false;
     FlushVersion++;
@@ -172,15 +174,15 @@ void SaveManager::FlushSecondaryBuffer(u8* dst, u32 dstLength)
     SecondaryBufferLock->lock();
     if (dst)
     {
-        memcpy(dst, SecondaryBuffer, SecondaryBufferLength);
+        memcpy(dst, SecondaryBuffer.get(), SecondaryBufferLength);
     }
     else
     {
         FileHandle* f = Platform::OpenFile(Path, FileMode::Write);
         if (f)
         {
-            Log(LogLevel::Info, "SaveManager: Written\n");
-            FileWrite(SecondaryBuffer, SecondaryBufferLength, 1, f);
+            FileWrite(SecondaryBuffer.get(), SecondaryBufferLength, 1, f);
+            Log(LogLevel::Info, "SaveManager: Wrote %u bytes to %s\n", SecondaryBufferLength, Path.c_str());
             CloseFile(f);
         }
     }
diff --git a/src/frontend/qt_sdl/SaveManager.h b/src/frontend/qt_sdl/SaveManager.h
index 33035ed..3d85af6 100644
--- a/src/frontend/qt_sdl/SaveManager.h
+++ b/src/frontend/qt_sdl/SaveManager.h
@@ -23,6 +23,7 @@
 #include <unistd.h>
 #include <time.h>
 #include <atomic>
+#include <memory>
 #include <QThread>
 #include <QMutex>
 
@@ -51,12 +52,12 @@ private:
 
     std::atomic_bool Running;
 
-    u8* Buffer;
+    std::unique_ptr<u8[]> Buffer;
     u32 Length;
     bool FlushRequested;
 
     QMutex* SecondaryBufferLock;
-    u8* SecondaryBuffer;
+    std::unique_ptr<u8[]> SecondaryBuffer;
     u32 SecondaryBufferLength;
 
     time_t TimeAtLastFlushRequest;
diff --git a/src/frontend/qt_sdl/main.cpp b/src/frontend/qt_sdl/main.cpp
index 7f0c0ef..a6cc248 100644
--- a/src/frontend/qt_sdl/main.cpp
+++ b/src/frontend/qt_sdl/main.cpp
@@ -513,6 +513,9 @@ void EmuThread::run()
             if (ROMManager::GBASave)
                 ROMManager::GBASave->CheckFlush();
 
+            if (ROMManager::FirmwareSave)
+                ROMManager::FirmwareSave->CheckFlush();
+
             if (!oglContext)
             {
                 FrontBufferLock.lock();