Re: [PATCH 2/5] rust: types: introduce `ForeignOwnable`

From: Martin Rodriguez Reboredo
Date: Sat Jan 28 2023 - 15:47:16 EST


On 1/28/23 14:05, Boqun Feng wrote:
> On Sat, Jan 28, 2023 at 11:53:45AM -0300, Martin Rodriguez Reboredo wrote:
> [...]
>>> + /// Borrows a foreign-owned object.
>>> + ///
>>> + /// # Safety
>>> + ///
>>> + /// `ptr` must have been returned by a previous call to [`ForeignOwnable::into_foreign`] for
>>> + /// which a previous matching [`ForeignOwnable::from_foreign`] hasn't been called yet.
>>> + /// Additionally, all instances (if any) of values returned by [`ForeignOwnable::borrow_mut`]
>>> + /// for this object must have been dropped.
>>> + unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> Self::Borrowed<'a>;
>>> +
>>> + /// Mutably borrows a foreign-owned object.
>>> + ///
>>> + /// # Safety
>>> + ///
>>> + /// `ptr` must have been returned by a previous call to [`ForeignOwnable::into_foreign`] for
>>> + /// which a previous matching [`ForeignOwnable::from_foreign`] hasn't been called yet.
>>> + /// Additionally, all instances (if any) of values returned by [`ForeignOwnable::borrow`] and
>>> + /// [`ForeignOwnable::borrow_mut`] for this object must have been dropped.
>>> + unsafe fn borrow_mut<T: ForeignOwnable>(ptr: *const core::ffi::c_void) -> ScopeGuard<T, fn(T)> {
>>> + // SAFETY: The safety requirements ensure that `ptr` came from a previous call to
>>> + // `into_foreign`.
>>> + ScopeGuard::new_with_data(unsafe { T::from_foreign(ptr) }, |d| {
>>> + d.into_foreign();
>>> + })
>>> + }
>>
>> Could these three methods have a borrowing equivalent? When I was
>> working on some features for the USB module I've stumbled upon the case
>> of having to encode a pointer (with a pivot) and I cannot do it without
>> taking ownership of the pointer.
>>
>
> *const T is Copy, so you can still use it after pass it to a function or
> a new binding, e.g.
>
> pub fn use_ptr(ptr: *const i32) { .. }
>
> let p: *const i32 = some_func();
>
> let q = p;
>
> // q is just a copy of p
> use_ptr(p);
> // passing to a function parameter is just copying
> use_ptr(p);
>
> maybe I'm missing something subtle, but if you have an example I can
> help take a look.
>
> Regards,
> Boqun
>

I'll use a much more simple example. If I want to take the byte offset
between two `ForeignWrapper`s I'd have to take ownership of them, but I
don't see it desirable in some cases.

fn byte_offset<P: PointerWrapper>(ptr: P, pivot: P) -> isize {
unsafe {
ptr.into_pointer().cast::<u8>()
.byte_offset(pivot.into_pointer().cast())
}
}

But if there was an `as_pointer(&self) -> *const c_void` method then the
above function will be able to borrow both `ForeignWrapper`s.

fn byte_offset<P: PointerWrapper>(ptr: &P, pivot: &P) -> isize {
unsafe {
ptr.as_pointer().cast::<u8>()
.byte_offset(pivot.as_pointer().cast())
}
}

Obviously those methods that borrow will announce invariancies in their
doc comments. If these can exist then great and if not then another
solution could be explored.

>>> +
>>> + /// Converts a foreign-owned object back to a Rust-owned one.
>>> + ///
>>> + /// # Safety
>>> + ///
>>> + /// `ptr` must have been returned by a previous call to [`ForeignOwnable::into_foreign`] for
>>> + /// which a previous matching [`ForeignOwnable::from_foreign`] hasn't been called yet.
>>> + /// Additionally, all instances (if any) of values returned by [`ForeignOwnable::borrow`] and
>>> + /// [`ForeignOwnable::borrow_mut`] for this object must have been dropped.
>>> + unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self;
>>> +}
>>> +
>>> /// Runs a cleanup function/closure when dropped.
>>> ///
>>> /// The [`ScopeGuard::dismiss`] function prevents the cleanup function from running.
>>> --
>>> 2.34.1
>>
>> Aside from these comments I observe that there's a possibility to make
>> ForeignOwnable a const trait and have non const implementors. Otherwise
>> if these things are out of scope, no problem whatsoever and this has my
>> OK.
>>
>> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@xxxxxxxxx>