[M] Change in fuchsia/fuchsia[main]: [zx][rs] Align all VMO read calls to use FromBytes.
5 views
Skip to first unread message
'Adam Perry (Gerrit)' via owners-override
Sep 18, 2025, 5:02:03 PM (4 days ago) Sep 18
to Owners Override, James Sullivan
Attention needed from James Sullivan and Owners Override
Adam Perry has uploaded the change for review![]( "Open in Gerrit")
Adam Perry would like Owners Override to review this change.
Commit message
[zx][rs] Align all VMO read calls to use FromBytes.
u8 already satisfies this trait so this is strictly more general.
This change does require adding some turbofishes in places where
Rust's type inference can no longer unambiguously pick a type.
Leaves the Vmo::read() call still using raw bytes to avoid needing
to call drop_in_place when we don't know if the call will succeed.
Requires callers to use `T: Copy` to avoid footguns about missed
`Drop` calls. Seems like all of the current callers of higher level
methods like read_to_vec are fine with this bound. We can always
revisit if we find it difficult to use in practice.
Change-Id: Ide1351c197c3b28c72beb12b929d604369d5fc48
Change diff
Change information
Files:
- M sdk/rust/zx/src/vmo.rs
- M src/starnix/kernel/mm/memory.rs
- M src/starnix/modules/fastrpc/fastrpc.rs
- M src/storage/conformance/tests/file/get_backing_memory.rs
- M src/storage/fxfs/platform/src/fuchsia/fxblob/blob.rs
- M src/storage/fxfs/platform/src/fuchsia/fxblob/writer.rs
- M src/storage/fxfs/platform/src/fuchsia/paged_object_handle.rs
- M src/storage/gpt/component/src/gpt.rs
- M src/storage/lib/block_server/integration/src/lib.rs
- M src/storage/lib/vfs/rust/src/temp_clone.rs
- M src/sys/component_manager/src/bootfs.rs
- M src/sys/component_manager/src/framework/realm.rs
- M src/sys/pkg/lib/package-directory/src/lib.rs
- M src/tee/binder-proxy/src/convert.rs
Change size: M
Delta: 14 files changed, 68 insertions(+), 50 deletions(-)
Related details
Attention is currently required from:
- James Sullivan
- Owners Override
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
You received this message because you are subscribed to the Google Groups "owners-override" group.
To unsubscribe from this group and stop receiving emails from it, send an email to owners-overri...@fuchsia.dev.
To view this discussion visit https://groups.google.com/a/fuchsia.dev/d/msgid/owners-override/f6706875491a7ce084300c18b219da0eecc7fc48-HTML%40fuchsia-review.googlesource.com.
'James Robinson (Gerrit)' via owners-override
Sep 18, 2025, 5:28:18 PM (4 days ago) Sep 18
to Adam Perry, Owners Override, James Sullivan, David Koloski, GI Try Builder, CQ Bot
Attention needed from Adam Perry, James Sullivan and Owners Override
James Robinson added 2 comments![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 4 (Latest):
James Robinson . resolved
what's the motivation here? is this tied to a bug? is it tied to a Rust requirement? is it tied to bug-prone patterns?
File sdk/rust/zx/src/vmo.rs
Line 132, Patchset 4 (Latest):
/// Provides the thinnest wrapper possible over `zx_vmo_read`.James Robinson . unresolved
is this still true?
Related details
Attention is currently required from:
- Adam Perry
- James Sullivan
- Owners Override
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
--
You received this message because you are subscribed to the Google Groups "owners-override" group.
To unsubscribe from this group and stop receiving emails from it, send an email to owners-overri...@fuchsia.dev.
To view this discussion visit https://groups.google.com/a/fuchsia.dev/d/msgid/owners-override/9a7a2146dc0594109391acf003487f57eef04fba-EmailReviewComments-HTML%40fuchsia-review.googlesource.com.You received this message because you are subscribed to the Google Groups "owners-override" group.
To unsubscribe from this group and stop receiving emails from it, send an email to owners-overri...@fuchsia.dev.
'James Sullivan (Gerrit)' via owners-override
Sep 18, 2025, 6:12:11 PM (4 days ago) Sep 18
to Adam Perry, James Robinson, Owners Override, David Koloski, GI Try Builder, CQ Bot
Attention needed from Adam Perry and Owners Override
James Sullivan added 1 comment![]( "Open in Gerrit")
Patchset-level comments
James Robinson . unresolved
what's the motivation here? is this tied to a bug? is it tied to a Rust requirement? is it tied to bug-prone patterns?
James Sullivan
https://fxrev.dev/1361058 is a bit of background.
We already have `Vmo::read_to_array`, but it requires size to be a constant. I was going to add `read_to_boxed_slice`, and Adam suggested we do this instead.
It's nice to encapsulate unsafe code in a well-vetted library like this, which is one argument for this change. Otherwise, clients who wish to do a zero-copy read of an array of structs from a VMO must do their own unsafe.
Related details
Attention is currently required from:
- Adam Perry
- Owners Override
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
--
You received this message because you are subscribed to the Google Groups "owners-override" group.
To unsubscribe from this group and stop receiving emails from it, send an email to owners-overri...@fuchsia.dev.
To view this discussion visit https://groups.google.com/a/fuchsia.dev/d/msgid/owners-override/7eedb7287d0654e4c8154cd6bb7ac9867d3010db-EmailReviewComments-HTML%40fuchsia-review.googlesource.com.You received this message because you are subscribed to the Google Groups "owners-override" group.
To unsubscribe from this group and stop receiving emails from it, send an email to owners-overri...@fuchsia.dev.
'James Robinson (Gerrit)' via owners-override
Sep 18, 2025, 7:14:52 PM (4 days ago) Sep 18
to Adam Perry, Owners Override, James Sullivan, David Koloski, GI Try Builder, CQ Bot
Attention needed from Adam Perry and Owners Override
James Robinson added 1 comment![]( "Open in Gerrit")
Patchset-level comments
James Robinson . resolved
James (S), you're an OWNER for a bunch of this code - could you give that a regular code review?
You received this message because you are subscribed to the Google Groups "owners-override" group.
To unsubscribe from this group and stop receiving emails from it, send an email to owners-overri...@fuchsia.dev.
To view this discussion visit https://groups.google.com/a/fuchsia.dev/d/msgid/owners-override/7fcc48325603c83fdb93a227a530c2d75b118e40-EmailReviewComments-HTML%40fuchsia-review.googlesource.com.To unsubscribe from this group and stop receiving emails from it, send an email to owners-overri...@fuchsia.dev.
'James Sullivan (Gerrit)' via owners-override
Sep 18, 2025, 8:12:54 PM (4 days ago) Sep 18
to Adam Perry, James Robinson, Owners Override, David Koloski, GI Try Builder, CQ Bot
Attention needed from Adam Perry and Owners Override
James Sullivan voted and added 1 comment![]( "Open in Gerrit")
Votes added by James Sullivan
| Code-Review | +2 |
1 comment
Patchset-level comments
James Sullivan . resolved
LGTM for //src/storage
Related details
Attention is currently required from:
- Adam Perry
- Owners Override
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
--
You received this message because you are subscribed to the Google Groups "owners-override" group.
To unsubscribe from this group and stop receiving emails from it, send an email to owners-overri...@fuchsia.dev.
To view this discussion visit https://groups.google.com/a/fuchsia.dev/d/msgid/owners-override/b88da347a0bedda54a120f712c0c3e8c7b39cd96-EmailReviewComments-HTML%40fuchsia-review.googlesource.com.You received this message because you are subscribed to the Google Groups "owners-override" group.
To unsubscribe from this group and stop receiving emails from it, send an email to owners-overri...@fuchsia.dev.
'Adam Perry (Gerrit)' via owners-override
Sep 18, 2025, 8:19:51 PM (4 days ago) Sep 18
to James Sullivan, James Robinson, Owners Override, David Koloski, GI Try Builder, CQ Bot
Attention needed from James Robinson and Owners Override
Adam Perry added 1 comment![]( "Open in Gerrit")
File sdk/rust/zx/src/vmo.rs
Line 132, Patchset 4 (Latest):
/// Provides the thinnest wrapper possible over `zx_vmo_read`.James Robinson . unresolved
is this still true?
Attention is currently required from:
- James Robinson
- Owners Override
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
--
You received this message because you are subscribed to the Google Groups "owners-override" group.
To unsubscribe from this group and stop receiving emails from it, send an email to owners-overri...@fuchsia.dev.
To view this discussion visit https://groups.google.com/a/fuchsia.dev/d/msgid/owners-override/cf6988dc87407230f5389cceef6517d0f9055154-EmailReviewComments-HTML%40fuchsia-review.googlesource.com.You received this message because you are subscribed to the Google Groups "owners-override" group.
To unsubscribe from this group and stop receiving emails from it, send an email to owners-overri...@fuchsia.dev.
'Adam Perry (Gerrit)' via owners-override
Sep 19, 2025, 3:46:10 PM (3 days ago) Sep 19
to James Sullivan, James Robinson, Owners Override, David Koloski, GI Try Builder, CQ Bot
Attention needed from James Robinson and Owners Override
Adam Perry voted and added 1 comment![]( "Open in Gerrit")
Votes added by Adam Perry
| Commit-Queue | +1 |
| Fuchsia-Auto-Submit | +1 |
1 comment
File sdk/rust/zx/src/vmo.rs
Line 132, Patchset 4:
/// Provides the thinnest wrapper possible over `zx_vmo_read`.James Robinson . resolved
Adam Perryis this still true?
It's very close to true but I'll rephrase.
Adam Perry
Done
Related details
Attention is currently required from:
- James Robinson
- Owners Override
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
--
You received this message because you are subscribed to the Google Groups "owners-override" group.
To unsubscribe from this group and stop receiving emails from it, send an email to owners-overri...@fuchsia.dev.
To view this discussion visit https://groups.google.com/a/fuchsia.dev/d/msgid/owners-override/1614f26e858123f301c0b549dfb9873757f82879-EmailReviewComments-HTML%40fuchsia-review.googlesource.com.You received this message because you are subscribed to the Google Groups "owners-override" group.
To unsubscribe from this group and stop receiving emails from it, send an email to owners-overri...@fuchsia.dev.
'James Robinson (Gerrit)' via owners-override
Sep 19, 2025, 4:16:36 PM (3 days ago) Sep 19
to Adam Perry, James Sullivan, Owners Override, David Koloski, GI Try Builder, CQ Bot
Attention needed from Adam Perry and Owners Override
James Robinson voted and added 1 comment![]( "Open in Gerrit")
Votes added by James Robinson
| Owners-Override | +1 |
James Sullivanwhat's the motivation here? is this tied to a bug? is it tied to a Rust requirement? is it tied to bug-prone patterns?
https://fxrev.dev/1361058 is a bit of background.
We already have `Vmo::read_to_array`, but it requires size to be a constant. I was going to add `read_to_boxed_slice`, and Adam suggested we do this instead.
It's nice to encapsulate unsafe code in a well-vetted library like this, which is one argument for this change. Otherwise, clients who wish to do a zero-copy read of an array of structs from a VMO must do their own unsafe.
Attention is currently required from:
- Adam Perry
- Owners Override
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
--
You received this message because you are subscribed to the Google Groups "owners-override" group.
To unsubscribe from this group and stop receiving emails from it, send an email to owners-overri...@fuchsia.dev.
To view this discussion visit https://groups.google.com/a/fuchsia.dev/d/msgid/owners-override/a136088e9db6c30ff23ca4d49dc896000872b085-EmailReviewComments-HTML%40fuchsia-review.googlesource.com.You received this message because you are subscribed to the Google Groups "owners-override" group.
To unsubscribe from this group and stop receiving emails from it, send an email to owners-overri...@fuchsia.dev.
'CQ Bot (Gerrit)' via owners-override
Sep 19, 2025, 6:44:41 PM (3 days ago) Sep 19
to Adam Perry, James Robinson, James Sullivan, Owners Override, David Koloski, GI Try Builder
CQ Bot submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
4 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: sdk/rust/zx/src/vmo.rs
Insertions: 1, Deletions: 1.
The diff is too large to show. Please review the diff.
```
Change information
Commit message:
[zx][rs] Align all VMO read calls to use FromBytes.
u8 already satisfies this trait so this is strictly more general.
This change does require adding some turbofishes in places where
Rust's type inference can no longer unambiguously pick a type.
Leaves the Vmo::read() call still using raw bytes to avoid needing
to call drop_in_place when we don't know if the call will succeed.
Requires callers to use `T: Copy` to avoid footguns about missed
`Drop` calls. Seems like all of the current callers of higher level
methods like read_to_vec are fine with this bound. We can always
revisit if we find it difficult to use in practice.
Change-Id: Ide1351c197c3b28c72beb12b929d604369d5fc48
Commit-Queue: Adam Perry <adam...@google.com>
Reviewed-by: David Koloski <dkol...@google.com>
Owners-Override: James Robinson <jam...@google.com>
Reviewed-by: James Sullivan <jfsu...@google.com>
Fuchsia-Auto-Submit: Adam Perry <adam...@google.com>
Files:
- M sdk/rust/zx/src/vmo.rs
- M src/starnix/kernel/mm/memory.rs
- M src/starnix/modules/fastrpc/fastrpc.rs
- M src/storage/conformance/tests/file/get_backing_memory.rs
- M src/storage/fxfs/platform/src/fuchsia/fxblob/blob.rs
- M src/storage/fxfs/platform/src/fuchsia/fxblob/writer.rs
- M src/storage/fxfs/platform/src/fuchsia/paged_object_handle.rs
- M src/storage/gpt/component/src/gpt.rs
- M src/storage/lib/block_server/integration/src/lib.rs
- M src/storage/lib/vfs/rust/src/temp_clone.rs
- M src/sys/component_manager/src/bootfs.rs
- M src/sys/component_manager/src/framework/realm.rs
- M src/sys/pkg/lib/package-directory/src/lib.rs
- M src/tee/binder-proxy/src/convert.rs
Change size: M
Delta: 14 files changed, 68 insertions(+), 50 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +2 by David Koloski, +2 by James Sullivan
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
--
You received this message because you are subscribed to the Google Groups "owners-override" group.
To unsubscribe from this group and stop receiving emails from it, send an email to owners-overri...@fuchsia.dev.
To view this discussion visit https://groups.google.com/a/fuchsia.dev/d/msgid/owners-override/a53350fd093be3fd51ae467cb33a36d91adca60c-HTML%40fuchsia-review.googlesource.com.You received this message because you are subscribed to the Google Groups "owners-override" group.
To unsubscribe from this group and stop receiving emails from it, send an email to owners-overri...@fuchsia.dev.
'GI Roller (Gerrit)' via owners-override
Sep 19, 2025, 6:56:28 PM (3 days ago) Sep 19
to Adam Perry, CQ Bot, James Robinson, James Sullivan, Owners Override, David Koloski, GI Try Builder
Message from GI Roller![]( "Open in Gerrit")
Change has been successfully rolled: https://turquoise-internal-review.googlesource.com/1047794
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
--
You received this message because you are subscribed to the Google Groups "owners-override" group.
To unsubscribe from this group and stop receiving emails from it, send an email to owners-overri...@fuchsia.dev.
To view this discussion visit https://groups.google.com/a/fuchsia.dev/d/msgid/owners-override/d657d900f3df10fd7e80020c736d778e703cc037-EmailReviewComments-HTML%40fuchsia-review.googlesource.com.You received this message because you are subscribed to the Google Groups "owners-override" group.
To unsubscribe from this group and stop receiving emails from it, send an email to owners-overri...@fuchsia.dev.
Reply all
Reply to author
Forward
0 new messages