From 53832edafca80916290df798ccabb6353c40a656 Mon Sep 17 00:00:00 2001 From: Eidolon Date: Sun, 19 May 2024 12:03:12 -0500 Subject: [PATCH] Move old data/profiles to .bak before writing The hypothesis for this patch is that the operating system has not actually finished writing the file to disk when moving the tmp file into place. The move operation is atomic, but the write is not, even when flushed or using unbuffered IO. So we reorder these operations, make the old save .bak atomically and write the new save in place. I doubt saving this backup will actually be useful given the frequency of saves in the game, but at the very least it leaves _some_ backup in place in the event of failure. --- src/g_gamedata.cpp | 35 +++++++++++++++++++---------------- src/k_profiles.cpp | 24 +++++++++++++++++------- 2 files changed, 36 insertions(+), 23 deletions(-) diff --git a/src/g_gamedata.cpp b/src/g_gamedata.cpp index a2f60b92d..a8890ed21 100644 --- a/src/g_gamedata.cpp +++ b/src/g_gamedata.cpp @@ -287,15 +287,28 @@ void srb2::save_ng_gamedata() std::string gamedataname_s {gamedatafilename}; fs::path savepath {fmt::format("{}/{}", srb2home, gamedataname_s)}; - int random_number = rand(); - fs::path tmpsavepath {fmt::format("{}/{}_{}.tmp", srb2home, gamedataname_s, random_number)}; + fs::path baksavepath {fmt::format("{}/{}.bak", srb2home, gamedataname_s)}; json ngdata_json = ng; + + if (fs::exists(savepath)) + { + try + { + fs::rename(savepath, baksavepath); + } + catch (const fs::filesystem_error& ex) + { + CONS_Alert(CONS_ERROR, "Failed to record backup save. Not attempting to save. %s\n", ex.what()); + return; + } + } + try { - std::string tmpsavepathstring = tmpsavepath.string(); - srb2::io::FileStream file {tmpsavepathstring, srb2::io::FileStreamMode::kWrite}; + std::string savepathstring = savepath.string(); + srb2::io::FileStream file {savepathstring, srb2::io::FileStreamMode::kWrite}; // The header is necessary to validate during loading. srb2::io::write(static_cast(GD_VERSION_MAJOR), file); // major @@ -308,21 +321,11 @@ void srb2::save_ng_gamedata() } catch (const std::exception& ex) { - CONS_Alert(CONS_ERROR, "NG Gamedata save failed: %s\n", ex.what()); + CONS_Alert(CONS_ERROR, "NG Gamedata save failed. Check directory for a ringdata.dat.bak. %s\n", ex.what()); } catch (...) { - CONS_Alert(CONS_ERROR, "NG Gamedata save failed\n"); - } - - try - { - // Now that the save is written successfully, move it over the old save - fs::rename(tmpsavepath, savepath); - } - catch (const fs::filesystem_error& ex) - { - CONS_Alert(CONS_ERROR, "NG Gamedata save succeeded but did not replace old save successfully: %s\n", ex.what()); + CONS_Alert(CONS_ERROR, "NG Gamedata save failed. Check directory for a ringdata.dat.bak.\n"); } } diff --git a/src/k_profiles.cpp b/src/k_profiles.cpp index 3491d70fc..0b9425fcb 100644 --- a/src/k_profiles.cpp +++ b/src/k_profiles.cpp @@ -319,12 +319,24 @@ void PR_SaveProfiles(void) std::vector ubjson = json::to_ubjson(ng); std::string realpath = fmt::format("{}/{}", srb2home, PROFILESFILE); - int random_number = rand(); - std::string tmppath = fmt::format("{}_{}.tmp", realpath, random_number); + std::string bakpath = fmt::format("{}.bak", realpath); + + if (fs::exists(realpath)) + { + try + { + fs::rename(realpath, bakpath); + } + catch (const fs::filesystem_error& ex) + { + CONS_Alert(CONS_ERROR, "Failed to record profiles backup. Not attempting to save profiles. %s\n", ex.what()); + return; + } + } try { - io::FileStream file {tmppath, io::FileStreamMode::kWrite}; + io::FileStream file {realpath, io::FileStreamMode::kWrite}; io::write(static_cast(0x52494E47), file, io::Endian::kBE); // "RING" io::write(static_cast(0x5052464C), file, io::Endian::kBE); // "PRFL" @@ -334,16 +346,14 @@ void PR_SaveProfiles(void) io::write(static_cast(0), file); // reserved4 io::write_exact(file, tcb::as_bytes(tcb::make_span(ubjson))); file.close(); - - fs::rename(tmppath, realpath); } catch (const std::exception& ex) { - I_Error("Couldn't save profiles. Are you out of Disk space / playing in a protected folder?\n\nException: %s", ex.what()); + I_Error("Couldn't save profiles. Are you out of Disk space / playing in a protected folder? Check directory for a ringprofiles.prf.bak if the profiles file is corrupt.\n\nException: %s", ex.what()); } catch (...) { - I_Error("Couldn't save profiles. Are you out of Disk space / playing in a protected folder?"); + I_Error("Couldn't save profiles. Are you out of Disk space / playing in a protected folder? Check directory for a ringprofiles.prf.bak if the profiles file is corrupt."); } }