refactor(link_layer/pktap): drop no-op byte-order conversions#292
Merged
domcyrus merged 1 commit intoMay 20, 2026
Merged
Conversation
Five `u32::from_le_bytes(x.to_le_bytes())` lines after the `read_unaligned` were identity transforms — `to_le_bytes` produces little-endian bytes and `from_le_bytes` interprets them as a native u32, so the round-trip is a no-op on every supported platform. PKTAP is macOS-only and macOS is little-endian, so the struct read already yields the correct field values; the conversions were never doing work. Drop them and update the SAFETY comment to spell out the endianness assumption so a future reader doesn't reintroduce the dead code.
Owner
|
@obchain Thanks a lot for your PR. This LGTM! |
This was referenced May 21, 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.
Summary
Drops five no-op byte-order conversions in
PktapHeader::from_bytes(src/network/link_layer/pktap.rs:60-64) and expands the SAFETY comment to spell out the little-endian assumption so the round-trip isn't reintroduced.Closes #291.
Why
Each of the five lines was
header.field = u32::from_le_bytes(header.field.to_le_bytes())— an identity transform.to_le_bytesserialises the nativeu32to little-endian;from_le_bytesreads those bytes back as au32. Same byte order in, same byte order out, on every platform Rust supports. The comment claimed "Convert from network byte order if needed", but PKTAP is macOS-only, macOS is exclusively little-endian (x86_64 + arm64), and the wire format is little-endian — there is no big-endian path to convert from. Theread_unalignedalready yields the decoded values directly.The expanded SAFETY comment now records the LE assumption next to the unsafe block so a future patcher adding another
u32field doesn't reintroduce the deadfrom_le_bytes(x.to_le_bytes())pattern.Test plan
cargo clippy --all-targets --all-features -- -D warnings— cleancargo fmt --check— cleancargo test --lib— 361 passed, 0 failedtest_pktap_header_size,test_invalid_pktap_datatests in the same file still cover the parse path