On 2/3/25 6:13 PM, Alice Ryhl wrote:
> On Sun, Feb 2, 2025 at 2:06 PM Asahi Lina <lina@xxxxxxxxxxxxx> wrote:
>>
>> By analogy to AlwaysRefCounted and ARef, an Ownable type is a (typically
>> C FFI) type that *may* be owned by Rust, but need not be. Unlike
>> AlwaysRefCounted, this mechanism expects the reference to be unique
>> within Rust, and does not allow cloning.
>>
>> Conceptually, this is similar to a KBox<T>, except that it delegates
>> resource management to the T instead of using a generic allocator.
>>
>> Signed-off-by: Asahi Lina <lina@xxxxxxxxxxxxx>
>
> Overall LGTM.
>
>> +/// A subtrait of Ownable that asserts that an `Owned<T>` Rust reference is not only unique
>> +/// within Rust and keeps the `T` alive, but also guarantees that the C code follows the
>> +/// usual mutable reference requirements. That is, the kernel will never mutate the
>> +/// `T` (excluding internal mutability that follows the usual rules) while Rust owns it.
>> +///
>> +/// When this type is implemented for an [`Ownable`] type, it allows `Owned<T>` to be
>> +/// dereferenced into a &mut T.
>> +///
>> +/// # Safety
>> +///
>> +/// Implementers must ensure that the kernel never mutates the underlying type while
>> +/// Rust owns it.
>> +pub unsafe trait OwnableMut: Ownable {}
>
> Giving out mutable references allows users to call core::mem::swap on
> the object. We must require that this is allowed.
Hmm, yeah. I don't use this yet, and I'm not sure if it makes much sense
with that caveat. I'll drop it for v2.
>
>> +impl<T: Ownable> Owned<T> {
>> + /// Creates a new instance of [`Owned`].
>> + ///
>> + /// It takes over ownership of the underlying object.
>> + ///
>> + /// # Safety
>> + ///
>> + /// Callers must ensure that the underlying object is acquired and can be considered owned by
>> + /// Rust.
>> + pub(crate) unsafe fn from_raw(ptr: NonNull<T>) -> Self {
>> + // INVARIANT: The safety requirements guarantee that the new instance now owns the
>> + // reference.
>> + Self {
>> + ptr,
>> + _p: PhantomData,
>> + }
>> + }
>> +
>> + /// Consumes the `Owned`, returning a raw pointer.
>> + ///
>> + /// This function does not actually relinquish ownership of the object.
>> + /// After calling this function, the caller is responsible for ownership previously managed
>> + /// by the `Owned`.
>> + #[allow(dead_code)]
>> + pub(crate) fn into_raw(me: Self) -> NonNull<T> {
>
> I would just make these methods public, like the ARef ones. Then you
> can drop the #[allow(dead_code)] annotation.
Does it make sense to ever have drivers doing this? I feel like these
methods should be limited to the kernel crate.
~~ Lina
