Re: [PATCH v5 1/4] rust: sync: change `<Arc<T> as ForeignOwnable>::PointedTo` to `T`

[Alice Ryhl]

On Thu, Feb 27, 2025 at 1:36 PM Andreas Hindborg <a.hindborg@xxxxxxxxxx> wrote:
>
> Using `ArcInner` as `PoinedTo` in the `ForeignOwnable` implementation for
> `Arc` is a bit unfortunate. Using `T` as `PointedTo` does not remove any
> functionality, but allows `ArcInner` to be private. Further, it allows
> downstream users to write code that is generic over `Box` and `Arc`, when
> downstream users need access to `T` after calling `into_foreign`.
>
> Reviewed-by: Fiona Behrens <me@xxxxxxxxxx>
> Reviewed-by: Daniel Almeida <daniel.almeida@xxxxxxxxxxxxx>
> Tested-by: Daniel Almeida <daniel.almeida@xxxxxxxxxxxxx>
> Signed-off-by: Andreas Hindborg <a.hindborg@xxxxxxxxxx>

We discussed this in the meeting yesterday, but just to summarize:

This isn't correct use of the trait. The trait is intended for cases
where you pass a void pointer into C code, and the C code treats that
void pointer entirely opaquely. That's why the docs for `into_foreign`
say this:

The foreign representation is a pointer to void. There are no
guarantees for this pointer. For example, it might be invalid,
dangling or pointing to uninitialized memory. Using it in any way
except for [`from_foreign`], [`try_from_foreign`], [`borrow`], or
[`borrow_mut`] can result in undefined behavior.

In this case, you want to make this change because the C code in
configfs will dereference the void pointer and read from it. But
that's not allowed with the ForeignOwnable trait. You need a new trait
if you want pointers that are not opaque.

Alice