Review comments

This commit is contained in:
jpirnay
2026-04-20 13:28:56 +02:00
parent 3fb9ac9e29
commit 29284c85c9
10 changed files with 129 additions and 127 deletions
@@ -223,7 +223,10 @@ std::string findXPathForParagraphInternal(const std::shared_ptr<Epub>& epub, con
// Partial parse missed — reset and retry from beginning with full-document context.
XML_ParserFree(parser);
parser = XML_ParserCreate(nullptr);
if (parser) {
if (!parser) {
LOG_ERR("KOX", "XML_ParserCreate failed on retry: spine=%d p[%u] tmp=%s", spineIndex, paragraphIndex,
tmpPath.c_str());
} else {
ParagraphState fullState(spineIndex, paragraphIndex, 0, false);
fullState.parser = parser;
XML_SetUserData(parser, &fullState);
+6
View File
@@ -76,6 +76,12 @@ class ChapterXPathIndexer {
* @param paragraphIndex 1-based paragraph index (from section LUT or XPath p[N])
* @param seekHint Optional XHTML byte offset to start scanning from (0 = from beginning).
* Pass Section::getXhtmlByteOffsetForPage() to avoid scanning the whole file.
* @param startParagraphCount Optional seed count (default 0) of direct-body-child <p> elements
* that precede the seekHint position. Should be provided when seekHint > 0 to
* avoid counting from scratch mid-document; callers should pass the paragraph
* index of the LUT entry at the seek page minus 1. If the partial parse with
* this seed doesn't find the target, the function falls back to runParse from
* byte 0 and re-counts with startParagraphCount = 0.
* @return Full-ancestry XPath like "/body/DocFragment[N]/body/div[1]/p[3]", or empty on failure
*/
static std::string findXPathForParagraph(const std::shared_ptr<Epub>& epub, int spineIndex, uint16_t paragraphIndex,
@@ -245,33 +245,43 @@ std::string decompressToTempFile(const std::shared_ptr<Epub>& epub, const int sp
return tmpPath;
}
namespace {
// Pump the open `file` through `parser` in fixed-size chunks. Returns true on clean EOF or
// XML_ERROR_ABORTED (caller used XML_StopParser to signal an early success). Returns false on
// XML_GetBuffer failure or any other parse error. The file is left open — caller closes it.
bool pumpExpatFromFile(XML_Parser parser, FsFile& file) {
constexpr size_t kBufSize = 1024;
int done;
do {
void* const buf = XML_GetBuffer(parser, kBufSize);
if (!buf) {
return false;
}
const size_t len = file.read(buf, kBufSize);
done = file.available() == 0;
if (XML_ParseBuffer(parser, static_cast<int>(len), done) == XML_STATUS_ERROR) {
return XML_GetErrorCode(parser) == XML_ERROR_ABORTED;
}
} while (!done);
return true;
}
} // namespace
bool runParse(XML_Parser parser, const std::string& path) {
FsFile file;
if (!Storage.openFileForRead("KOX", path, file)) {
return false;
}
constexpr size_t kBufSize = 1024;
bool ok = true;
int done;
do {
void* const buf = XML_GetBuffer(parser, kBufSize);
if (!buf) {
ok = false;
break;
}
const size_t len = file.read(buf, kBufSize);
done = file.available() == 0;
if (XML_ParseBuffer(parser, static_cast<int>(len), done) == XML_STATUS_ERROR) {
ok = (XML_GetErrorCode(parser) == XML_ERROR_ABORTED);
break;
}
} while (!done);
const bool ok = pumpExpatFromFile(parser, file);
file.close();
return ok;
}
// Starts Expat mid-document. Since the parser has no ancestor context (html/body stack is
// missing), unmatched closing tags may appear, and callbacks emitted before the first start
// tag can look structurally odd — an empty result is normal here. Callers
// (ChapterXPathForwardMapper.cpp) recognise the empty result and fall back to runParse from
// byte 0 with full document context.
bool runParseFromOffset(XML_Parser parser, const std::string& path, const uint32_t seekBytes) {
if (seekBytes == 0) {
return runParse(parser, path);
@@ -287,23 +297,7 @@ bool runParseFromOffset(XML_Parser parser, const std::string& path, const uint32
return runParse(parser, path); // fall back to full scan if seek fails
}
constexpr size_t kBufSize = 1024;
bool ok = true;
int done;
do {
void* const buf = XML_GetBuffer(parser, kBufSize);
if (!buf) {
ok = false;
break;
}
const size_t len = file.read(buf, kBufSize);
done = file.available() == 0;
if (XML_ParseBuffer(parser, static_cast<int>(len), done) == XML_STATUS_ERROR) {
ok = (XML_GetErrorCode(parser) == XML_ERROR_ABORTED);
break;
}
} while (!done);
const bool ok = pumpExpatFromFile(parser, file);
file.close();
return ok;
}
+8 -2
View File
@@ -63,8 +63,14 @@ KOReaderPosition ProgressMapper::toKOReader(const std::shared_ptr<Epub>& epub, c
// where paragraphs are nested inside divs/sections. Fall back to the progress-based
// scan (which works for any content) when no paragraph index is available.
if (pos.hasParagraphIndex && pos.paragraphIndex > 0) {
result.xpath =
ChapterXPathIndexer::findXPathForParagraph(epub, pos.spineIndex, pos.paragraphIndex, pos.xhtmlSeekHint);
// When a seek hint is set, the LUT entry's paragraphIndex equals pos.paragraphIndex
// (both describe the same page). The byte offset now points at the body-child element
// that was current at the page break, so re-parsing from there will re-encounter that
// paragraph — seed startParagraphCount with paragraphIndex-1 to avoid double counting.
const uint16_t startCount =
pos.xhtmlSeekHint > 0 && pos.paragraphIndex > 0 ? static_cast<uint16_t>(pos.paragraphIndex - 1) : 0;
result.xpath = ChapterXPathIndexer::findXPathForParagraph(epub, pos.spineIndex, pos.paragraphIndex,
pos.xhtmlSeekHint, startCount);
}
if (result.xpath.empty()) {
result.xpath = ChapterXPathIndexer::findXPathForProgress(epub, pos.spineIndex, intraSpineProgress);