Accessing MaybeUninit

help
1

I'm working on an ffi crate and need some advice in one key area.
My types usually look like this:

pub struct Wrapper {
    inner: Pin<Box<MaybeUninit<sys::c_struct>>>,
    init: bool,
}

Pin is used because it's an audio library and moves can lead to seg faults.
MaybeUninit is used because of how I initialize the c struct. It usually follows the same pattern:

pub fn new() -> Result<Wrapper> {
    let mut wrapper = Wrapper::new_uninit();
    let res = unsafe { sys::c_initializer_function(wrapper.maybe_uninit_mut_ptr())} // below
    // Check res for errors
    wrapper.set_init(); // set wrapper.init = true
    Ok(wrapper)
}

fn new_uninit() -> Wrapper {
    let mut inner = Box::pin(MaybeUninit::<sys::c_struct>::uninit());
    Wrapper { 
        inner,
        init: false,
    }
}

Now, I need 3 helpers to access the inner pointer.

Used to pass the unitialized ptr to c_initializer_function
fn maybe_uninit_mut_ptr(&mut self) -> *mut sys::c_struct {
    self.inner.as_mut_ptr()
}

And 2 other got getting a *const and a *mut after it's initialized.

fn assume_init_ptr(&self) -> *const sys::c_struct {
    debug_assert!(self.init, "Wrapper used before initialization.");
    unsafe { &self.inner.assume_init() as *const _ }
}

fn assume_init_mut_ptr(&mut self) -> *mut sys::c_struct {
    debug_assert!(self.init, "Wrapper used before initialization.");
    unsafe { self.inner.assume_init_mut() }
}

The main concern I have is with the last 2, if my helpers are sound. I am unsure of the difference between:
unsafe { &self.inner.assume_init() as *const _}
and simply and simply
self.inner.as_ptr()

Or between
unsafe { self.inner.assume_init_mut() }
and
self.inner.as_mut_ptr()

What are correct the ways I should access the pointers here?

2

you may want to check out moveit, which handles the boilerplates of Pin and MaybeUninit, so you can emulate the C++ constructor semantic in safe code:

btw, you combined MaybeUninit<T> and a boolean flag init in your example, which can be replaced by Option<T> without any unsafe. you also get the bonus niche optimization if your type supports it, which is not possible with MaybeUninit and bool.

3

Your current definition is not good. The type inside your pinned box implements Unpin, so the Pin has no effect. For good measure, you probably want to use UnsafeCell too. This gives you:

pub struct Wrapper {
    inner: Pin<Box<CStruct>>,
    init: bool,
}

struct CStruct {
    inner: UnsafeCell<MaybeUninit<sys::c_struct>>,
    _not_unpin: PhantomPinned,
}

the other option, which is equally legal, is to use a raw pointer:

pub struct Wrapper {
    inner: *mut sys::c_struct, // or NonNull<sys::c_struct>
    init: bool,
}

impl Drop for Wrapper {
    fn drop(&mut self) {
        // clean up the c struct if required
        drop(unsafe { Box::from_raw(self.inner) });
    }
}
6 Likes
4

Thank you! I do like the simplicity of inner: NonNull<sys::c_struct>. If needed I can wrap that in an Option, but for now at least, it doesn't seem necessary with how it gets initialized.

5

But OP needs to pass an out-pointer to sys::c_initializer_function. You cannot change in-place variant of an enum.

6

You can not do that with most enums. But that's very explicitly permitted for Option<NonNull<…>>. Precisely because it's so useful for FFI. Of course that's only true when you pointer is for Sized type, but that's Ok for topicstarter needs.

7

What I intended is:

let alloc = Box::new_uninit();
let ptr = Box::into_raw(alloc);
unsafe { sys::c_initializer_function(ptr) };
Self {
    inner: ptr,
}
1 Like
8

That's kind of what I went with

    let mut alloc: Box<MaybeUninit<sys::c_struct>> = Box::new_uninit();
    unsafe { sys::c_initializer_function(mem.as_mut_ptr()) };
    let leaked: *mut sys::c_struct = Box::into_raw(alloc).cast::<sys::c_stryct>();
    Ok(Wrapper {
        inner: unsafe { NonNull::new_unchecked(leaked) },
    })

With then dropping is as drop(unsafe { Box::from_raw(self.inner.as_ptr()) });

1 Like
9

i would recommend

let leaked = NonNull::from_mut(Box::leak(alloc)).cast::<sys::c_struct>()

to avoid the unnecesary new_unchecked. the best would be Box::into_non_null ofc, but that is not stable

1 Like
10

This is not good, although it does allow you to avoid on tiny bit of unsafe, it makes it impossible to correctly (soundly) deallocate that box.
But converting it to a reference you've lost the provenance of the original allocation. This means it's ub to deallocate using this pointer.

Box::into_non_null can't come soon enough

See these two related issues:

11

@RustyYato I had the same take until I was corrected. Yes, provenance matters. But the provenance of a &mut T is enough to deallocate. Apparently.

And stdlib does suggest people use leak + NonNull::from to do exactly this.

12

After some tweaks:

    let mut alloc: Box<MaybeUninit<sys::c_struct>> = Box::new_uninit();
    unsafe { sys::c_initializer_function(alloc.as_mut_ptr()) };
    let alloc: Box<sys::c_struct> = unsafe { alloc.assume_init() };
    let inner = NonNull::new(Box::into_raw(alloc)).expect("NonNull pointer to c_struct");
    Ok(Wrapper { inner })

I guess into_raw and from_raw fit better.