Merge pull request #269 from jpirnay/test

fix: Resolve pagelist oom error

lib/Epub/Epub.cpp

@@ -14,38 +14,13 @@
 
 #include "Epub/parsers/ContainerParser.h"
 #include "Epub/parsers/ContentOpfParser.h"
+#include "Epub/parsers/PageListSink.h"
 #include "Epub/parsers/PageMapParser.h"
 #include "Epub/parsers/TocNavParser.h"
 #include "Epub/parsers/TocNcxParser.h"
 
 namespace {
 
-// Serialise a list of printed-page entries (href, anchor, label) to pagelist.bin in the
-// book cache. Templated on the parser's entry type so both NCX <pageList> and EPUB 3
-// <nav epub:type="page-list"> share the same writer.
-template <typename Entry>
-void writePageListBin(const std::string& cachePath, const std::vector<Entry>& pageList) {
-  const auto pageListPath = cachePath + "/pagelist.bin";
-  if (pageList.empty()) {
-    Storage.remove(pageListPath.c_str());
-    return;
-  }
-  FsFile pageListFile;
-  if (!Storage.openFileForWrite("EBP", pageListPath, pageListFile)) {
-    LOG_ERR("EBP", "Could not write pagelist.bin");
-    return;
-  }
-  serialization::writePod(pageListFile, static_cast<uint16_t>(pageList.size()));
-  for (const auto& entry : pageList) {
-    serialization::writeString(pageListFile, entry.href);
-    serialization::writeString(pageListFile, entry.anchor);
-    serialization::writeString(pageListFile, entry.label);
-  }
-  pageListFile.flush();
-  pageListFile.close();
-  LOG_DBG("EBP", "Wrote pagelist.bin with %u entries", static_cast<unsigned>(pageList.size()));
-}
-
 enum class CoverImageFormat { Unknown, Jpeg, Png };
 
 CoverImageFormat detectCoverImageFormat(FsFile& imageFile) {
@@ -373,7 +348,10 @@ bool Epub::parseTocNcxFile() const {
   }
   const auto ncxSize = tempNcxFile.size();
 
-  TocNcxParser ncxParser(contentBasePath, ncxSize, bookMetadataCache.get());
+  // Stream <pageList> entries straight to pagelist.bin (long printed-page lists used to
+  // blow the X3 heap when accumulated in a std::vector — see PageListSink).
+  PageListSink ncxPageListSink(getCachePath());
+  TocNcxParser ncxParser(contentBasePath, ncxSize, bookMetadataCache.get(), &ncxPageListSink);
 
   if (!ncxParser.setup()) {
     LOG_ERR("EBP", "Could not setup toc ncx parser");
@@ -405,11 +383,10 @@ bool Epub::parseTocNcxFile() const {
   tempNcxFile.close();
   Storage.remove(tmpNcxPath.c_str());
 
-  // Persist the printed-page list (NCX <pageList>) to a small cache file so the
-  // section builder can stamp printed-page labels onto rendered pages without
-  // re-parsing the NCX. Format: u16 count, then per entry: writeString(href),
-  // writeString(anchor), writeString(label).
-  writePageListBin(getCachePath(), ncxParser.getPageList());
+  // Flush u16 count + close pagelist.bin (or remove it if no <pageList> entries were
+  // streamed). The section builder later reads this file to stamp printed-page labels
+  // onto rendered pages without re-parsing the NCX.
+  ncxPageListSink.finalize();
 
   LOG_DBG("EBP", "Parsed TOC items");
   return true;
@@ -439,7 +416,9 @@ bool Epub::parseTocNavFile() const {
   // Note: We can't use `contentBasePath` here as the nav file may be in a different folder to the content.opf
   // and the HTMLX nav file will have hrefs relative to itself
   const std::string navContentBasePath = tocNavItem.substr(0, tocNavItem.find_last_of('/') + 1);
-  TocNavParser navParser(navContentBasePath, navSize, bookMetadataCache.get());
+  // Stream <nav epub:type="page-list"> entries straight to pagelist.bin (see PageListSink).
+  PageListSink navPageListSink(getCachePath());
+  TocNavParser navParser(navContentBasePath, navSize, bookMetadataCache.get(), &navPageListSink);
 
   if (!navParser.setup()) {
     LOG_ERR("EBP", "Could not setup toc nav parser");
@@ -468,9 +447,8 @@ bool Epub::parseTocNavFile() const {
   tempNavFile.close();
   Storage.remove(tmpNavPath.c_str());
 
-  // Persist EPUB 3 <nav epub:type="page-list"> entries to pagelist.bin (same format
-  // as the NCX writer); the section builder consumes either source uniformly.
-  writePageListBin(getCachePath(), navParser.getPageList());
+  // Flush u16 count + close pagelist.bin (or remove it if no entries were streamed).
+  navPageListSink.finalize();
 
   LOG_DBG("EBP", "Parsed TOC nav items");
   return true;
@@ -500,7 +478,9 @@ bool Epub::parsePageMapFile() const {
 
   // page-map hrefs are relative to the page-map file itself (typically content.opf's dir).
   const std::string pageMapBasePath = pageMapItem.substr(0, pageMapItem.find_last_of('/') + 1);
-  PageMapParser pageMapParser(pageMapBasePath, pageMapSize);
+  // Stream page-map entries straight to pagelist.bin (see PageListSink).
+  PageListSink pageMapPageListSink(getCachePath());
+  PageMapParser pageMapParser(pageMapBasePath, pageMapSize, &pageMapPageListSink);
 
   if (!pageMapParser.setup()) {
     LOG_ERR("EBP", "Could not setup page-map parser");
@@ -531,7 +511,7 @@ bool Epub::parsePageMapFile() const {
   tempPageMapFile.close();
   Storage.remove(tmpPageMapPath.c_str());
 
-  writePageListBin(getCachePath(), pageMapParser.getPageList());
+  pageMapPageListSink.finalize();
   LOG_DBG("EBP", "Parsed page-map entries");
   return true;
 }

lib/Epub/Epub/parsers/PageListSink.cpp

@@ -0,0 +1,50 @@
+#include "PageListSink.h"
+
+#include <Logging.h>
+#include <Serialization.h>
+
+PageListSink::PageListSink(const std::string& cachePath) : path(cachePath + "/pagelist.bin") {
+  if (!Storage.openFileForWrite("EBP", path, file)) {
+    LOG_ERR("EBP", "PageListSink: could not open pagelist.bin for writing");
+    return;
+  }
+  // Placeholder count; patched in finalize().
+  serialization::writePod(file, static_cast<uint16_t>(0));
+}
+
+PageListSink::~PageListSink() {
+  if (!finalized) {
+    finalize();
+  }
+}
+
+void PageListSink::addEntry(const std::string& href, const std::string& anchor, const std::string& label) {
+  if (!file.isOpen() || finalized) return;
+  serialization::writeString(file, href);
+  serialization::writeString(file, anchor);
+  serialization::writeString(file, label);
+  count++;
+}
+
+void PageListSink::finalize() {
+  if (finalized) return;
+  finalized = true;
+  if (!file.isOpen()) return;
+
+  if (count == 0) {
+    file.close();
+    Storage.remove(path.c_str());
+    return;
+  }
+
+  file.flush();
+  if (!file.seek(0)) {
+    LOG_ERR("EBP", "PageListSink: could not seek to patch count");
+    file.close();
+    return;
+  }
+  serialization::writePod(file, count);
+  file.flush();
+  file.close();
+  LOG_DBG("EBP", "Wrote pagelist.bin with %u entries", static_cast<unsigned>(count));
+}

lib/Epub/Epub/parsers/PageListSink.h

@@ -0,0 +1,52 @@
+#pragma once
+
+#include <HalStorage.h>
+
+#include <string>
+
+// Streams printed-page entries (href, anchor, label) directly to pagelist.bin
+// instead of buffering them in a std::vector. Buffering blows the X3 heap on
+// books with long EPUB 3 <nav epub:type="page-list"> sections (a few hundred
+// entries × 3 std::string each is enough to throw std::bad_alloc).
+//
+// File format matches the legacy writePageListBin() in Epub.cpp so the reader
+// side (Epub::loadPrintedPageList) is unchanged: u16 count, then per entry
+// writeString(href), writeString(anchor), writeString(label).
+//
+// Lifecycle: construct (opens the file and writes a placeholder count of 0),
+// addEntry(...) for each parsed entry, then finalize() to seek back and patch
+// the real count. If no entries were added, finalize() removes the file —
+// matching the legacy writer's "remove on empty" behaviour so subsequent
+// fallback parsers (NCX after nav, page-map after both) see no stale file.
+//
+// Not copyable; not thread-safe (each book parse is single-threaded).
+class PageListSink {
+ public:
+  explicit PageListSink(const std::string& cachePath);
+  ~PageListSink();
+
+  PageListSink(const PageListSink&) = delete;
+  PageListSink& operator=(const PageListSink&) = delete;
+
+  // True if the file was opened successfully. When false, addEntry() and
+  // finalize() are no-ops; callers don't need to check before each push.
+  bool isOpen() const { return file.isOpen(); }
+
+  // Number of entries successfully streamed so far. Callers (e.g. Epub.cpp
+  // orchestrator deciding whether the NCX fallback should run) read this
+  // instead of inspecting the file.
+  uint16_t entryCount() const { return count; }
+
+  void addEntry(const std::string& href, const std::string& anchor, const std::string& label);
+
+  // Patches the placeholder count at offset 0 with the real entry count and
+  // closes the file. If no entries were added, the file is removed instead.
+  // Safe to call multiple times — subsequent calls are no-ops.
+  void finalize();
+
+ private:
+  std::string path;
+  FsFile file;
+  uint16_t count = 0;
+  bool finalized = false;
+};

lib/Epub/Epub/parsers/PageMapParser.cpp

@@ -3,6 +3,8 @@
 #include <FsHelpers.h>
 #include <Logging.h>
 
+#include "PageListSink.h"
+
 bool PageMapParser::setup() {
   parser = XML_ParserCreate(nullptr);
   if (!parser) {
@@ -88,7 +90,7 @@ void XMLCALL PageMapParser::startElement(void* userData, const XML_Char* name, c
     }
   }
 
-  if (label.empty() || rawHref.empty()) {
+  if (!self->pageListSink || label.empty() || rawHref.empty()) {
     return;
   }
 
@@ -99,5 +101,5 @@ void XMLCALL PageMapParser::startElement(void* userData, const XML_Char* name, c
     anchor = href.substr(pos + 1);
     href = href.substr(0, pos);
   }
-  self->pageList.push_back({std::move(href), std::move(anchor), std::move(label)});
+  self->pageListSink->addEntry(href, anchor, label);
 }

lib/Epub/Epub/parsers/PageMapParser.h

@@ -3,36 +3,30 @@
 #include <expat.h>
 
 #include <string>
-#include <vector>
+
+class PageListSink;
 
 // Parser for EPUB 2.01 page-map.xml. Each <page name="X" href="...#anchor"/> element
 // maps a printed page number to a spine location. Same output shape as TocNcxParser
 // and TocNavParser so all three feed the shared pagelist.bin writer.
 class PageMapParser final : public Print {
- public:
-  struct PageListEntry {
-    std::string href;
-    std::string anchor;
-    std::string label;
-  };
-
  private:
   const std::string& baseContentPath;
   size_t remainingSize;
   XML_Parser parser = nullptr;
-  std::vector<PageListEntry> pageList;
+  // Page-list entries are streamed straight to disk via this sink. Owned by
+  // the caller (Epub.cpp); may be null when no page-list output is wanted.
+  PageListSink* pageListSink;
 
   static void startElement(void* userData, const XML_Char* name, const XML_Char** atts);
 
  public:
-  explicit PageMapParser(const std::string& baseContentPath, const size_t xmlSize)
-      : baseContentPath(baseContentPath), remainingSize(xmlSize) {}
+  explicit PageMapParser(const std::string& baseContentPath, const size_t xmlSize, PageListSink* pageListSink)
+      : baseContentPath(baseContentPath), remainingSize(xmlSize), pageListSink(pageListSink) {}
   ~PageMapParser() override;
 
   bool setup();
 
   size_t write(uint8_t) override;
   size_t write(const uint8_t* buffer, size_t size) override;
-
-  const std::vector<PageListEntry>& getPageList() const { return pageList; }
 };

lib/Epub/Epub/parsers/TocNavParser.cpp

@@ -4,6 +4,7 @@
 #include <Logging.h>
 
 #include "../BookMetadataCache.h"
+#include "PageListSink.h"
 
 bool TocNavParser::setup() {
   parser = XML_ParserCreate(nullptr);
@@ -178,7 +179,7 @@ void XMLCALL TocNavParser::endElement(void* userData, const XML_Char* name) {
   // ---- Page-list nav close handlers (checked before TOC handlers because IN_PL_* states
   // sort after IN_NAV_TOC, but we want exact-state matching either way).
   if (strcmp(name, "a") == 0 && self->state == IN_PL_ANCHOR) {
-    if (!self->currentPageLabel.empty() && !self->currentPageHref.empty()) {
+    if (self->pageListSink && !self->currentPageLabel.empty() && !self->currentPageHref.empty()) {
       std::string href = FsHelpers::normalisePath(self->baseContentPath + self->currentPageHref);
       std::string anchor;
       const size_t pos = href.find('#');
@@ -186,10 +187,10 @@ void XMLCALL TocNavParser::endElement(void* userData, const XML_Char* name) {
         anchor = href.substr(pos + 1);
         href = href.substr(0, pos);
       }
-      self->pageList.push_back({std::move(href), std::move(anchor), std::move(self->currentPageLabel)});
-      self->currentPageLabel.clear();
-      self->currentPageHref.clear();
+      self->pageListSink->addEntry(href, anchor, self->currentPageLabel);
     }
+    self->currentPageLabel.clear();
+    self->currentPageHref.clear();
     self->state = IN_PL_LI;
     return;
   }

lib/Epub/Epub/parsers/TocNavParser.h

@@ -3,9 +3,9 @@
 #include <expat.h>
 
 #include <string>
-#include <vector>
 
 class BookMetadataCache;
+class PageListSink;
 
 // Parser for EPUB 3 nav.xhtml navigation documents
 // Parses HTML5 nav elements with epub:type="toc" (table of contents) and
@@ -25,22 +25,15 @@ class TocNavParser final : public Print {
     IN_PL_ANCHOR,      // Inside <a> (within page-list nav)
   };
 
- public:
-  // One printed-page entry from <nav epub:type="page-list">: file href (normalised),
-  // anchor fragment, and visible label. Matches TocNcxParser::PageListEntry in shape so
-  // both parsers can feed the same pagelist.bin writer.
-  struct PageListEntry {
-    std::string href;
-    std::string anchor;
-    std::string label;
-  };
-
  private:
   const std::string& baseContentPath;
   size_t remainingSize;
   XML_Parser parser = nullptr;
   ParserState state = START;
   BookMetadataCache* cache;
+  // Page-list entries are streamed straight to disk via this sink. Owned by
+  // the caller (Epub.cpp); may be null when no page-list output is wanted.
+  PageListSink* pageListSink;
 
   // Track nesting depth for <ol> elements to determine TOC depth
   uint8_t olDepth = 0;
@@ -53,21 +46,19 @@ class TocNavParser final : public Print {
   uint8_t plOlDepth = 0;
   std::string currentPageLabel;
   std::string currentPageHref;
-  std::vector<PageListEntry> pageList;
 
   static void startElement(void* userData, const XML_Char* name, const XML_Char** atts);
   static void characterData(void* userData, const XML_Char* s, int len);
   static void endElement(void* userData, const XML_Char* name);
 
  public:
-  explicit TocNavParser(const std::string& baseContentPath, const size_t xmlSize, BookMetadataCache* cache)
-      : baseContentPath(baseContentPath), remainingSize(xmlSize), cache(cache) {}
+  explicit TocNavParser(const std::string& baseContentPath, const size_t xmlSize, BookMetadataCache* cache,
+                        PageListSink* pageListSink)
+      : baseContentPath(baseContentPath), remainingSize(xmlSize), cache(cache), pageListSink(pageListSink) {}
   ~TocNavParser() override;
 
   bool setup();
 
   size_t write(uint8_t) override;
   size_t write(const uint8_t* buffer, size_t size) override;
-
-  const std::vector<PageListEntry>& getPageList() const { return pageList; }
 };

lib/Epub/Epub/parsers/TocNcxParser.cpp

@@ -4,6 +4,7 @@
 #include <Logging.h>
 
 #include "../BookMetadataCache.h"
+#include "PageListSink.h"
 
 bool TocNcxParser::setup() {
   parser = XML_ParserCreate(nullptr);
@@ -228,7 +229,7 @@ void XMLCALL TocNcxParser::endElement(void* userData, const XML_Char* name) {
   }
 
   if (self->state == IN_PAGE_TARGET && strcmp(name, "pageTarget") == 0) {
-    if (!self->currentPageLabel.empty() && !self->currentPageSrc.empty()) {
+    if (self->pageListSink && !self->currentPageLabel.empty() && !self->currentPageSrc.empty()) {
       std::string href = FsHelpers::normalisePath(self->baseContentPath + self->currentPageSrc);
       std::string anchor;
       const size_t pos = href.find('#');
@@ -236,7 +237,7 @@ void XMLCALL TocNcxParser::endElement(void* userData, const XML_Char* name) {
         anchor = href.substr(pos + 1);
         href = href.substr(0, pos);
       }
-      self->pageList.push_back({std::move(href), std::move(anchor), self->currentPageLabel});
+      self->pageListSink->addEntry(href, anchor, self->currentPageLabel);
     }
     self->currentPageLabel.clear();
     self->currentPageSrc.clear();

lib/Epub/Epub/parsers/TocNcxParser.h

@@ -3,9 +3,9 @@
 #include <expat.h>
 
 #include <string>
-#include <vector>
 
 class BookMetadataCache;
+class PageListSink;
 
 class TocNcxParser final : public Print {
   enum ParserState {
@@ -22,20 +22,15 @@ class TocNcxParser final : public Print {
     IN_PAGE_TARGET_LABEL_TEXT,
   };
 
- public:
-  // One printed-page reference from <pageList>: file href (normalised) + anchor fragment + visible label.
-  struct PageListEntry {
-    std::string href;    // normalised path to spine item
-    std::string anchor;  // fragment (empty = top of file)
-    std::string label;   // value shown to the reader (e.g. "1", "iv")
-  };
-
  private:
   const std::string& baseContentPath;
   size_t remainingSize;
   XML_Parser parser = nullptr;
   ParserState state = START;
   BookMetadataCache* cache;
+  // Page-list entries are streamed straight to disk via this sink. Owned by
+  // the caller (Epub.cpp); may be null when no page-list output is wanted.
+  PageListSink* pageListSink;
 
   std::string currentLabel;
   std::string currentSrc;
@@ -44,21 +39,19 @@ class TocNcxParser final : public Print {
   // <pageList> collection state
   std::string currentPageLabel;
   std::string currentPageSrc;
-  std::vector<PageListEntry> pageList;
 
   static void startElement(void* userData, const XML_Char* name, const XML_Char** atts);
   static void characterData(void* userData, const XML_Char* s, int len);
   static void endElement(void* userData, const XML_Char* name);
 
  public:
-  explicit TocNcxParser(const std::string& baseContentPath, const size_t xmlSize, BookMetadataCache* cache)
-      : baseContentPath(baseContentPath), remainingSize(xmlSize), cache(cache) {}
+  explicit TocNcxParser(const std::string& baseContentPath, const size_t xmlSize, BookMetadataCache* cache,
+                        PageListSink* pageListSink)
+      : baseContentPath(baseContentPath), remainingSize(xmlSize), cache(cache), pageListSink(pageListSink) {}
   ~TocNcxParser() override;
 
   bool setup();
 
   size_t write(uint8_t) override;
   size_t write(const uint8_t* buffer, size_t size) override;
-
-  const std::vector<PageListEntry>& getPageList() const { return pageList; }
 };

src/activities/reader/EpubReaderActivity.cpp

@@ -2541,8 +2541,14 @@ void EpubReaderActivity::renderStatusBar() const {
                                                       static_cast<uint16_t>(section->currentPage));
   std::string printedPageLabel;
   if (section) {
-    if (const auto label = section->getPrintedPageLabelForPage(static_cast<uint16_t>(section->currentPage))) {
+    const auto page = static_cast<uint16_t>(section->currentPage);
+    if (const auto label = section->getPrintedPageLabelForPage(page)) {
+      // Exact-match label (already parenthesised, may be "7/8" when multiple anchors collapse).
       printedPageLabel = *label;
+    } else if (const auto nearest = section->getNearestPrintedPageLabelAtOrBefore(page)) {
+      // No pagebreak on this device page: show the last printed-page label we passed within
+      // this section so the status bar still tells the reader which printed page they're on.
+      printedPageLabel = std::string("(") + *nearest + ")";
     }
   }
   GUI.drawStatusBar(renderer, bookProgress, currentPage, pageCount, title, 0, isStarred, printedPageLabel);