Re: [PATCH] rust: task: add `as_raw()` to `Task`

From: Wedson Almeida Filho <wedsonaf@gmail.com>
To: Antonio Hickey <antoniohickey99@gmail.com>
Cc: ojeda@kernel.org, alex.gaynor@gmail.com, boqun.feng@gmail.com,
	 gary@garyguo.net, bjorn3_gh@protonmail.com,
	benno.lossin@proton.me,  a.hindborg@samsung.com,
	aliceryhl@google.com, rust-for-linux@vger.kernel.org
Subject: Re: [PATCH] rust: task: add `as_raw()` to `Task`
Date: Tue, 16 Jan 2024 04:29:38 -0300	[thread overview]
Message-ID: <CANeycqpk77REoVvmdM-Lbb=rW0kyW7kNFfj_wRYto9G+qzaAmA@mail.gmail.com> (raw)
In-Reply-To: <20240116022823.64058-1-antoniohickey99@gmail.com>

On Mon, 15 Jan 2024 at 23:29, Antonio Hickey <antoniohickey99@gmail.com> wrote:
> +    /// Returns a raw pointer to the underlying C task struct.
> +    pub fn as_raw(&self) -> *mut bindings::task_struct {
> +        self.0.get()
> +    }
> +

Note that the wrapped `bindings::task_struct` in `Task` was
`pub(crate)`, but you're making `as_raw` fully public.

We don't want  modules to use the definitions in `bindings` (only
subsystems). So I'd suggest limiting `as_raw` to `pub(crate)`.

>      /// Wakes up the task.
>      pub fn wake_up(&self) {
> -        // SAFETY: By the type invariant, we know that `self.0.get()` is non-null and valid.
> +        // SAFETY: By the type invariant, we know that `self.raw()` is non-null and valid.

Typo in the comment: `raw` vs `as_raw`?