Add IgmpSlice for zero-copy IGMP parsing#148
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThis PR adds zero-copy, lifetime-bound parsing for complete IGMP packets and IGMPv3 group records. Constructors validate lengths, accessors expose decoded fields and borrowed payload regions, iterators parse multiple records, and the new APIs are publicly re-exported. ChangesIGMP slice parsing
Estimated code review effort: 4 (Complex) | ~50 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
Author
|
Thank you for the work you did on the previous IGMP PR @JulianSchmid! I've updated my IGMP slice parsing to conform to the changes you've made. Please let me know if there's any changes you would like me to make here! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
etherparse/src/transport/igmp_slice.rs (1)
230-258: 💤 Low valueMinor: redundant slicing in test.
bytesis already[u8; 16], so&bytes[..16]is equivalent to&bytes. Not worth fixing on its own — flagging only in case you touch this test for other reasons.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@etherparse/src/transport/igmp_slice.rs` around lines 230 - 258, In the from_slice_v3_query test, remove the redundant slice expression when calling IgmpSlice::from_slice by passing the byte array directly (replace &bytes[..16] with &bytes) to simplify the test; update the call site in the from_slice_v3_query test that invokes IgmpSlice::from_slice and keep all other assertions and uses of bytes unchanged.etherparse/src/transport/igmp/report_group_record_v3_slice.rs (1)
148-152: 💤 Low valueDoc nit: clarify what
slice()returns vs. the constructor input.Worth noting in the doc that
slice()is exactlyrecord_lenbytes (the full record), not the original slice passed tofrom_slice. This contrasts withIgmpSlice::slice(), which exposes the full input including trailing payload — small clarification will help users avoid confusion.📝 Suggested doc tweak
- /// Returns the full slice of this group record. + /// Returns the bytes of this group record (header + sources + aux data). + /// + /// Note: this is exactly the validated record length, not any + /// trailing bytes from the original input passed to + /// [`ReportGroupRecordV3Slice::from_slice`] (those are returned + /// separately as the second element of the tuple). #[inline] pub fn slice(&self) -> &'a [u8] { self.slice }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@etherparse/src/transport/igmp/report_group_record_v3_slice.rs` around lines 148 - 152, Update the doc comment for the method slice() on ReportGroupRecordV3Slice to explicitly state that it returns exactly the record_len bytes of the group record (the full record slice stored in the struct) rather than the original input slice passed to from_slice; mention the contrast with IgmpSlice::slice() which exposes the entire input including trailing payload so callers understand slice() here is trimmed to the record length.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@etherparse/src/transport/igmp_slice.rs`:
- Around line 230-258: In the from_slice_v3_query test, remove the redundant
slice expression when calling IgmpSlice::from_slice by passing the byte array
directly (replace &bytes[..16] with &bytes) to simplify the test; update the
call site in the from_slice_v3_query test that invokes IgmpSlice::from_slice and
keep all other assertions and uses of bytes unchanged.
In `@etherparse/src/transport/igmp/report_group_record_v3_slice.rs`:
- Around line 148-152: Update the doc comment for the method slice() on
ReportGroupRecordV3Slice to explicitly state that it returns exactly the
record_len bytes of the group record (the full record slice stored in the
struct) rather than the original input slice passed to from_slice; mention the
contrast with IgmpSlice::slice() which exposes the entire input including
trailing payload so callers understand slice() here is trimmed to the record
length.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b14abde8-90f2-450f-a770-9f0f791ace9e
📒 Files selected for processing (4)
etherparse/src/transport/igmp/mod.rsetherparse/src/transport/igmp/report_group_record_v3_slice.rsetherparse/src/transport/igmp_slice.rsetherparse/src/transport/mod.rs
Adds zero-copy slice types for IGMP packets covering v1, v2, and v3: - IgmpSlice: wraps raw IGMP packet bytes with field accessors - ReportGroupRecordV3Slice: zero-copy access to IGMPv3 group records - ReportGroupRecordV3SliceIter: iterates group records in v3 reports
JulianSchmid
approved these changes
Jul 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds zero-copy slice types for IGMP packets covering v1, v2, and v3:
Summary by CodeRabbit