trim alloc#9
Conversation
WalkthroughThis PR refactors the LZMA allocator infrastructure to support custom allocators. It introduces a new Changes
Possibly Related PRs
Poem
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~75 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@xz-core/src/raw_alloc.rs`:
- Around line 180-209: Add unit tests that exercise the typed allocation/free
helpers by calling internal_alloc_object and internal_alloc_array (with a
repr(align) test type), asserting the returned pointers are non-null and
properly aligned, then freeing them with the corresponding
internal_free_object/internal_free_array functions to ensure the direct
alloc/dealloc path is exercised; locate these helpers (internal_alloc_object,
internal_alloc_array, internal_free_object, internal_free_array) in raw_alloc.rs
and mirror the existing raw byte tests' structure (unsafe block, size/alignment
checks, then call the matching free) so mismatched free/alloc paths are caught.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f421664f-6285-41be-b360-15b5f27774b3
📒 Files selected for processing (30)
perf-probe/Cargo.tomlperf-probe/src/main.rsxz-core/src/alloc.rsxz-core/src/alloc/custom.rsxz-core/src/alloc/rust.rsxz-core/src/alloc/rust_only.rsxz-core/src/common/alone_decoder.rsxz-core/src/common/alone_encoder.rsxz-core/src/common/auto_decoder.rsxz-core/src/common/block_decoder.rsxz-core/src/common/block_encoder.rsxz-core/src/common/file_info.rsxz-core/src/common/index_decoder.rsxz-core/src/common/index_encoder.rsxz-core/src/common/lzip_decoder.rsxz-core/src/common/microlzma_decoder.rsxz-core/src/common/microlzma_encoder.rsxz-core/src/common/outqueue.rsxz-core/src/common/stream_decoder_mt.rsxz-core/src/common/stream_encoder_mt.rsxz-core/src/delta/delta_common.rsxz-core/src/lib.rsxz-core/src/lz/lz_decoder.rsxz-core/src/lz/lz_encoder.rsxz-core/src/lzma/lzma2_decoder.rsxz-core/src/lzma/lzma2_encoder.rsxz-core/src/lzma/lzma_decoder.rsxz-core/src/lzma/lzma_encoder.rsxz-core/src/raw_alloc.rsxz-core/src/simple/simple_coder.rs
💤 Files with no reviewable changes (1)
- xz-core/src/alloc/rust_only.rs
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
|
|
||
| #[test] | ||
| fn raw_allocator_round_trip() { | ||
| unsafe { | ||
| let ptr = alloc_bytes(32); | ||
| assert!(!ptr.is_null()); | ||
| free_ptr(ptr); | ||
| } | ||
| } | ||
|
|
||
| #[test] | ||
| fn raw_allocation_respects_alignment() { | ||
| #[repr(align(32))] | ||
| struct Align32([u8; 32]); | ||
|
|
||
| unsafe { | ||
| let ptr = alloc_impl( | ||
| core::mem::size_of::<Align32>(), | ||
| core::mem::align_of::<Align32>(), | ||
| false, | ||
| ); | ||
| assert!(!ptr.is_null()); | ||
| assert_eq!((ptr as usize) % core::mem::align_of::<Align32>(), 0); | ||
| free_impl(ptr); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add coverage for the typed alloc/free helpers.
These tests only exercise the header-backed byte path. The new internal_alloc_object/internal_alloc_array + internal_free* path uses direct alloc/dealloc, so a small round-trip test there would catch free-family mismatches before they turn into UB.
Test sketch
#[test]
fn raw_allocation_respects_alignment() {
#[repr(align(32))]
struct Align32([u8; 32]);
unsafe {
let ptr = alloc_impl(
core::mem::size_of::<Align32>(),
core::mem::align_of::<Align32>(),
false,
);
assert!(!ptr.is_null());
assert_eq!((ptr as usize) % core::mem::align_of::<Align32>(), 0);
free_impl(ptr);
}
}
+
+#[test]
+fn typed_allocator_round_trip() {
+ unsafe {
+ let object = internal_alloc_object::<u64>(core::ptr::null());
+ assert!(!object.is_null());
+ internal_free(object, core::ptr::null());
+
+ let array = internal_alloc_array::<u32>(4, core::ptr::null());
+ assert!(!array.is_null());
+ internal_free_array(array, 4, core::ptr::null());
+ }
+}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@xz-core/src/raw_alloc.rs` around lines 180 - 209, Add unit tests that
exercise the typed allocation/free helpers by calling internal_alloc_object and
internal_alloc_array (with a repr(align) test type), asserting the returned
pointers are non-null and properly aligned, then freeing them with the
corresponding internal_free_object/internal_free_array functions to ensure the
direct alloc/dealloc path is exercised; locate these helpers
(internal_alloc_object, internal_alloc_array, internal_free_object,
internal_free_array) in raw_alloc.rs and mirror the existing raw byte tests'
structure (unsafe block, size/alignment checks, then call the matching free) so
mismatched free/alloc paths are caught.
Summary by CodeRabbit
New Features
--allocatorcommand-line option.xz-core-custom-allocatorfeature flag for extended allocator support.Chores