Re: [PATCH v2 5/7] rust: file: add `Kuid` wrapper

From: Benno Lossin <benno.lossin@proton.me>
To: Alice Ryhl <aliceryhl@google.com>
Cc: "Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Wedson Almeida Filho" <wedsonaf@gmail.com>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Andreas Hindborg" <a.hindborg@samsung.com>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Alexander Viro" <viro@zeniv.linux.org.uk>,
	"Christian Brauner" <brauner@kernel.org>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Arve Hjønnevåg" <arve@android.com>,
	"Todd Kjos" <tkjos@android.com>,
	"Martijn Coenen" <maco@android.com>,
	"Joel Fernandes" <joel@joelfernandes.org>,
	"Carlos Llamas" <cmllamas@google.com>,
	"Suren Baghdasaryan" <surenb@google.com>,
	"Dan Williams" <dan.j.williams@intel.com>,
	"Kees Cook" <keescook@chromium.org>,
	"Matthew Wilcox" <willy@infradead.org>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Daniel Xu" <dxu@dxuuu.xyz>,
	linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org,
	linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v2 5/7] rust: file: add `Kuid` wrapper
Date: Fri, 08 Dec 2023 16:40:09 +0000	[thread overview]
Message-ID: <jtCKrRw-FNajNJOXOuI1sweeDxI8T_uYnJ7DxMuqnJc9sgWjS0zouT_XIS-KmPferL7lU51BwD6nu73jZtzzB0T17pDeQP0-sFGRQxdjnaA=@proton.me> (raw)
In-Reply-To: <20231206-alice-file-v2-5-af617c0d9d94@google.com>

On 12/6/23 12:59, Alice Ryhl wrote:
> +    /// Returns the given task's pid in the current pid namespace.
> +    pub fn pid_in_current_ns(&self) -> Pid {
> +        // SAFETY: Calling `task_active_pid_ns` with the current task is always safe.
> +        let namespace = unsafe { bindings::task_active_pid_ns(bindings::get_current()) };

Why not create a safe wrapper for `bindings::get_current()`?
This patch series has three occurrences of `get_current`, so I think it
should be ok to add a wrapper.
I would also prefer to move the call to `bindings::get_current()` out of
the `unsafe` block.

> +        // SAFETY: We know that `self.0.get()` is valid by the type invariant.

What about `namespace`?

-- 
Cheers,
Benno

> +        unsafe { bindings::task_tgid_nr_ns(self.0.get(), namespace) }
> +    }