docs(package): record known gaps and correct stale claims
Notes the export verb's overwrite-consent obligation post-append, the append's extension-divergence behavior, and readFilePayload's 4GiB blind spot; fixes a stale u8string() reference and marks the 4GiB guard as accepted-unexercised.
This commit is contained in:
@@ -19,7 +19,7 @@ belong to the verbs.
|
|||||||
`std::filesystem` decodes a narrow path through the runtime ANSI code page (measured
|
`std::filesystem` decodes a narrow path through the runtime ANSI code page (measured
|
||||||
`GetACP() == 1252`), so a bare `fs::path(std::string)` turns `café.rsbank` into
|
`GetACP() == 1252`), so a bare `fs::path(std::string)` turns `café.rsbank` into
|
||||||
`café.rsbank` or fails to open it. Every path a verb hands in or gets back —
|
`café.rsbank` or fails to open it. Every path a verb hands in or gets back —
|
||||||
including `listFolderFileNames`' results, which use `u8string()` and never
|
including `listFolderFileNames`' results, which go through `pathToUtf8()` and never
|
||||||
`string()` — is UTF-8. `core/util/file_bytes` has the un-converted shape, which is
|
`string()` — is UTF-8. `core/util/file_bytes` has the un-converted shape, which is
|
||||||
why `readFilePayload` reads through this module's own `PackageFileReader` instead.
|
why `readFilePayload` reads through this module's own `PackageFileReader` instead.
|
||||||
- **Atomic package write, to the limit of a rename.** A package accumulates in a
|
- **Atomic package write, to the limit of a rename.** A package accumulates in a
|
||||||
@@ -47,7 +47,14 @@ belong to the verbs.
|
|||||||
writer could win the race against. Collision handling (auto-rename) remains the
|
writer could win the race against. Collision handling (auto-rename) remains the
|
||||||
import plan's job upstream. The package writer itself DOES replace an existing
|
import plan's job upstream. The package writer itself DOES replace an existing
|
||||||
destination — the export save dialog's own overwrite confirm is the consent — and
|
destination — the export save dialog's own overwrite confirm is the consent — and
|
||||||
that asymmetry is deliberate.
|
that asymmetry is deliberate. **Known gap, obligation on the export verb:**
|
||||||
|
`pickPackageSavePath`'s own `.rsbank` re-append (see its Gotcha below) can turn a
|
||||||
|
confirmed path `X` into a write target `X.rsbank` that the dialog never asked about.
|
||||||
|
The export verb MUST re-check `fileStatus()` on the path actually handed to
|
||||||
|
`PackageFileWriter` — after any extension append — and get its own consent if that
|
||||||
|
re-checked path is `Present`; the dialog's confirm only ever covered the pre-append
|
||||||
|
path. Not fixed at this seam: prompting is verb-level UX, and `pickPackageSavePath`
|
||||||
|
has no caller yet, so the gap is latent, not live.
|
||||||
- **The rollback delete is prune's ONE carve-out, and only HALF of it is structural.**
|
- **The rollback delete is prune's ONE carve-out, and only HALF of it is structural.**
|
||||||
The citation and the full discriminator live at `package_rollback.cpp`'s header.
|
The citation and the full discriminator live at `package_rollback.cpp`'s header.
|
||||||
"Did this call create it" is structural: only exclusively-created paths are
|
"Did this call create it" is structural: only exclusively-created paths are
|
||||||
@@ -88,7 +95,13 @@ belong to the verbs.
|
|||||||
an extension from `extension_list` when the user omits one — `pickPackageSavePath`
|
an extension from `extension_list` when the user omits one — `pickPackageSavePath`
|
||||||
re-appends `.rsbank` itself so the returned path is correct regardless of how that
|
re-appends `.rsbank` itself so the returned path is correct regardless of how that
|
||||||
lands (the superseded Win32 path had `ofn.lpstrDefExt` for this; `GetUserFileName`
|
lands (the superseded Win32 path had `ofn.lpstrDefExt` for this; `GetUserFileName`
|
||||||
has no equivalent parameter).
|
has no equivalent parameter). The re-append is suffix-blind: it only skips when the
|
||||||
|
path already ends in `.rsbank`, so a path carrying a DIFFERENT extension gets
|
||||||
|
`.rsbank` appended after it (`mybank.bak` → `mybank.bak.rsbank`), unlike the
|
||||||
|
superseded `ofn.lpstrDefExt`, which appended only when the path had no extension at
|
||||||
|
all. Defensible for a format-locked export, but a real divergence from the old
|
||||||
|
picker's behavior — whoever tests the picker under `[verify — DAW]` should expect
|
||||||
|
the double-extension result on a path that already has one.
|
||||||
- `pickPackageSavePath`'s `suggestedPath` doubles as the dialog's starting directory
|
- `pickPackageSavePath`'s `suggestedPath` doubles as the dialog's starting directory
|
||||||
when it is a full path. The verbs should seed it from the project directory —
|
when it is a full path. The verbs should seed it from the project directory —
|
||||||
passing a bare name leaves the dialog on REAPER's process working directory, which
|
passing a bare name leaves the dialog on REAPER's process working directory, which
|
||||||
|
|||||||
@@ -155,6 +155,12 @@ PayloadBuffer PackageFileReader::readRange(std::uint64_t offset, std::uint64_t l
|
|||||||
// so would read past a truncated allocation) and a length past the sanity
|
// so would read past a truncated allocation) and a length past the sanity
|
||||||
// ceiling, which exists so a merely large-but-real file size can't still hand
|
// ceiling, which exists so a merely large-but-real file size can't still hand
|
||||||
// std::vector a multi-gigabyte demand.
|
// std::vector a multi-gigabyte demand.
|
||||||
|
//
|
||||||
|
// `length > size_` short-circuits before the two branches below ever see a real
|
||||||
|
// file, so neither is reachable without a genuine >4 GiB fixture — this guard
|
||||||
|
// ships unexercised by test_package_io.cpp, which covers past-the-end,
|
||||||
|
// starts-at-the-end, and zero-length only. The ordering (cheap size check first)
|
||||||
|
// is deliberate and correct; it is not reordered to make the branch testable.
|
||||||
constexpr std::uint64_t kMaxReadRangeBytes = std::uint64_t{4} << 30; // 4 GiB
|
constexpr std::uint64_t kMaxReadRangeBytes = std::uint64_t{4} << 30; // 4 GiB
|
||||||
if (!ok_ || length == 0 || length > size_ || offset > size_ - length ||
|
if (!ok_ || length == 0 || length > size_ || offset > size_ - length ||
|
||||||
length > kMaxReadRangeBytes ||
|
length > kMaxReadRangeBytes ||
|
||||||
|
|||||||
@@ -101,7 +101,9 @@ private:
|
|||||||
};
|
};
|
||||||
|
|
||||||
// One source file read whole as one entry's payload — a bank file IS the streaming
|
// One source file read whole as one entry's payload — a bank file IS the streaming
|
||||||
// unit. Empty on any failure, per PackageFileReader.
|
// unit. Empty on any failure, per PackageFileReader — including a source file over
|
||||||
|
// readRange's 4 GiB ceiling, which reads as empty exactly like an unreadable file;
|
||||||
|
// fileStatus() cannot tell the two apart either, since it only checks openability.
|
||||||
PayloadBuffer readFilePayload(const std::string& absPath);
|
PayloadBuffer readFilePayload(const std::string& absPath);
|
||||||
|
|
||||||
// Export must tell a missing indexed file from an unreadable one in its refusal
|
// Export must tell a missing indexed file from an unreadable one in its refusal
|
||||||
|
|||||||
Reference in New Issue
Block a user