Skip to content

Commit

Permalink
use MaybeUninit as requested in #41 (comment)
Browse files Browse the repository at this point in the history
  • Loading branch information
problame committed Feb 8, 2024
1 parent f583d4a commit 04f5e7a
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 15 deletions.
4 changes: 2 additions & 2 deletions tokio-epoll-uring/src/ops/statx.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::system::submission::op_fut::Op;
use crate::util::submitting_box::SubmittingBox;
use std::os::fd::AsRawFd;
use std::{mem::MaybeUninit, os::fd::AsRawFd};
use uring_common::libc;
pub use uring_common::libc::statx;
use uring_common::{
Expand Down Expand Up @@ -28,7 +28,7 @@ where
{
ByFileDescriptor {
file: F,
statxbuf: Box<uring_common::libc::statx>,
statxbuf: Box<MaybeUninit<uring_common::libc::statx>>,
},
}

Expand Down
25 changes: 17 additions & 8 deletions tokio-epoll-uring/src/system/lifecycle/handle.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Owned handle to an explicitly [`System::launch`](crate::System::launch)ed system.
use futures::FutureExt;
use std::{os::fd::OwnedFd, path::Path, task::ready};
use std::{mem::MaybeUninit, os::fd::OwnedFd, path::Path, task::ready};
use uring_common::{buf::BoundedBufMut, io_fd::IoFd};

use crate::{
Expand Down Expand Up @@ -182,12 +182,8 @@ impl crate::SystemHandle {
crate::system::submission::op_fut::Error<std::io::Error>,
>,
) {
// TODO: avoid the allocation, or optimize using a slab cacke?
let buf: Box<uring_common::libc::statx> = Box::new(
// TODO replace with Box<MaybeUninit>, https://github.com/rust-lang/rust/issues/63291
// SAFETY: we only use the memory if the fstat succeeds, should be using MaybeUninit here.
unsafe { std::mem::zeroed() },
);
// TODO: avoid the allocation, or optimize using a slab cache?
let buf: Box<MaybeUninit<uring_common::libc::statx>> = Box::new(MaybeUninit::uninit());
let op = statx::op(statx::Resources::ByFileDescriptor {
file,
statxbuf: buf,
Expand All @@ -196,7 +192,20 @@ impl crate::SystemHandle {
let (resources, result) = execute_op(op, inner.submit_side.weak(), None).await;
let crate::ops::statx::Resources::ByFileDescriptor { file, statxbuf } = resources;
match result {
Ok(()) => (file, Ok(statxbuf)),
Ok(()) => (
file,
Ok({
// TODO: replace this with Box::assume_init once it stabilizes
// SAFETY: if the kernel tells us the call went ok, we know the statx has been initialized
unsafe {
// It seems weird that current rust 1.75 Box::assume_init doesn't do the assert_inhabited
// that the regular MaybeUninit::assume_init does. Out of precaution, do that here.
statxbuf.assume_init_ref();
let raw = Box::into_raw(statxbuf);
Box::from_raw(raw as *mut uring_common::libc::statx)
}
}),
),
Err(e) => (file, Err(e)),
}
}
Expand Down
12 changes: 7 additions & 5 deletions tokio-epoll-uring/src/util/submitting_box.rs
Original file line number Diff line number Diff line change
@@ -1,20 +1,22 @@
//! See [`SubmittingBox`].
use std::mem::MaybeUninit;

/// A wrapper around [`Box`] with an API that forces users to spell out
/// ownerhsip transitions of the memory between kernel and userspace.
pub enum SubmittingBox<T>
where
T: 'static,
{
NotSubmitting { inner: Box<T> },
Submitting(*mut T),
NotSubmitting { inner: Box<MaybeUninit<T>> },
Submitting(*mut MaybeUninit<T>),
Undefined,
}

unsafe impl<T> Send for SubmittingBox<T> where T: Send {}

impl<T> SubmittingBox<T> {
pub(crate) fn new(inner: Box<T>) -> Self {
pub(crate) fn new(inner: Box<MaybeUninit<T>>) -> Self {
Self::NotSubmitting { inner }
}

Expand All @@ -29,7 +31,7 @@ impl<T> SubmittingBox<T> {
SubmittingBox::NotSubmitting { inner } => {
let leaked = Box::into_raw(inner);
*self = Self::Submitting(leaked);
leaked
leaked as *mut T
}
SubmittingBox::Submitting(_) => {
panic!("must not call this function more than once without ownership_back_in_userspace() inbetween")
Expand All @@ -50,7 +52,7 @@ impl<T> SubmittingBox<T> {
///
/// Callers must ensure that userspace, and in particular, _the caller_ has again exclusive ownership
/// over the memory.
pub(crate) unsafe fn ownership_back_in_userspace(mut self) -> Box<T> {
pub(crate) unsafe fn ownership_back_in_userspace(mut self) -> Box<MaybeUninit<T>> {
match std::mem::replace(&mut self, SubmittingBox::Undefined) {
SubmittingBox::NotSubmitting { .. } => {
panic!("must not call this function without prior call to start_submitting()")
Expand Down

0 comments on commit 04f5e7a

Please sign in to comment.