rust: allocation-free filesystem CRUD, locked to the C core - #102
Conversation
Rust firmware works with the whole EEPROM, not just host tooling, so the file operations can no longer live in C alone. Port src/jeefs.c to jeefs_header::fs over a caller-owned &mut [u8]: format, a validating chain iterator, read, add, overwrite and delete, plus the board-header consistency check. Nothing allocates — the module builds in bare no_std, next to the header core, while whole-image build/parse stays behind the alloc feature. Every rule is the C core's rule: the fs_version gate, headerCrc32 verified before any field is trusted, NUL-terminated names, contiguity and bounds checks, an erased 0xFFFF link as a terminator, device.id taking the first slot with the shifted chain relinked, the headerless image claimed on first write but never a matching magic with an unknown version, and failure leaving the buffer byte-for-byte untouched. Two deliberate spellings differ from the C signatures: outcomes are Result rather than negative codes, and "the file is already there" — which AddFile reports as zero bytes written — is FsError::FileExists.
Add shared mutation vectors: a scenario script (init, format, add, write, delete, read, list, poke, consistency) that a C runner and a Rust runner both apply to the same starting image. The ctest case fs_mutation_c_matches_rs requires the two to agree on every operation's outcome and on the resulting image byte for byte, so a divergence in chain layout, link rewriting, wiping or error classification fails CI rather than reaching a device. The eight committed vectors cover the plain lifecycle, device.id insert-first, deletion from the middle, front and tail, both overwrite paths, claiming garbage and erased media, validation and capacity failures, header and payload corruption, and the fs_version gates.
The charter line reserving FS operations for C/C++ no longer holds: state that Rust carries an allocation-free port, and that any FS port is bound to the C core by the shared mutation vectors.
The C core is fuzzed; the Rust port was not. Teach the mutation driver to generate scenarios — the same operations in pseudo-random order, against zeroed, erased and garbage media, with byte pokes in between — and run 50 of them alongside the committed vectors on every ctest run. The seed is fixed, so a divergence is reproducible from the failure message; a wider sweep is a --random/--seed away. This is what catches a Rust panic (slice, subtraction, cast) or a layout drift on inputs nobody thought to write by hand: C and Rust must still agree on the journal and on every byte.
There was a problem hiding this comment.
🟡 Changes recommended
The new mutation-vector runners and CMake test wiring have unchecked/unbounded size handling that can crash CI or misconfigure tests (e.g., missing Python gating).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adds an allocation-free Rust implementation of JEEFS filesystem CRUD (jeefs_header::fs) intended to be mechanically locked to the existing C core via shared cross-language mutation vectors, and updates the project contract/docs accordingly.
Changes:
- Introduces
rust/jeefs-header/src/fs.rs(CRUD over&mut [u8]) plus a Rust unit test suite validating key invariants. - Adds shared
.opsmutation vectors and C/Rust runners, with a Python harness to compare journals and final image bytes. - Updates repo documentation/contract (README, IMPLEMENTATION_CONTRACT, CLAUDE.md) to reflect Rust FS support and the new conformance mechanism.
File summaries
| File | Description |
|---|---|
| tests/cross-language/verify_fs_mutation.py | Python harness comparing C vs Rust mutation-vector journals and output images |
| tests/cross-language/fs_vectors/01_basic_crud.ops | New shared FS mutation scenario |
| tests/cross-language/fs_vectors/02_device_id_first.ops | New shared FS mutation scenario |
| tests/cross-language/fs_vectors/03_delete_compaction.ops | New shared FS mutation scenario |
| tests/cross-language/fs_vectors/04_write_paths.ops | New shared FS mutation scenario |
| tests/cross-language/fs_vectors/05_claim_and_gates.ops | New shared FS mutation scenario |
| tests/cross-language/fs_vectors/06_error_paths.ops | New shared FS mutation scenario |
| tests/cross-language/fs_vectors/07_corruption.ops | New shared FS mutation scenario |
| tests/cross-language/fs_vectors/08_fs_version.ops | New shared FS mutation scenario |
| tests/cross-language/CMakeLists.txt | Builds new runners and adds a new cross-language conformance test |
| tests/cross-language/apply_ops_c.c | C runner for applying .ops scripts and emitting a journal + image |
| rust/jeefs-header/tests/fs.rs | Rust unit tests for the FS port’s invariants and error behavior |
| rust/jeefs-header/src/lib.rs | Exposes the new fs module |
| rust/jeefs-header/src/fs.rs | Core allocation-free Rust FS CRUD implementation |
| rust/jeefs-header/src/bin/apply_ops_rs.rs | Rust runner for applying .ops scripts and emitting a journal + image |
| rust/jeefs-header/Cargo.toml | Registers apply_ops_rs as a bins-gated binary |
| README.md | Updates language capabilities summary to include Rust FS |
| docs/IMPLEMENTATION_CONTRACT.md | Binds FS ports to shared mutation vectors |
| CLAUDE.md | Updates charter/architecture notes to reflect Rust FS support and conformance locking |
Review details
Suppressed comments (3)
tests/cross-language/apply_ops_c.c:71
init_image()castssizeintouint16_tbut then uses the originalsizeformemset/the loop without bounds checking. A malicious/accidental.opsvector with a large size would overflowimageand cause UB/crashes in CI.
static void init_image(const char *kind, unsigned size) {
image_size = (uint16_t) size;
if (strcmp(kind, "erased") == 0)
memset(image, 0xFF, size);
else if (strcmp(kind, "garbage") == 0)
rust/jeefs-header/src/bin/apply_ops_rs.rs:105
readallocatesvec![0u8; cap]directly from the script argument, which can OOM on a malformed vector. The C runner clamps reads to 65535 bytes; clamping here keeps the runner resilient and consistent.
"read" => {
let cap: usize = arg2.parse().unwrap_or(0);
let mut buf = vec![0u8; cap];
match read_file(&image, arg1, &mut buf) {
rust/jeefs-header/src/bin/apply_ops_rs.rs:44
init_image()will allocate aVecof whatever size the script requests. Adding an upper bound avoids accidental OOMs and keeps behavior closer to the C runner (which is inherently limited by its fixed buffer).
fn init_image(kind: &str, size: usize) -> Vec<u8> {
match kind {
"erased" => vec![0xFFu8; size],
// deterministic, carries no magic
"garbage" => (0..size).map(|i| (i.wrapping_mul(37).wrapping_add(11)) as u8).collect(),
- Files reviewed: 19/19 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The porting guide covered the C build only. State what a bare no_std dependency provides (header, device.id and the FS port over a caller buffer), when to add alloc or std, that the walker remains C-only, and which harness holds the Rust FS to the C core.
There was a problem hiding this comment.
🟡 Changes recommended
The new cross-language runners/verifier contain robustness issues (notably a potential buffer overflow in the C runner and unbounded allocations in the Rust runner) that should be fixed before relying on these tests in CI.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
rust/jeefs-header/src/bin/apply_ops_rs.rs:109
- The
readop allocates aVec<u8>of sizecapdirectly from the script, which allows.opsinput to force very large allocations even though the image itself bounds what can be read. Clampingcaptoimage.len()avoids OOM/slowdowns without changing operation semantics.
rust/jeefs-header/src/bin/apply_ops_rs.rs:29
parse_payload()can allocate an arbitrarily largeVec(e.g.fill:0:999999999or a huge hex string) before the FS layer rejects the size, which can OOM or stall CI when running mutation tests. Since the FS core caps payloads atINT16_MAXanyway, cap the parsed payload length here too and returnNoneon oversized specs.
/// "fill:<byte>:<count>" or a hex string.
fn parse_payload(spec: &str) -> Option<Vec<u8>> {
if let Some(rest) = spec.strip_prefix("fill:") {
let (b, n) = rest.split_once(':')?;
return Some(vec![b.parse::<u8>().ok()?; n.parse::<usize>().ok()?]);
- Files reviewed: 20/20 changed files
- Comments generated: 2
- Review effort level: Lite
Copilot found three ways the runners themselves could misbehave: a fill count in the millions made the Rust runner allocate before the FS code could reject the payload, a fill byte above 255 wrapped in the C runner's memset while Rust refused it (a runner disagreement, not a port one), and the new ctest case ran a Python driver while gated only on cargo. Bound every scenario-driven allocation in the Rust runner, reject an out-of-range fill byte in the C one, and require both toolchains for the test. The line-by-line audit also noted that C's ListFiles stops at its caller-supplied cap while the Rust iterator has none, so mirror that cap in the runner: the journals must compare the ports, not the API shapes.
Second Copilot pass on the harness: init_image memset the requested size into a fixed static buffer without checking it, so an oversized scenario ran past the end and truncated through the uint16_t cast — the gate existed in the Rust runner only. The driver also indexed past its argument list when --random or --seed came without a value. Refuse an out-of-range image in the C runner and print usage on a malformed invocation instead of raising.
There was a problem hiding this comment.
🟡 Changes recommended
The new test runners/docs include correctness issues (notably an out-of-bounds write risk in the C runner and a version mismatch in PORTING.md) that should be fixed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
rust/jeefs-header/src/fs.rs:158
filename_validaccepts Rust strings with embedded NUL ("\0"). That name can be written to the on-media NUL-terminated header but becomes unaddressable/ambiguous on subsequent operations, and it cannot occur in the C API (C strings can’t contain interior NULs). Reject embedded NULs to keep Rust semantics aligned with the C core.
tests/cross-language/apply_ops_c.c:75
init_imagewritessizebytes into the fixedimage[MAX_IMG]buffer without checking thatsize <= MAX_IMG, so a malformed/accidental scenario (e.g.init zeros 999999) will write out of bounds and corrupt memory during tests.
}
static void init_image(const char *kind, unsigned size) {
/* The scenario is external input: an oversized value would run past
* the static buffer and truncate through the uint16_t cast. */
if (size == 0 || size > MAX_IMG) {
fprintf(stderr, "image size out of range: %u\n", size);
exit(2);
}
- Files reviewed: 20/20 changed files
- Comments generated: 2
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
The new cross-language mutation harness has a few confirmed C/Rust runner parsing mismatches on malformed inputs that can cause false divergences unrelated to FS correctness.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
rust/jeefs-header/src/bin/apply_ops_rs.rs:193
parse_uint()returnsusize::MAXon parse failure, but the C runner'sstrtoul(..)returns 0 when it cannot parse. On malformedpokeoffsets this makes the runners behave differently (C may poke byte 0; Rust will no-op and report a huge offset), which can create false conformance failures.
tests/cross-language/apply_ops_c.c:65- Hex payload parsing is not strict:
sscanf("%2x")will accept a single hex digit (e.g. "0g" parses as 0) which the Rust runner rejects. That can make the C runner accept malformed.opsvectors and diverge from Rust for reasons unrelated to the FS port itself.
- Files reviewed: 20/20 changed files
- Comments generated: 1
- Review effort level: Lite
The previous fix made the C runner exit on `init ... 0` while the Rust one carried on with an empty buffer — an asymmetry that would show up as a journal divergence blamed on the ports.
There was a problem hiding this comment.
🔵 Needs a closer look
The Rust firmware porting docs pin jeefs-header to a version that doesn’t match the crate version in-repo and should be corrected before merge.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
rust/jeefs-header/src/bin/apply_ops_rs.rs:187
- The doc comment says this matches "C's strtoul(.., 0)", but the C runner explicitly avoids base-0 parsing (to prevent leading-zero octal) and instead accepts only decimal or a 0x/0X-prefixed hex value. Updating this avoids misleading future maintainers when debugging vector parsing mismatches.
- Files reviewed: 20/20 changed files
- Comments generated: 0 new
- Review effort level: Lite
The example pinned 0.11, a version the repo does not carry; show `cargo add` instead, which pins whatever the build resolves and cannot drift out of date.
The C runner no longer uses base-0 strtoul (it would read a leading zero as octal); say what both runners actually accept.
There was a problem hiding this comment.
🔵 Needs a closer look
It introduces a full new filesystem CRUD implementation (safety-critical in-place mutation logic) where final approval should include a careful human review beyond automated diff inspection.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
rust/jeefs-header/src/bin/apply_ops_rs.rs:193
- The
parse_uint()doc comment says it behaves “like C's strtoul(.., 0)”, but the C runner intentionally avoids base-0 parsing (to prevent octal) andparse_uint()also returnsusize::MAXon parse failure (unlikestrtoul, which yields 0). This mismatch can confuse future maintenance of the cross-language runner parity.
Update the comment to describe the actual behavior (decimal or 0x-hex, and invalid input treated as out-of-range).
- Files reviewed: 20/20 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
The C/Rust mutation-vector runners handle malformed poke arguments inconsistently (silent truncation/defaulting), which can create false cross-language divergences unrelated to FS behavior.
Review details
Suppressed comments (3)
Previously missed (2) — in code that hasn't changed since the last review.
rust/jeefs-header/src/bin/apply_ops_rs.rs:169
pokecurrently usesu8::from_str_radix(...).unwrap_or(0), which silently turns invalid or out-of-range hex into 0. That can hide malformed vectors and also disagrees with the C runner’s current behavior (which truncates >0xFF). For a differential test driver it’s better to fail loudly on bad input so divergences reflect FS logic, not script parsing.
This issue also appears on line 190 of the same file.
tests/cross-language/apply_ops_c.c:176
- In
poke,valis parsed withstrtoul(..., 16)and then truncated touint8_twithout validating that the input is actually a single byte. If a vector accidentally uses a value > 0xFF (or a non-hex string), C will silently truncate (or treat it as 0) while the Rust runner currently maps such cases differently, which can create false C-vs-Rust divergence unrelated to FS behavior. Consider validating the parse (endptr) and enforcingval <= 0xFF, failing fast on malformed vectors.
rust/jeefs-header/src/bin/apply_ops_rs.rs:194
parse_uint()returnsusize::MAXon parse failure, which makes invalid offsets become silent no-ops (and prints a huge offset in the journal). The C runner will typically parse such inputs as 0 viastrtoul, so malformed vectors can cause spurious C-vs-Rust mismatches. Failing fast here keeps the mutation-vector harness focused on FS conformance rather than argument parsing quirks.
fn parse_uint(s: &str) -> usize {
match s.strip_prefix("0x").or_else(|| s.strip_prefix("0X")) {
Some(hex) => usize::from_str_radix(hex, 16).unwrap_or(usize::MAX),
None => s.parse().unwrap_or(usize::MAX),
}
- Files reviewed: 20/20 changed files
- Comments generated: 0 new
- Review effort level: Lite
Closes #99.
Rust firmware works with the whole EEPROM, so the file operations no longer live in C alone.
jeefs_header::fsis a line-by-line port ofsrc/jeefs.cover a caller-owned&mut [u8]— allocation-free, available in bareno_std(thealloc-gatedimagemodule stays what it was).format,files(validating chain iterator),read_file,add_file,write_file,delete_file,header_check_consistency.0xFFFFlink as terminator,device.idfirst slot with the shifted chain relinked, headerless-image claim (never over a matching magic with an unknown version), and atomic failure — an error return leaves the buffer untouched.Resultinstead of negative codes, andFsError::FileExistsfor whatAddFilereports as zero bytes written.Mechanical conformance. New shared mutation vectors (
tests/cross-language/fs_vectors/*.ops): a scenario script that a C runner and a Rust runner both apply to the same starting image. ctestfs_mutation_c_matches_rsrequires identical operation journals and identical image bytes. Eight vectors cover the lifecycle, device.id insert-first, deletion from middle/front/tail, both overwrite paths, claiming garbage and erased media, validation and capacity failures, header and payload corruption, and the fs_version gates.The charter line reserving FS for C/C++ is amended accordingly, and the contract now binds any FS port to these vectors.
Test plan: 24 new integration tests (TDD, RED first), cargo test 24+20+12 green, bare
--no-default-featuresand--features allocbuilds clean, clippy --all-features clean, ctest 135/135 including the new conformance case, prek green.