review #3: #4 の指摘9件への修正

docs/file-formats.md

@@ -90,13 +90,6 @@ if (parsedSize != fileSize) {
 
 ## `section.bin`
 
-### Version 49
-
-Version 49 keeps the version 48 serialized layout unchanged. It was bumped
-because variation selectors (U+FE00..U+FE0F and U+E0100..U+E01EF) are now
-dropped while parsing instead of being laid out as replacement glyphs, so
-cached line breaks and word positions from version 48 no longer match.
-
 ### Version 48
 
 Version 48 keeps the version 47 serialized layout unchanged. It was bumped
@@ -207,7 +200,7 @@ import std.mem;
 import std.string;
 import std.core;
 
-#define EXPECTED_VERSION 49
+#define EXPECTED_VERSION 48
 #define MAX_STRING_LENGTH 65535
 #define FOOTNOTE_NUMBER_LEN 32
 #define FOOTNOTE_HREF_LEN 256
@@ -422,7 +415,7 @@ Written by `lib/LibraryIndex/LibraryBuilder.cpp`, read by `LibraryIndexFile`. On
 file describing every book on the card, so the shelf can sort and search
 thousands of titles without opening any of them.
 
-Format version 3. An index written by another version fails validation on open
+Format version 2. An index written by another version fails validation on open
 and is rebuilt; that is the entire migration mechanism.
 
 ### Layout
@@ -433,15 +426,14 @@ and is rebuilt; that is the entire migration mechanism.
 | Folders | `folderStart` | length-prefixed paths, one per folder |
 | Records | `recordStart` | `bookCount` × 128-byte `ClixRecord` |
 | Permutations | `permStart` | `bookCount` u16 author order, then `bookCount` u16 arrival order |
-| Name blob | `nameStart` | per record: path hash, name, canonical author, title, source author, author reading (see below) |
+| Name blob | `nameStart` | per record: path hash, name, canonical author, title, source author (see below) |
 
 The arrival permutation runs oldest first, keyed by the record's FAT
 modification time (when the file landed on the card); `firstSeen` — the
 build-assigned discovery counter — breaks ties and carries books whose
 filesystem reports no time. Fold version 3 introduced the timestamp key; a
 fold bump rebuilds ranks while preserving `firstSeen`.
 Fold version 4 preserves leading articles in title sort and search keys.
-Fold version 5 folds a book's `file-as` reading where it carries one (see below).
 
 Sections are 512-byte aligned so each starts on an SD block boundary.
 
@@ -456,22 +448,6 @@ the author's words folded and sorted so that "Victor Hugo" and "Hugo Victor" gro
 one person. `authorKey` is a GROUPING key, not an ordering one: the shelf orders by
 surname, derived separately from the display name.
 
-A book that carries sort forms in its package metadata (EPUB 3 `file-as`, or
-EPUB 2 `opf:file-as`) is folded from those instead of its display strings. For
-Japanese that is the kana reading, the only way to order a 漢字 title; the fold
-maps katakana to hiragana and fullwidth ASCII to ASCII so either spelling gives
-one key, and the group initial of a kana fold is the head of its gojūon row
-(が → か). An author reading keys the group in the publisher's own order,
-family name first, and orders the shelf in place of the surname guess.
-
-`authorKey` and the phase-local sort keys hold the fold in `packSortKey()` byte
-form: ASCII and every non-Japanese script keep their UTF-8 bytes, hiragana packs
-to one byte each at `0x80 + (cp - 0x3040)`, so twelve bytes hold twelve kana
-instead of four and kana sort after Latin. Titles whose packed prefixes tie are
-ordered by their full folds, read back from the stage for those runs only
-(one checked 6,336-byte allocation; a run longer than 64 keeps walk order), so
-the volumes of a series come out in order.
-
 The byte before the folded title records metadata extraction status: not
 attempted, extracted, or failed. The final four bytes contain the packed FAT
 modification date and time returned by SdFat. A zero timestamp is not trusted.
@@ -492,13 +468,8 @@ Per record, at `nameStart + nameOff`:
 [u8][author]     display author, one spelling chosen per authorKey across the library
 [u8][title]      the book's own title, or length 0 if it never gave one
 [u8][source]     cleaned author spelling before the library-wide spelling vote
-[u8][reading]    folded file-as reading of the author, or length 0 if it gave none
 ```
 
-The reading is stored so an unchanged rebuild can reproduce the author order
-without opening the book again; it is folded already because only its fold is
-ever compared.
-
 The filename must stay the first textual field and stay the filename: `readPath`
 rebuilds a book's path from it, so writing the display title there makes the book
 impossible to open. That was a real defect, and it is why title has its own field.

lib/EpdFont/SdCardFontCache.cpp

@@ -7,13 +7,17 @@
 #include <SdCardFont.h>
 
 #include <algorithm>
+#include <atomic>
 #include <cstring>
 
 #include "SdCardFontCacheFormat.h"
 
 namespace SdCardFontCache {
 namespace {
 
+// Set by invalidateUntilReboot() from the task running a firmware update, read by the render task.
+std::atomic<bool> slotBeingUpdated{false};
+
 using sd_card_font_cache_format::Header;
 
 constexpr size_t CHUNK_SIZE = 4096;
@@ -132,6 +136,7 @@ bool sourceFits(const char* sourcePath, size_t* payloadBytes) {
 
 bool isValidFor(const char* sourcePath, size_t* payloadBytes) {
   if (payloadBytes) *payloadBytes = 0;
+  if (slotBeingUpdated.load()) return false;
 
   const HalOtaSlot slot = HalOtaSlot::inactive();
   Header header{};
@@ -145,7 +150,7 @@ bool isValidFor(const char* sourcePath, size_t* payloadBytes) {
 
 bool readAt(size_t offset, void* data, size_t length, size_t payloadBytes) {
   static const HalOtaSlot slot = HalOtaSlot::inactive();
-  if (payloadBytes > payloadCapacity(slot) ||
+  if (slotBeingUpdated.load() || payloadBytes > payloadCapacity(slot) ||
       !sd_card_font_cache_format::containsPayloadRange(payloadBytes, offset, length)) {
     return false;
   }
@@ -252,4 +257,6 @@ const char* resultName(Result result) {
   return "unknown";
 }
 
+void invalidateUntilReboot() { slotBeingUpdated.store(true); }
+
 }  // namespace SdCardFontCache

lib/EpdFont/SdCardFontCache.h

@@ -43,5 +43,10 @@ bool readAt(size_t offset, void* data, size_t length, size_t payloadBytes);
 // (copy, then read-back verification).
 Result preload(const char* sourcePath, ProgressCallback progress = nullptr, void* context = nullptr);
 const char* resultName(Result result);
+// A firmware update is about to erase and write the slot the copy lives in. From here until the
+// next boot isValidFor() and readAt() report no copy, so a loaded font falls back to the card
+// instead of reading erased or new-image bytes (an update that fails returns to the UI without
+// rebooting). A successful update reboots, and the header check then rebuilds the copy.
+void invalidateUntilReboot();
 
 }  // namespace SdCardFontCache

lib/Epub/Epub/ParsedText.cpp

@@ -123,38 +123,38 @@ std::vector<size_t> cjkCharacterBreakByteOffsets(const std::string& text) {
   // which for a Japanese paragraph -- one unspaced "word" -- cost 8x the paragraph's
   // bytes in one block: 20 KB for a 2.5 KB paragraph, requested mid-build where the
   // largest free block was 5 KB. With -fno-exceptions the failed reserve was a
-  // terminate() (crash 2026-09-17, horizontal-regression-test ch.14). The result vector
-  // is bounded by one entry per codepoint; pure CJK is 3 bytes each, so reserve a third
-  // and let a mixed run grow.
-  std::vector<size_t> allowedOffsets;
-  const auto* ptr = reinterpret_cast<const unsigned char*>(text.c_str());
-  const auto* const start = ptr;
-  uint32_t prevCp = utf8NextCodepoint(&ptr);
-  if (prevCp == 0) return {};
-  size_t prevEnd = static_cast<size_t>(ptr - start);
-  bool hasCjkBreakable = utf8IsCjkBreakable(prevCp);
-  bool reserved = false;
-  while (*ptr) {
-    const uint32_t cp = utf8NextCodepoint(&ptr);
-    if (cp == 0) break;
-    if (utf8IsCjkBreakable(cp)) hasCjkBreakable = true;
-    if (hasCjkBreakOpportunityBetween(prevCp, cp)) {
-      if (!reserved) {
-        // First opportunity found: size the result once. Skip the whole per-character
-        // split when even that block cannot be placed -- the word then stays one token,
-        // and the line pre-check reports the page instead of the allocator aborting.
-        const size_t want = text.size() / 3 + 1;
-        constexpr size_t RESERVE_HEADROOM = 4 * 1024;
-        if (ESP.getMaxAllocHeap() < want * sizeof(size_t) + RESERVE_HEADROOM) return {};
-        allowedOffsets.reserve(want);
-        reserved = true;
-      }
-      allowedOffsets.push_back(prevEnd);
+  // terminate() (crash 2026-09-17, horizontal-regression-test ch.14). The result is sized
+  // exactly: a first pass counts the opportunities without allocating (a guess by byte
+  // length is short for mixed runs such as "a漢a漢", where every boundary breaks, and
+  // over-reserves pure CJK), then the vector is reserved once and never regrows.
+  const auto* const start = reinterpret_cast<const unsigned char*>(text.c_str());
+  const auto forEachBreak = [start](auto&& onBreak) {
+    const auto* ptr = start;
+    uint32_t prevCp = utf8NextCodepoint(&ptr);
+    if (prevCp == 0) return false;
+    size_t prevEnd = static_cast<size_t>(ptr - start);
+    bool hasCjkBreakable = utf8IsCjkBreakable(prevCp);
+    while (*ptr) {
+      const uint32_t cp = utf8NextCodepoint(&ptr);
+      if (cp == 0) break;
+      if (utf8IsCjkBreakable(cp)) hasCjkBreakable = true;
+      if (hasCjkBreakOpportunityBetween(prevCp, cp)) onBreak(prevEnd);
+      prevCp = cp;
+      prevEnd = static_cast<size_t>(ptr - start);
     }
-    prevCp = cp;
-    prevEnd = static_cast<size_t>(ptr - start);
-  }
-  if (!hasCjkBreakable) return {};
+    return hasCjkBreakable;
+  };
+
+  size_t count = 0;
+  if (!forEachBreak([&count](size_t) { count++; }) || count == 0) return {};
+  // Skip the whole per-character split when even that block cannot be placed -- the word
+  // then stays one token, and the line pre-check reports the page instead of the allocator
+  // aborting.
+  constexpr size_t RESERVE_HEADROOM = 4 * 1024;
+  if (ESP.getMaxAllocHeap() < count * sizeof(size_t) + RESERVE_HEADROOM) return {};
+  std::vector<size_t> allowedOffsets;
+  allowedOffsets.reserve(count);
+  forEachBreak([&allowedOffsets](size_t offset) { allowedOffsets.push_back(offset); });
   return allowedOffsets;
 }
 

lib/Epub/Epub/Section.cpp

@@ -67,7 +67,7 @@ namespace {
 // v53: Vertical columns carry link rectangles, so the footnote selector can reach them.
 // v54: Images honour max-width/max-height; vertical books set images in the column flow.
 // v55: A vertical page holding a single image centres it horizontally.
-constexpr uint8_t SECTION_FILE_VERSION = 55;
+constexpr uint8_t SECTION_FILE_VERSION = 56;
 // Written into the version field while a build is in progress; patched to
 // SECTION_FILE_VERSION only when the build is finalized. An abandoned /
 // crash-interrupted .bin therefore carries version 0, which loadSectionFile rejects

lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp

@@ -1045,6 +1045,17 @@ void ChapterHtmlSlimParser::emitHorizontalRule(const BlockStyle& blockStyle) {
     startNewTextBlock(parentBlockStyle);
   }
 
+  if (isVertical) {
+    // Vertical pages fill by column (currentPageNextX) and never advance currentPageNextY, so a
+    // horizontal rule would be drawn across the column heads. The break is kept as one blank
+    // column, the usual form of a scene break in vertical text. At the start of a page it adds
+    // nothing, and a cursor already past the left edge makes the next column start a new page.
+    if (currentPage && !currentPage->elements.empty() && verticalCursorPageIndex == completedPageCount) {
+      currentPageNextX = static_cast<int16_t>(currentPageNextX - verticalColumnWidth() - verticalColumnSpacing());
+    }
+    return;
+  }
+
   if (!currentPage) {
     currentPage.reset(new (std::nothrow) Page());
     if (!currentPage) {
@@ -1156,6 +1167,9 @@ void ChapterHtmlSlimParser::closeTableCell() {
 }
 
 void ChapterHtmlSlimParser::addTableRowSeparator() {
+  // Vertical pages never advance currentPageNextY: every separator would land as a full-width
+  // line across the column heads. The stacked rows read as separate paragraphs without it.
+  if (isVertical) return;
   if (!currentPage || currentPage->elements.empty() || viewportWidth == 0 ||
       currentPageNextY + TABLE_ROW_SEPARATOR_GAP > viewportHeight) {
     return;

lib/GfxRenderer/FontCacheManager.cpp

@@ -207,12 +207,13 @@ FontCacheManager::PrewarmScope::PrewarmScope(FontCacheManager& manager) : manage
 void FontCacheManager::PrewarmScope::endScanAndPrewarm() {
   const bool wasScanning = manager_->scanMode_ == ScanMode::Scanning;
   manager_->scanMode_ = ScanMode::None;
-  if (manager_->scanCodepointCount_ == 0) return;
-
+  // Before the empty-scan return: a page scope whose hook never fired has to report zero, not the
+  // previous page's figures.
   if (wasScanning) {
     manager_->lastScanBytes_ = 0;
     manager_->lastScanFonts_ = 0;
   }
+  if (manager_->scanCodepointCount_ == 0) return;
 
   std::sort(manager_->scanCodepoints_, manager_->scanCodepoints_ + manager_->scanCodepointCount_);
 

lib/Serialization/BufferedFile.h

@@ -156,17 +156,22 @@ inline void writeString(BufferedFileWriter& out, const std::string& s) {
 // rejected length leaves the stream desynced, so the caller must abort the
 // whole record rather than continue parsing.
 [[nodiscard]] inline bool readString(BufferedFileReader& in, std::string& s) {
-  uint32_t len;
-  readPod(in, len);
+  uint32_t len = 0;
+  if (in.read(&len, sizeof(len)) != sizeof(len)) {
+    s.clear();
+    return false;
+  }
   // See Serialization.h's MAX_SERIALIZED_STRING_LEN for why this is capped.
   if (len > MAX_SERIALIZED_STRING_LEN) {
     LOG_ERR("SER", "readString: length %u exceeds max, treating as corrupt", len);
     s.clear();
     return false;
   }
   s.resize(len);
-  if (len > 0) {
-    in.read(&s[0], len);
+  // A file cut short inside the string is as corrupt as an oversized length.
+  if (len > 0 && in.read(&s[0], len) != len) {
+    s.clear();
+    return false;
   }
   return true;
 }

lib/Serialization/Serialization.h

@@ -58,28 +58,39 @@ constexpr uint32_t MAX_SERIALIZED_STRING_LEN = 65535;
 // already returns nullptr on its own corruption checks) rather than pressing
 // on.
 [[nodiscard]] inline bool readString(std::istream& is, std::string& s) {
-  uint32_t len;
+  uint32_t len = 0;
   readPod(is, len);
-  if (len > MAX_SERIALIZED_STRING_LEN) {
+  if (!is || len > MAX_SERIALIZED_STRING_LEN) {
     LOG_ERR("SER", "readString: length %u exceeds max, treating as corrupt", len);
     s.clear();
     return false;
   }
   s.resize(len);
-  is.read(&s[0], len);
+  // A stream cut short inside the string is as corrupt as an oversized length.
+  if (len > 0 && !is.read(&s[0], len)) {
+    s.clear();
+    return false;
+  }
   return true;
 }
 
 [[nodiscard]] inline bool readString(HalFile& file, std::string& s) {
-  uint32_t len;
-  readPod(file, len);
+  uint32_t len = 0;
+  if (file.read(reinterpret_cast<uint8_t*>(&len), sizeof(len)) != static_cast<int>(sizeof(len))) {
+    s.clear();
+    return false;
+  }
   if (len > MAX_SERIALIZED_STRING_LEN) {
     LOG_ERR("SER", "readString: length %u exceeds max, treating as corrupt", len);
     s.clear();
     return false;
   }
   s.resize(len);
-  file.read(&s[0], len);
+  // A file cut short inside the string is as corrupt as an oversized length.
+  if (len > 0 && file.read(&s[0], len) != static_cast<int>(len)) {
+    s.clear();
+    return false;
+  }
   return true;
 }
 }  // namespace serialization

lib/ZipFile/ZipFile.cpp

@@ -457,8 +457,9 @@ namespace {
 // more room (2026-09-17, p050.jpg of a commercial volume). Smaller chunks cost
 // more trips to the card and nothing else, so a slow picture beats no picture.
 uint8_t* allocChunkDownTo(const size_t want, size_t* got) {
-  constexpr size_t CHUNK_FLOOR = 512;
-  for (size_t size = want; size >= CHUNK_FLOOR; size /= 2) {
+  // A request already below 512 is its own floor.
+  const size_t floor = std::min<size_t>(want, 512);
+  for (size_t size = want; size >= floor && size > 0; size /= 2) {
     if (auto* p = static_cast<uint8_t*>(malloc(size))) {
       *got = size;
       if (size != want) {

scripts/ost_version.py

@@ -181,7 +181,8 @@ def main(env):
     project_dir = env.subst('$PROJECT_DIR')
     pioenv = env.subst('$PIOENV')
     board = BOARD_TAGS.get(pioenv, '')
-    version = next_version(project_dir, advance=(pioenv == 'default'))
+    # Only a board with its own tag may share the day's number: its image name still differs.
+    version = next_version(project_dir, advance=(pioenv not in BOARD_TAGS))
     diag_at_pre = has_input_diag(env)
     write_build_id_header(project_dir, f'{version}{board}{"-diag" if diag_at_pre else ""}')
 

src/activities/settings/TextSettingsActivity.cpp

@@ -394,6 +394,9 @@ void TextSettingsActivity::applySize(int listIndex) {
 void TextSettingsActivity::applyFlashCacheSetting() {
   if (SETTINGS.sdFontFamilyName[0] == '\0') return;  // built-in family: nothing to copy, nothing to reload
   const bool wantFlash = SETTINGS.sdFontFlashCache != 0;
+  // Copy off and the font already read from the card: the family/size change has just loaded
+  // it that way, and a forced reload would read its tables from the SD a second time.
+  if (!wantFlash && !sdFontSystem.readerFontFromFlash()) return;
   if (wantFlash && !runFlashCopy()) {
     // runFlashCopy switched the setting off; the reload below returns the font to the card.
   }

src/activities/util/ClockActivity.cpp

@@ -153,7 +153,7 @@ void ClockActivity::render(RenderLock&&) {
   }
 #endif
 
-  Rtc::DateTime now;
+  Rtc::DateTime now{};
   const bool haveTime = halClock.getLocalDateTime(now);
   if (haveTime) {
     renderedMinute = now.minute;