[PATCH v2] rust: types: extend `Opaque` documentation

[PATCH v2] rust: types: extend `Opaque` documentation

[Dirk Behme]

Update the `Opaque` documentation and add an example as proposed by
Miguel Ojeda in [1]. The documentation update is mainly taken from
Benno Lossin's description [2].

Suggested-by: Miguel Ojeda <ojeda@kernel.org>
Cc: Benno Lossin <benno.lossin@proton.me>
Cc: Nell Shamrell-Harrington <nells@linux.microsoft.com>
Link :https://rust-for-linux.zulipchat.com/#narrow/stream/291565-Help/topic/What.20to.20work.20on.20next.3F/near/467478085 [1]
Link: https://rust-for-linux.zulipchat.com/#narrow/stream/291565-Help/topic/What.20to.20work.20on.20next.3F/near/470498289 [2]
Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
---

Changes in v2: Correct typos and formatting. Drop non-camel-case.
               Add SAFETY comment.

 rust/kernel/types.rs | 45 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 10e3b1a999f4c..5e96f2c46f9fd 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -290,7 +290,50 @@ fn drop(&mut self) {
 
 /// Stores an opaque value.
 ///
-/// This is meant to be used with FFI objects that are never interpreted by Rust code.
+/// `Opaque<T>` is meant to be used with FFI objects that are never interpreted by Rust code.
+///
+/// It is used to wrap structs from the C side, like for example `Opaque<bindings::mutex>`.
+/// It gets rid of all the usual assumptions that Rust has for a value:
+///
+/// * The value is allowed to be uninitialized (for example have invalid bit patterns: `3` for a `bool`).
+/// * The value is allowed to be mutated, when a `&Opaque<T>` exists on the Rust side.
+/// * No uniqueness for mutable references: it is fine to have multiple &mut `Opaque<T>` point to the same value.
+/// * The value is not allowed to be shared with other threads (i.e. it is `!Sync`).
+///
+/// This has to be used for all values that the C side has access to, because it can't be ensured
+/// that the C side is adhering to the usual constraints that Rust needs.
+///
+/// Using `Opaque<T>` allows to continue to use references on the Rust side even for values shared with C.
+///
+/// # Examples
+///
+/// ```rust
+/// # #![allow(unreachable_pub, clippy::disallowed_names)]
+/// use kernel::types::Opaque;
+/// # // Emulate a C struct binding which is from C, maybe uninitialized or not, only the C side knows.
+/// # mod bindings {
+/// #     pub struct Foo {pub val: u8}
+/// # }
+///
+/// // `foo.val` is assumed to be handled on the C side, so we use `Opaque` to wrap it.
+/// pub struct Foo {
+///     foo: Opaque<bindings::Foo>,
+/// }
+///
+/// impl Foo {
+///     pub fn get_val(&self) -> u8 {
+///         let ptr = Opaque::get(&self.foo);
+///
+///         // SAFETY: Self is valid from C side
+///         unsafe { (*ptr).val }
+///     }
+/// }
+///
+/// // Create an instance of `Foo` with the `Opaque` wrapper.
+/// let foo = Foo { foo: Opaque::new(bindings::Foo {val: 0xdb}) };
+///
+/// assert_eq!(foo.get_val(), 0xdb);
+/// ```
 #[repr(transparent)]
 pub struct Opaque<T> {
     value: UnsafeCell<MaybeUninit<T>>,
-- 
2.46.2

Re: [PATCH v2] rust: types: extend `Opaque` documentation

[Benno Lossin]

On 02.10.24 07:03, Dirk Behme wrote:
> Update the `Opaque` documentation and add an example as proposed by
> Miguel Ojeda in [1]. The documentation update is mainly taken from
> Benno Lossin's description [2].
> 
> Suggested-by: Miguel Ojeda <ojeda@kernel.org>
> Cc: Benno Lossin <benno.lossin@proton.me>
> Cc: Nell Shamrell-Harrington <nells@linux.microsoft.com>
> Link :https://rust-for-linux.zulipchat.com/#narrow/stream/291565-Help/topic/What.20to.20work.20on.20next.3F/near/467478085 [1]
> Link: https://rust-for-linux.zulipchat.com/#narrow/stream/291565-Help/topic/What.20to.20work.20on.20next.3F/near/470498289 [2]
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> ---
> 
> Changes in v2: Correct typos and formatting. Drop non-camel-case.
>                Add SAFETY comment.
> 
>  rust/kernel/types.rs | 45 +++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index 10e3b1a999f4c..5e96f2c46f9fd 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -290,7 +290,50 @@ fn drop(&mut self) {
> 
>  /// Stores an opaque value.
>  ///
> -/// This is meant to be used with FFI objects that are never interpreted by Rust code.
> +/// `Opaque<T>` is meant to be used with FFI objects that are never interpreted by Rust code.
> +///
> +/// It is used to wrap structs from the C side, like for example `Opaque<bindings::mutex>`.
> +/// It gets rid of all the usual assumptions that Rust has for a value:
> +///
> +/// * The value is allowed to be uninitialized (for example have invalid bit patterns: `3` for a `bool`).
> +/// * The value is allowed to be mutated, when a `&Opaque<T>` exists on the Rust side.
> +/// * No uniqueness for mutable references: it is fine to have multiple &mut `Opaque<T>` point to the same value.

The "`" should be in front of the "&mut".

> +/// * The value is not allowed to be shared with other threads (i.e. it is `!Sync`).
> +///
> +/// This has to be used for all values that the C side has access to, because it can't be ensured
> +/// that the C side is adhering to the usual constraints that Rust needs.
> +///
> +/// Using `Opaque<T>` allows to continue to use references on the Rust side even for values shared with C.

Miguel already mentioned this: could you add my Co-developed-by, since
most of this stuff is from me. Thanks!

---
Cheers,
Benno

Re: [PATCH v2] rust: types: extend `Opaque` documentation

[Dirk Behme]

On 23.10.2024 21:51, Benno Lossin wrote:
> On 02.10.24 07:03, Dirk Behme wrote:
>> Update the `Opaque` documentation and add an example as proposed by
>> Miguel Ojeda in [1]. The documentation update is mainly taken from
>> Benno Lossin's description [2].
>>
>> Suggested-by: Miguel Ojeda <ojeda@kernel.org>
>> Cc: Benno Lossin <benno.lossin@proton.me>
>> Cc: Nell Shamrell-Harrington <nells@linux.microsoft.com>
>> Link :https://rust-for-linux.zulipchat.com/#narrow/stream/291565-Help/topic/What.20to.20work.20on.20next.3F/near/467478085 [1]
>> Link: https://rust-for-linux.zulipchat.com/#narrow/stream/291565-Help/topic/What.20to.20work.20on.20next.3F/near/470498289 [2]
>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
>> ---
>>
>> Changes in v2: Correct typos and formatting. Drop non-camel-case.
>>                 Add SAFETY comment.
>>
>>   rust/kernel/types.rs | 45 +++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 44 insertions(+), 1 deletion(-)
>>
>> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
>> index 10e3b1a999f4c..5e96f2c46f9fd 100644
>> --- a/rust/kernel/types.rs
>> +++ b/rust/kernel/types.rs
>> @@ -290,7 +290,50 @@ fn drop(&mut self) {
>>
>>   /// Stores an opaque value.
>>   ///
>> -/// This is meant to be used with FFI objects that are never interpreted by Rust code.
>> +/// `Opaque<T>` is meant to be used with FFI objects that are never interpreted by Rust code.
>> +///
>> +/// It is used to wrap structs from the C side, like for example `Opaque<bindings::mutex>`.
>> +/// It gets rid of all the usual assumptions that Rust has for a value:
>> +///
>> +/// * The value is allowed to be uninitialized (for example have invalid bit patterns: `3` for a `bool`).
>> +/// * The value is allowed to be mutated, when a `&Opaque<T>` exists on the Rust side.
>> +/// * No uniqueness for mutable references: it is fine to have multiple &mut `Opaque<T>` point to the same value.
> 
> The "`" should be in front of the "&mut".
> 
>> +/// * The value is not allowed to be shared with other threads (i.e. it is `!Sync`).
>> +///
>> +/// This has to be used for all values that the C side has access to, because it can't be ensured
>> +/// that the C side is adhering to the usual constraints that Rust needs.
>> +///
>> +/// Using `Opaque<T>` allows to continue to use references on the Rust side even for values shared with C.
> 
> Miguel already mentioned this: could you add my Co-developed-by, since
> most of this stuff is from me. Thanks!

Yes, sure. I think I was just waiting for your Ack for that ;)

Miguel: Do you want a v3 adding Benno's CDB & SOB and the movement of 
"`" above? Or could you do that while applying?

Thanks!

Dirk

Re: [PATCH v2] rust: types: extend `Opaque` documentation

[Miguel Ojeda]

On Thu, Oct 24, 2024 at 6:55 AM Dirk Behme <dirk.behme@de.bosch.com> wrote:
>
> Yes, sure. I think I was just waiting for your Ack for that ;)

Yeah, one needs permission to add it, so you did the right thing :)

> Miguel: Do you want a v3 adding Benno's CDB & SOB and the movement of
> "`" above? Or could you do that while applying?

I can do that, no worries -- thanks!

Cheers,
Miguel

Re: [PATCH v2] rust: types: extend `Opaque` documentation

[Miguel Ojeda]

On Wed, Oct 2, 2024 at 7:03 AM Dirk Behme <dirk.behme@de.bosch.com> wrote:
>
> Update the `Opaque` documentation and add an example as proposed by
> Miguel Ojeda in [1]. The documentation update is mainly taken from
> Benno Lossin's description [2].
>
> Suggested-by: Miguel Ojeda <ojeda@kernel.org>
> Cc: Benno Lossin <benno.lossin@proton.me>
> Cc: Nell Shamrell-Harrington <nells@linux.microsoft.com>
> Link :https://rust-for-linux.zulipchat.com/#narrow/stream/291565-Help/topic/What.20to.20work.20on.20next.3F/near/467478085 [1]
> Link: https://rust-for-linux.zulipchat.com/#narrow/stream/291565-Help/topic/What.20to.20work.20on.20next.3F/near/470498289 [2]
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>

Applied to `rust-next` -- thanks everyone!

    [ Used `expect`. Rewrapped docs. Added intra-doc link. Formatted
      example. Reworded to fix tag typo/order. Fixed `&mut` formatting
      as discussed. Added Benno's SOB and CDB as discussed. Shortened
      links. - Miguel ]

Cheers,
Miguel