Skip to content

DESIGN.md's IndexWriter and IndexReader sections document classes that do not exist #405

Description

@typeless

DESIGN.md's index chapter presents two module surfaces as C++ classes. Neither class exists. Documentation only — nothing in the code is wrong.

The two instances

### IndexWriter opens with

class IndexWriter {
    static auto write(path, index) -> Result<Unit>;
};

### IndexReader does the same for a reader type.

$ grep -rn 'class IndexWriter\|struct IndexWriter\|class IndexReader\|struct IndexReader' src/ include/
$

Both surfaces are free functions:

Documented Actual
IndexWriter::write(path, index) -> Result<Unit> write_index(std::string_view, Index const&) -> Result<void>src/index/writer.cpp:150
serialize_index(Index const&) -> Result<Vec<std::byte>>src/index/writer.cpp:163
IndexReader::… read_index(IndexFile const&) -> Result<Index>src/index/reader.cpp:126; read_index(std::string_view) -> Result<Index>reader.cpp:186
index_verify_checksum(IndexFile const&) -> boolreader.cpp:437, declared include/pup/index/reader.hpp:82

The return type is wrong too: Result<Unit>, where the codebase uses Result<void>.

The "Write process" list is wrong in the same way

Under ### IndexWriter:

  1. Serialize to temporary file
  2. Compute SHA-256 checksum
  3. Write footer with checksum
  4. Atomic rename to final path

Steps 1-3 describe operations on a file, and none of them touch one. serialize_index builds a Vec<std::byte> in memory; the checksum is computed over that buffer and the footer written into it at src/index/writer.cpp:363-366, before any file exists. write_index then hands the finished bytes to pup::platform::atomic_write, and the temporary file and the rename both live inside atomic_write — the platform layer's business, not the writer's. So step 4 names a real thing but attributes it to the wrong layer.

Why this is worth fixing rather than tolerating

The atomicity paragraph merged as 26ddb61 sits directly beneath that list and is load-bearing: it is what entitles the record reader to announce damage rather than suspect its own half-finished write. The stale list above it currently misattributes where the checksum lands, so a reader who catches the falsehood is invited to doubt the correct paragraph below it. Worse, a list claiming the writer stages its own file invites exactly the change that guarantee cannot survive — a writer that stages a file outside atomic_write.

The class is DESIGN.md presenting a module surface as a class that does not exist. Both instances should be fixed as a pair so the fix is checked as one.

Shape of the fix

Present both surfaces as the free functions they are; drop the class blocks. Re-cut the write list along the real seam:

  1. serialize_index builds the whole record in memory, computing the SHA-256 over the buffer and writing the footer into it.
  2. write_index hands that buffer to pup::platform::atomic_write, which owns the temporary file and the rename.

Constraint for whoever takes it: do not restate atomic_write's temp/fsync/rename steps in the list, and do not reword the atomicity paragraph below — the paragraph is that rationale's home, and restating it would put the same claim at two sites.

Found while promoting the record laws into DESIGN.md; deliberately left out of that doc-only change because it is an independent defect. The IndexReader sibling was found during the design review of the fix.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions