diff --git a/src/activities/reader/EpubReaderUtils.h b/src/activities/reader/EpubReaderUtils.h index 7dc0fb63..e9ac05b8 100644 --- a/src/activities/reader/EpubReaderUtils.h +++ b/src/activities/reader/EpubReaderUtils.h @@ -1,23 +1,19 @@ #pragma once #include -#include #include +#include "ProgressFile.h" + namespace EpubReaderUtils { // Persists reader progress for an EPUB to its cache directory. Returns true on success. -inline bool saveProgress(Epub& epub, int spineIndex, int pageNumber, int pageCount) { +inline bool saveProgress(const Epub& epub, int spineIndex, int pageNumber, int pageCount) { if (spineIndex < 0 || spineIndex > 0xFFFF || pageNumber < 0 || pageNumber > 0xFFFF || pageCount < 0 || pageCount > 0xFFFF) { LOG_ERR("ERS", "Progress values out of range: spine=%d page=%d count=%d", spineIndex, pageNumber, pageCount); return false; } - HalFile f; - if (!Storage.openFileForWrite("ERS", epub.getCachePath() + "/progress.bin", f)) { - LOG_ERR("ERS", "Could not open progress file for write!"); - return false; - } uint8_t data[6]; data[0] = spineIndex & 0xFF; data[1] = (spineIndex >> 8) & 0xFF; @@ -25,9 +21,7 @@ inline bool saveProgress(Epub& epub, int spineIndex, int pageNumber, int pageCou data[3] = (pageNumber >> 8) & 0xFF; data[4] = pageCount & 0xFF; data[5] = (pageCount >> 8) & 0xFF; - const size_t written = f.write(data, sizeof(data)); - if (written != sizeof(data)) { - LOG_ERR("ERS", "Short write saving progress: %u/%u bytes", (unsigned)written, (unsigned)sizeof(data)); + if (!ProgressFile::writeAtomic(epub.getCachePath(), data, sizeof(data))) { return false; } LOG_DBG("ERS", "Progress saved: spine=%d page=%d", spineIndex, pageNumber); diff --git a/src/activities/reader/ProgressFile.h b/src/activities/reader/ProgressFile.h new file mode 100644 index 00000000..5238a157 --- /dev/null +++ b/src/activities/reader/ProgressFile.h @@ -0,0 +1,64 @@ +#pragma once + +#include +#include + +#include +#include +#include + +namespace ProgressFile { + +// Writes `len` bytes of reader progress to `/progress.bin` without +// ever leaving the canonical file half-written. +// +// The bytes go to a temporary `progress.bin.tmp` first; only once that is fully +// written and closed is it renamed over progress.bin. An interrupted write +// (power loss or a crash mid-SPI) therefore damages only the throwaway temp file. +// Previously a truncate-in-place write that was cut short left progress.bin with +// a broken FAT cluster chain that the firmware could neither rewrite nor clear, +// stranding the book on an old page (issue #2275). +// +// This is crash-safe, not metadata-atomic: on FAT the replace is remove + rename, +// two separate directory operations, so a crash between them can leave neither +// file -- which simply reads as "no saved progress" on next launch, never a +// corrupt or unclearable file. The point is that progress.bin is never torn. +// +// Note: this prevents corruption on a healthy card going forward. It cannot +// repair an already-corrupted progress.bin -- removing the stale file may itself +// fail at the FAT level, in which case recovery still requires fsck on a host. +// +// Returns true only if the new progress.bin is fully in place. +inline bool writeAtomic(const std::string& cachePath, const uint8_t* data, size_t len) { + const std::string finalPath = cachePath + "/progress.bin"; + const std::string tmpPath = cachePath + "/progress.bin.tmp"; + + { + HalFile f; + if (!Storage.openFileForWrite("PRG", tmpPath, f)) { + LOG_ERR("PRG", "Could not open temp progress file for write: %s", tmpPath.c_str()); + return false; + } + const size_t written = f.write(data, len); + if (written != len) { + LOG_ERR("PRG", "Short write saving progress to %s: %u/%u bytes", tmpPath.c_str(), (unsigned)written, + (unsigned)len); + return false; + } + f.flush(); + // f (the temp file) is closed at scope exit (DESTRUCTOR_CLOSES_FILE=1) before + // the rename below -- SdFat must not rename a path that still has an open FsFile. + } + + // SdFat's rename does not overwrite an existing destination, so drop the old + // canonical file first. The brief window where neither file exists reads as + // "no saved progress" on next launch -- never a corrupt, unclearable file. + Storage.remove(finalPath.c_str()); + if (!Storage.rename(tmpPath.c_str(), finalPath.c_str())) { + LOG_ERR("PRG", "Failed to rename temp progress into place: %s", finalPath.c_str()); + return false; + } + return true; +} + +} // namespace ProgressFile diff --git a/src/activities/reader/TxtReaderActivity.cpp b/src/activities/reader/TxtReaderActivity.cpp index 0f3a7992..0464c614 100644 --- a/src/activities/reader/TxtReaderActivity.cpp +++ b/src/activities/reader/TxtReaderActivity.cpp @@ -11,6 +11,7 @@ #include "CrossPointSettings.h" #include "CrossPointState.h" #include "MappedInputManager.h" +#include "ProgressFile.h" #include "ReaderUtils.h" #include "RecentBooksStore.h" #include "components/UITheme.h" @@ -421,14 +422,13 @@ void TxtReaderActivity::renderStatusBar() const { } void TxtReaderActivity::saveProgress() const { - HalFile f; - if (Storage.openFileForWrite("TRS", txt->getCachePath() + "/progress.bin", f)) { - uint8_t data[4]; - data[0] = currentPage & 0xFF; - data[1] = (currentPage >> 8) & 0xFF; - data[2] = 0; - data[3] = 0; - f.write(data, 4); + uint8_t data[4]; + data[0] = currentPage & 0xFF; + data[1] = (currentPage >> 8) & 0xFF; + data[2] = 0; + data[3] = 0; + if (!ProgressFile::writeAtomic(txt->getCachePath(), data, sizeof(data))) { + LOG_ERR("TRS", "Failed to save progress: page %d", currentPage); } } diff --git a/src/activities/reader/XtcReaderActivity.cpp b/src/activities/reader/XtcReaderActivity.cpp index c78aeea1..c2306aa8 100644 --- a/src/activities/reader/XtcReaderActivity.cpp +++ b/src/activities/reader/XtcReaderActivity.cpp @@ -17,6 +17,7 @@ #include "CrossPointSettings.h" #include "CrossPointState.h" #include "MappedInputManager.h" +#include "ProgressFile.h" #include "ReaderUtils.h" #include "RecentBooksStore.h" #include "XtcReaderChapterSelectionActivity.h" @@ -375,15 +376,13 @@ void XtcReaderActivity::renderPage() { } void XtcReaderActivity::saveProgress() const { - HalFile f; - if (Storage.openFileForWrite("XTR", xtc->getCachePath() + "/progress.bin", f)) { - uint8_t data[4]; - data[0] = currentPage & 0xFF; - data[1] = (currentPage >> 8) & 0xFF; - data[2] = (currentPage >> 16) & 0xFF; - data[3] = (currentPage >> 24) & 0xFF; - f.write(data, 4); - f.close(); + uint8_t data[4]; + data[0] = currentPage & 0xFF; + data[1] = (currentPage >> 8) & 0xFF; + data[2] = (currentPage >> 16) & 0xFF; + data[3] = (currentPage >> 24) & 0xFF; + if (!ProgressFile::writeAtomic(xtc->getCachePath(), data, sizeof(data))) { + LOG_ERR("XTR", "Failed to save progress: page %lu", currentPage); } }