From 19c197e0205e2605a3d6dbdbfdaf43e50f11a4b2 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Sat, 28 Feb 2026 16:19:45 +0100 Subject: [PATCH 1/3] Establish LUT cache --- lib/Epub/Epub/Section.cpp | 53 +++++++++++++++++++++++++-------------- lib/Epub/Epub/Section.h | 4 ++- 2 files changed, 37 insertions(+), 20 deletions(-) diff --git a/lib/Epub/Epub/Section.cpp b/lib/Epub/Epub/Section.cpp index 814f5029..e85ece26 100644 --- a/lib/Epub/Epub/Section.cpp +++ b/lib/Epub/Epub/Section.cpp @@ -71,9 +71,8 @@ bool Section::loadSectionFile(const int fontId, const float lineCompression, con uint8_t version; serialization::readPod(file, version); if (version != SECTION_FILE_VERSION) { - file.close(); LOG_ERR("SCT", "Deserialization failed: Unknown version %u", version); - clearCache(); + clearCache(); // closes file before removal return false; } @@ -97,21 +96,30 @@ bool Section::loadSectionFile(const int fontId, const float lineCompression, con extraParagraphSpacing != fileExtraParagraphSpacing || paragraphAlignment != fileParagraphAlignment || viewportWidth != fileViewportWidth || viewportHeight != fileViewportHeight || hyphenationEnabled != fileHyphenationEnabled || embeddedStyle != fileEmbeddedStyle) { - file.close(); LOG_ERR("SCT", "Deserialization failed: Parameters do not match"); - clearCache(); + clearCache(); // closes file before removal return false; } } serialization::readPod(file, pageCount); - file.close(); - LOG_DBG("SCT", "Deserialization succeeded: %d pages", pageCount); + + // Load LUT into memory (file is now positioned at the lutOffset field) + uint32_t lutOffset; + serialization::readPod(file, lutOffset); + lut.resize(pageCount); + file.seek(lutOffset); + for (uint32_t& pos : lut) { + serialization::readPod(file, pos); + } + // File is intentionally left open; subsequent loadPageFromSectionFile() calls + // seek within this handle instead of re-opening the file each time. + LOG_DBG("SCT", "Deserialization succeeded: %d pages, LUT cached", pageCount); return true; } -// Your updated class method (assuming you are using the 'SD' object, which is a wrapper for a specific filesystem) -bool Section::clearCache() const { +bool Section::clearCache() { + file.close(); // Must be closed before removal on FAT32 if (!Storage.exists(filePath.c_str())) { LOG_DBG("SCT", "Cache does not exist, no action needed"); return true; @@ -242,23 +250,30 @@ bool Section::createSectionFile(const int fontId, const float lineCompression, c if (cssParser) { cssParser->clear(); } + + // Cache the LUT in memory and open the file for reading so that + // subsequent loadPageFromSectionFile() calls can seek directly without re-opening. + this->lut = std::move(lut); + Storage.openFileForRead("SCT", filePath, file); return true; } std::unique_ptr Section::loadPageFromSectionFile() { - if (!Storage.openFileForRead("SCT", filePath, file)) { + if (currentPage < 0 || currentPage >= static_cast(lut.size())) { + LOG_ERR("SCT", "loadPageFromSectionFile: page %d out of LUT range (%u entries)", currentPage, + static_cast(lut.size())); return nullptr; } - file.seek(HEADER_SIZE - sizeof(uint32_t)); - uint32_t lutOffset; - serialization::readPod(file, lutOffset); - file.seek(lutOffset + sizeof(uint32_t) * currentPage); - uint32_t pagePos; - serialization::readPod(file, pagePos); - file.seek(pagePos); + if (!file) { + // Safety fallback: file was closed unexpectedly; reopen + LOG_ERR("SCT", "loadPageFromSectionFile: file not open, reopening"); + if (!Storage.openFileForRead("SCT", filePath, file)) { + return nullptr; + } + } - auto page = Page::deserialize(file); - file.close(); - return page; + file.seek(lut[currentPage]); + return Page::deserialize(file); + // File is intentionally NOT closed; stays open for the next page load } diff --git a/lib/Epub/Epub/Section.h b/lib/Epub/Epub/Section.h index 42a6d993..90792191 100644 --- a/lib/Epub/Epub/Section.h +++ b/lib/Epub/Epub/Section.h @@ -1,6 +1,7 @@ #pragma once #include #include +#include #include "Epub.h" @@ -13,6 +14,7 @@ class Section { GfxRenderer& renderer; std::string filePath; FsFile file; + std::vector lut; // Cached page byte-offsets; loaded once, avoids per-page LUT seek void writeSectionFileHeader(int fontId, float lineCompression, bool extraParagraphSpacing, uint8_t paragraphAlignment, uint16_t viewportWidth, uint16_t viewportHeight, bool hyphenationEnabled, @@ -31,7 +33,7 @@ class Section { ~Section() = default; bool loadSectionFile(int fontId, float lineCompression, bool extraParagraphSpacing, uint8_t paragraphAlignment, uint16_t viewportWidth, uint16_t viewportHeight, bool hyphenationEnabled, bool embeddedStyle); - bool clearCache() const; + bool clearCache(); bool createSectionFile(int fontId, float lineCompression, bool extraParagraphSpacing, uint8_t paragraphAlignment, uint16_t viewportWidth, uint16_t viewportHeight, bool hyphenationEnabled, bool embeddedStyle, const std::function& popupFn = nullptr); From e547f70cae0fff94d6e745d2de3972b071aec1ae Mon Sep 17 00:00:00 2001 From: jpirnay Date: Sat, 28 Feb 2026 16:42:05 +0100 Subject: [PATCH 2/3] Addressing review comments --- lib/Epub/Epub/Section.cpp | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/lib/Epub/Epub/Section.cpp b/lib/Epub/Epub/Section.cpp index e85ece26..4c5d9f0c 100644 --- a/lib/Epub/Epub/Section.cpp +++ b/lib/Epub/Epub/Section.cpp @@ -104,11 +104,22 @@ bool Section::loadSectionFile(const int fontId, const float lineCompression, con serialization::readPod(file, pageCount); + // Sanity check: same upper bound used by TextBlock::deserialize for word count + if (pageCount > 10000) { + LOG_ERR("SCT", "Deserialization failed: page count %u exceeds maximum", pageCount); + clearCache(); + return false; + } + // Load LUT into memory (file is now positioned at the lutOffset field) uint32_t lutOffset; serialization::readPod(file, lutOffset); lut.resize(pageCount); - file.seek(lutOffset); + if (!file.seek(lutOffset)) { + LOG_ERR("SCT", "Deserialization failed: seek to LUT offset %u failed", lutOffset); + clearCache(); + return false; + } for (uint32_t& pos : lut) { serialization::readPod(file, pos); } @@ -120,6 +131,10 @@ bool Section::loadSectionFile(const int fontId, const float lineCompression, con bool Section::clearCache() { file.close(); // Must be closed before removal on FAT32 + lut.clear(); + pageCount = 0; + currentPage = 0; + if (!Storage.exists(filePath.c_str())) { LOG_DBG("SCT", "Cache does not exist, no action needed"); return true; @@ -254,7 +269,10 @@ bool Section::createSectionFile(const int fontId, const float lineCompression, c // Cache the LUT in memory and open the file for reading so that // subsequent loadPageFromSectionFile() calls can seek directly without re-opening. this->lut = std::move(lut); - Storage.openFileForRead("SCT", filePath, file); + if (!Storage.openFileForRead("SCT", filePath, file)) { + LOG_ERR("SCT", "Failed to open section file for reading after creation"); + return false; + } return true; } @@ -273,7 +291,10 @@ std::unique_ptr Section::loadPageFromSectionFile() { } } - file.seek(lut[currentPage]); + if (!file.seek(lut[currentPage])) { + LOG_ERR("SCT", "loadPageFromSectionFile: seek to page %d offset %u failed", currentPage, lut[currentPage]); + return nullptr; + } return Page::deserialize(file); // File is intentionally NOT closed; stays open for the next page load } From 3eadc1e923534e6cae472ba54bff92c5496751c5 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Sat, 28 Feb 2026 16:51:04 +0100 Subject: [PATCH 3/3] Nitpick comments --- lib/Epub/Epub/Section.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/Epub/Epub/Section.cpp b/lib/Epub/Epub/Section.cpp index 4c5d9f0c..82d8ed11 100644 --- a/lib/Epub/Epub/Section.cpp +++ b/lib/Epub/Epub/Section.cpp @@ -122,6 +122,11 @@ bool Section::loadSectionFile(const int fontId, const float lineCompression, con } for (uint32_t& pos : lut) { serialization::readPod(file, pos); + if (pos < HEADER_SIZE || pos >= lutOffset) { + LOG_ERR("SCT", "Deserialization failed: LUT entry %u out of range [%u, %u)", pos, HEADER_SIZE, lutOffset); + clearCache(); + return false; + } } // File is intentionally left open; subsequent loadPageFromSectionFile() calls // seek within this handle instead of re-opening the file each time. @@ -268,11 +273,11 @@ bool Section::createSectionFile(const int fontId, const float lineCompression, c // Cache the LUT in memory and open the file for reading so that // subsequent loadPageFromSectionFile() calls can seek directly without re-opening. - this->lut = std::move(lut); if (!Storage.openFileForRead("SCT", filePath, file)) { LOG_ERR("SCT", "Failed to open section file for reading after creation"); return false; } + this->lut = std::move(lut); return true; }