Messages in this thread |  | | Date | Sat, 28 Jan 2023 17:46:50 -0300 | Subject | Re: [PATCH 2/5] rust: types: introduce `ForeignOwnable` | From | Martin Rodriguez Reboredo <> |
| |
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@gmail.com>
|  |