Re: [PATCH] rust: irq: add support for request_irq()

[Alice Ryhl <aliceryhl@google.com>]

On Wed, Jan 15, 2025 at 1:31 PM Daniel Almeida
<daniel.almeida@collabora.com> wrote:
>
>
>
> > On 15 Jan 2025, at 08:38, Gary Guo <gary@garyguo.net> wrote:
> >
> > On Tue, 14 Jan 2025 15:57:57 -0300
> > Daniel Almeida <daniel.almeida@collabora.com> wrote:
> >
> >>>
> >>> It's not the pin_init! stuff, but the Opaque stuff. If it fails, then
> >>> it runs the destructor of Opaque<T>, which does *not* run the
> >>> destructor of T.
> >>>
> >>> Alice
> >>
> >> This is pretty unintuitive if you take into account trivial examples like
> >>
> >> ```
> >> struct Foo(T)
> >> ```
> >>
> >> Where dropping Foo drops T.
> >>
> >> Is there any reason why dropping Opaque<T> doesn’t behave similarly?
> >>
> >> — Daniel
> >
> > `Opaque` means that "this is a blob of bytes and don't touch it". It
> > can be uninitialized, so no meaningful action can be performed when
> > it's dropped.
> >
> > Best,
> > Gary
> >
>
> I really think that the documentation for this type has to be improved somehow.
>
> How is this a blob of bytes that can’t be touched, if it gives out a *mut T?
>
> I think I’ve consistently seen code that either accesses or mutates the inner T through
> the pointer.

It's not a problem to give out a *mut T. A Opaque is a blob of bytes
that *might* or *might not* contain a valid `T`. When you dereference
a raw pointer to the inner value, you are unsafely asserting that
*right now*, it does in fact contain a valid T.

The problem we're running into here is that Opaque is intended for
wrapping C types, but you're using it to wrap a Rust type instead. I
think we should have a new container type called `Aliased<T>` defined
like Opaque but without the MaybeUninit.

#[repr(transparent)]
pub struct Aliased<T> {
    value: UnsafeCell<T>,
    _pin: PhantomPinned,
}

This type can then serve the purpose of a Rust type that might be
accessed in weird ways, but *is* guaranteed to hold a valid Rust type.
It's destructor will run the destructor of T.

Alice