Replace ByteValued with zerocopy#374
Open
arctic-alpaca wants to merge 5 commits intorust-vmm:mainfrom
Open
Conversation
beed516 to
23caac5
Compare
ShadowCurse
reviewed
Apr 9, 2026
Collaborator
There was a problem hiding this comment.
Overall LGTM. Please add commit messages to simplify two unsafes with zerocopy and simplify two unsafes with zerocopy. Also maybe change commit titles to zerocopy: ..., but no hard push here.
@roypat this might be of interest to you (we still have an internal ticket with your name on it about this change)
P.S. the CI fails with coverage change https://buildkite.com/rust-vmm/vm-memory-ci/builds/1246#019d3ad3-8e00-442a-9be3-fb3511606b95/L96 so please add a commit with update for it.
CHANGELOG.md
Outdated
|
|
||
| ## Removed | ||
|
|
||
| - [...] Removed `ByteValued` trait and `endian` module in favor of the `zerocopy` crate. |
Collaborator
There was a problem hiding this comment.
You can fill the [...] now.
The zerocopy crate provides the same functionality. Signed-off-by: Julian Schindel <mail@arctic-alpaca.de>
The safety documentation for the `ByteValued` trait states that a type
`T` can only implement `ByteValued` if and only if it can be initialized
by reading its contents from a byte array. This doesn't cover the
`ByteValued::as{_mut}_slice` methods, which allow treating type `T` as
slice of bytes. From this, the `ByteValued` invariants are captured only
by requiring both, `zerocopy::FromBytes` and `zerocopy::IntoBytes`.
Additionally, `ByteValued::zeroed` allows constructing a `T` with all
bytes set to zero, which means `zerocopy::FromZeroes` is also required
to map `ByteValued` to `zerocopy` traits.
All `ByteValued` bounds are replaced by the existing bounds
`Copy + Send + Sync` and the new bounds
`FromBytes + IntoBytes + FromZeros`.
Signed-off-by: Julian Schindel <mail@arctic-alpaca.de>
Only creates a (mutable) slice from a pointer instead of creating a reference to a type `T`. Moves alignement and size checks to `zerocopy`. Signed-off-by: Julian Schindel <mail@arctic-alpaca.de>
23caac5 to
824425a
Compare
Removes mentions of `ByteValued` and updates the changelog. Signed-off-by: Julian Schindel <mail@arctic-alpaca.de>
Adapts expected coverage to code change from `ByteValued` removal. Signed-off-by: Julian Schindel <mail@arctic-alpaca.de>
824425a to
dc5f184
Compare
ShadowCurse
approved these changes
Apr 10, 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
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 of the PR
Implementing
ByteValuedis unsafe and requires care to not introduce undefined behaviour. Thezerocopycrate provides derivable traits that check at compiletime whether the struct in question satisfies the safety requirements instead.The safety documentation for the
ByteValuedtrait states that a typeTcan only implementByteValuedif and only if it can be initialized by reading its contents from a byte array. This doesn't cover theByteValued::as{_mut}_slicemethods, which allow treating typeTas slice of bytes. From this, theByteValuedinvariants are captured only by requiring both,zerocopy::FromBytesandzerocopy::IntoBytes. Additionally,ByteValued::zeroedallows constructing aTwith all bytes set to zero, which meanszerocopy::FromZeroesis also required to mapByteValuedtozerocopytraits.All
ByteValuedbounds are replaced by the existing boundsCopy + Send + Syncand the new boundsFromBytes + IntoBytes + FromZeros. Additionally, theendianmodule is removed in favour ofzerocopy's endian handling.Fixes #246
Requirements
Before submitting your PR, please make sure you addressed the following
requirements:
git commit -s), and the commit message has max 60 characters for thesummary and max 75 characters for each description line.
test.
Release" section of CHANGELOG.md (if no such section exists, please create one).
unsafecode is properly documented.