[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <92aaf8d8-13f5-1fef-557e-013e91e5c97b@gmail.com>
Date: Sun, 29 Jan 2023 00:52:11 -0300
From: Martin Rodriguez Reboredo <yakoyoku@...il.com>
To: Boqun Feng <boqun.feng@...il.com>
Cc: wedsonaf@...il.com, alex.gaynor@...il.com,
bjorn3_gh@...tonmail.com, gary@...yguo.net,
linux-kernel@...r.kernel.org, ojeda@...nel.org,
rust-for-linux@...r.kernel.org
Subject: Re: [PATCH 2/5] rust: types: introduce `ForeignOwnable`
On 1/28/23 19:07, Boqun Feng wrote:
> On Sat, Jan 28, 2023 at 05:46:50PM -0300, Martin Rodriguez Reboredo wrote:
>> 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.
>>
>
> For this particular use case, IIUC, what you need is exactly `AsRef`:
>
> fn byte_offset<T, P: AsRef<T>>(ptr: &P, pivot: &P) -> isize {
> unsafe {
> (ptr.as_ref() as *const _).cast::<u8>()
> .byte_offset((pivot.as_ref() *const _).cast())
> }
> }
>
> or you can implement a as_pointer() helper function to avoid the
> duplicate `.as_ref() as *const _`:
>
> fn as_pointer<T, P: AsRef<T>>(ptr: &P) -> *const T {
> ptr.as_ref() as *const T
> }
>
> Although, we need to `impl AsRef<T> for Arc<T>` to make it work for
> `Arc<T>`, which is currently missing.
>
> Regards,
> Boqun
>
Now that you say it I think that I can do what I wanted by further
constraining it to `ForeignOwnable` plus `AsRef`, so thanks for the tip
Boqun.
>>>>> +
>>>>> + /// 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@...il.com>
Powered by blists - more mailing lists