This repository was archived by the owner on Jul 13, 2026. It is now read-only.
region: document GuestMemoryRegion::len() non-zero invariant#382
Closed
Aakash456 wants to merge 1 commit into
Closed
region: document GuestMemoryRegion::len() non-zero invariant#382Aakash456 wants to merge 1 commit into
Aakash456 wants to merge 1 commit into
Conversation
Custom GuestMemoryRegion implementations must return a non-zero length from len(), since last_addr() computes self.len() - 1 and would otherwise underflow. This was previously enforced implicitly for built-in mmap-backed regions but undocumented for the public trait. Also adds a debug_assert in last_addr() to catch violations early, and corrects the now-inaccurate safety comment on the unchecked_add call. Fixes #380 Signed-off-by: Aakash Chaudhary <aakashchoudhary66@gmail.com>
Aakash456
requested review from
ShadowCurse,
XanClic,
alexandruag,
bonzini,
jiangliu and
roypat
as code owners
June 18, 2026 06:40
andreeaflorescu
left a comment
andreeaflorescu
left a comment
Member
There was a problem hiding this comment.
Thanks for the PR. I just left a small comment on the documentation.
We now moved this crate to the rust-vmm monorepo. To be able to merge this, can you please re-create your PR there: https://github.com/rust-vmm/rust-vmm
|
|
||
| /// Returns the maximum (inclusive) address managed by the region. | ||
| /// | ||
| /// # Panics (debug builds only) |
Member
There was a problem hiding this comment.
We don't typically document debug panics, I don't think this is necessary.
Member
|
Closing this as the repo is now read only. Please re-open in rust-vmm/rust-vmm. |
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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
Documents that
GuestMemoryRegion::len()must always return a non-zero value, sincelast_addr()'s default implementation computesself.len() - 1and would otherwise underflow.This invariant was previously enforced implicitly for built-in mmap-backed regions but undocumented for the public
GuestMemoryRegiontrait, which custom backends also implement.Changes
# Invariantsection to the doc comment onGuestMemoryRegion::len()debug_assert!inlast_addr()to fail fast in debug builds if a custom implementation violates the invariantunchecked_addcall, which previously implied the safety guarantee holds universally — it only held for the built-in mmap region, not for arbitrary custom implementationsFixes rust-vmm/rust-vmm#50
Happy to adjust scope if you'd prefer this as doc-only without the
debug_assert, or if a hardassert!is preferred overdebug_assert!given this is a safety-adjacent invariant.