Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use u32 to describe vsock related buffer sizes #4627

Open
roypat opened this issue May 28, 2024 · 1 comment · May be fixed by #4637
Open

Use u32 to describe vsock related buffer sizes #4627

roypat opened this issue May 28, 2024 · 1 comment · May be fixed by #4637
Assignees
Labels
Good first issue Indicates a good issue for first-time contributors Priority: Low Indicates that an issue or pull request should be resolved behind issues or pull requests labelled ` Status: Parked Indicates that an issues or pull request will be revisited later

Comments

@roypat
Copy link
Contributor

roypat commented May 28, 2024

In #4556, we updated our generic virtio buffer handling code to describe buffer sizes using u32 instead of usize, since the virtio specification states that lengths of virtio-buffers fit into u32 (and header fields in the virtio protocol describing lengths are typed to be 32 bit integers).

From that work, a small follow up is possible in the vsock module, where we can now also describe lengths of vsock packages using u32 instead of usize. This will allow us to get rid of a few more explicit casts. Particularly, we can do the following changes to function signatures:

  • VsockConnection::peer_avail_credit can return u32
  • VsockPacket::buf_size can return u32
  • VsockPacket::read_at_offset_from should be (&self, &mut T, u32, u32) -> Result<u32, VsockError> (e..g. the conversion to usize should only happen right around the write_volatile_at call
  • ditto for VsockPacket::write_from_offset_to
    Maybe we can even get away with changing the arguments IoVecBuffer[Mut]::{read,write}_volatile_at to u32s
@roypat roypat added Good first issue Indicates a good issue for first-time contributors Priority: Low Indicates that an issue or pull request should be resolved behind issues or pull requests labelled ` labels May 28, 2024
@brandonpike
Copy link

Can pick this one up as well as a followup to #4556.

@brandonpike brandonpike linked a pull request Jun 10, 2024 that will close this issue
9 tasks
@zulinx86 zulinx86 added the Status: Parked Indicates that an issues or pull request will be revisited later label Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good first issue Indicates a good issue for first-time contributors Priority: Low Indicates that an issue or pull request should be resolved behind issues or pull requests labelled ` Status: Parked Indicates that an issues or pull request will be revisited later
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants