fix(prmi-sys): accept canonical NULL+0 arenas in backward-spectrum batch FFI - #51
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughTightens null-pointer validation in the backward batch FFI shim to accept ChangesNULL+0 arena safety for backward batch
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…tch FFI The `prmi-sys` coding guideline requires every C ABI entrypoint to handle the canonical empty-slice case `NULL + len 0` (a C caller's conventional empty buffer) without `slice::from_raw_parts(NULL, 0)`, which is UB — the empty slice must be built directly. The backward-spectrum batch entrypoints did not: they rejected a NULL arena with a hard `-1` whenever `ntasks > 0`, even when the arena's declared length was 0. Split the null gate so `tasks` / `out_nsteps` (length `ntasks`) stay required while `reads_arena` / `steps_arena` are rejected only when their declared length is non-zero, and branch the per-task read/step slices to `&[]` / `&mut []` for zero-length windows (`write_bwd_task_steps` returns early on empty steps). A NULL arena with a non-zero declared length is still a hard `-1`. Also give `prmi_backward_spectrum_batch_lockstep` an explicit, self-contained return-code table (it previously deferred to the serial entrypoint), and update the serial entrypoint's `-1` description to document the NULL+0 acceptance. Covered by a new backward NULL+0 FFI regression test over both the serial and lockstep entrypoints, including the still-rejected NULL-with-nonzero-length case.
243f1c4 to
e678ad2
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
What
The
prmi-syscoding guideline requires everyextern "C"entrypoint to handle the canonical empty-slice caseNULL + len 0— a C caller's conventional way to pass an empty buffer — without callingslice::from_raw_parts(NULL, 0), which is UB. The empty slice must be built directly.The backward-spectrum batch entrypoints (
prmi_backward_spectrum_batch,prmi_backward_spectrum_batch_lockstep) did not honor this: they rejected a NULL arena with a hard-1wheneverntasks > 0, even when the arena's declared length was0.Changes
backward_spectrum_batch_impl:tasks/out_nsteps(lengthntasks) stay required, whilereads_arena/steps_arenaare rejected only when their declared length is non-zero.&[]/&mut []for zero-length read/step windows, so aNULL + 0arena is never passed tofrom_raw_parts.write_bwd_task_stepsreturns early on empty steps.-1.prmi_backward_spectrum_batch_lockstepan explicit, self-contained return-code table (it previously deferred to the serial entrypoint), and updated the serial entrypoint's-1description to document the NULL+0 acceptance.Testing
New
backward_spectrum_batch_accepts_null_plus_zero_arenasFFI regression test exercises both the serial and lockstep entrypoints withntasks > 0, all-empty tasks, andNULL + 0arenas (assertsrc == 0,out_nsteps == [0, 0], no NULL deref), plus the still-rejected NULL-with-nonzero-length case.clippyclean; fullprmi-syssuite passes.Relationship to #50
This is the backward-batch sibling of the forward-batch
NULL + 0hardening in #50. The two paths are independent code, so this is a standalone PR offmainrather than part of #50.Summary by CodeRabbit
Bug Fixes
Tests