[PATCH v3 0/2] rust: add NonEmpty

[PATCH v3 0/2] rust: add NonEmpty

[Lorenzo Delgado]

Some functions check that a container is not empty and later rely on it
through `unwrap_unchecked()`, `unwrap()` or an error path that can't be
taken. Gary suggested a non-empty type for this while reviewing Thorsten
Blum's uaccess patch [1], and Miguel filed it as #1262 [2].

Patch 1 adds `NonEmpty<C>` in `kernel::non_empty`, with only what the
first user needs: slices, `try_from_mut()`, `from_mut()` for arrays
and `last_mut()`. Patch 2 makes `strcpy_into_buf()` take a
`NonEmpty<[u8]>`, which drops an `unsafe` block and the `EINVAL` case.

Some notes:

- `IsEmpty` is a safe trait. Unsafe code only relies on the
  implementations in `kernel::non_empty`, and the docs require the
  result not to change while the container is only reachable through a
  shared reference. If a generic accessor ever needs to rely on it, it
  can become an `unsafe trait` then.

- Since APIs aren't added without users, other accessors (`first()`,
  `try_from_ref()`, ...) and containers are left for later. I plan to
  send follow-ups for `net/phy.rs` and `scatterlist.rs`, which both
  reject an empty slice and then rely on it.

- I put it in its own `kernel::non_empty` module. Would a broader home
  like `kernel::collections` be better?

Francis Laniel posted a slice-only RFC [3]. Following Gary's suggestion
on the issue, this version is generic over the container.

[1] https://lore.kernel.org/rust-for-linux/DM0D9TERQAM3.2Y3HV6P68E2N8@garyguo.net/
[2] https://github.com/Rust-for-Linux/linux/issues/1262
[3] https://lore.kernel.org/rust-for-linux/20261011191540.56199-1-laniel_francis@npnm.eu/

Signed-off-by: Lorenzo Delgado <lnsdev@proton.me>
---
Changes in v3:
- Replace `from_array_mut()` with a `const fn from_mut()` on
  `NonEmpty<[T; N]>`, and get `NonEmpty<[T]>` from it by unsize
  coercion instead of a slice pointer cast (Gary).
- Link to v2: https://patch.msgid.link/20261012-nonempty-slice-v2-0-0f2b67d9463e@proton.me

Changes in v2:
- Take `&mut NonEmpty<[u8]>` in `strcpy_into_buf()` and drop the
  `EINVAL` case for an empty buffer (Miguel).
- Add `NonEmpty::from_array_mut()`, which checks the array length at
  compile time, so callers of `strcpy_into_buf()` with an array buffer
  need no runtime check.
- Link to v1: https://patch.msgid.link/20261012-nonempty-slice-v1-0-37d1fecff82c@proton.me

---
Lorenzo Delgado (2):
      rust: non_empty: add NonEmpty
      rust: uaccess: use NonEmpty in strcpy_into_buf()

 rust/kernel/lib.rs       |   1 +
 rust/kernel/non_empty.rs | 123 +++++++++++++++++++++++++++++++++++++++++++++++
 rust/kernel/uaccess.rs   |  15 ++----
 3 files changed, 129 insertions(+), 10 deletions(-)
---
base-commit: 2dee3d3adcadcc57d62ba6608cd02f50b0c10264
change-id: 20261011-nonempty-slice-022084ba9a5a

Best regards,
--  
Lorenzo Delgado <lnsdev@proton.me>

[PATCH v3 1/2] rust: non_empty: add NonEmpty

[Lorenzo Delgado]

Some functions reject an empty container up front and later rely on it
having at least one element. The type system does not see that check,
so the later access ends up as an `unwrap_unchecked()` with a SAFETY
comment pointing back at it, or as an `unwrap()` or an error path that
can never be taken. `strcpy_into_buf()` in uaccess is one such case.

Add `NonEmpty<C>`, a transparent wrapper whose constructors check that
the container is not empty. It dereferences to the container, and a
method whose `Option` only exists for the empty case can be shadowed by
a version that returns the value directly.

`DerefMut` is implemented per container, only where a mutable reference
cannot change the length. For a growable container it would let
`clear()` break the invariant.

For now only slices and arrays are supported. A slice of unknown
length goes through `try_from_mut()`, which returns `None` if it is
empty. An array goes through `from_mut()`, which checks the length at
compile time instead of returning an `Option`. The resulting
`&mut NonEmpty<[T; N]>` coerces to `&mut NonEmpty<[T]>`, so a caller
with a fixed-size buffer needs no runtime check and no extra cast. The
only accessor is `last_mut()`, which uaccess needs. Other containers
and accessors can follow with their users.

Suggested-by: Gary Guo <gary@garyguo.net>
Link: https://lore.kernel.org/rust-for-linux/DM0D9TERQAM3.2Y3HV6P68E2N8@garyguo.net/
Link: https://github.com/Rust-for-Linux/linux/issues/1262
Signed-off-by: Lorenzo Delgado <lnsdev@proton.me>
---
 rust/kernel/lib.rs       |   1 +
 rust/kernel/non_empty.rs | 123 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 124 insertions(+)

diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 7225abc64084..8e1928deac35 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -105,6 +105,7 @@
 pub mod module_param;
 #[cfg(CONFIG_NET)]
 pub mod net;
+pub mod non_empty;
 pub mod num;
 pub mod of;
 #[cfg(CONFIG_PM_OPP)]
diff --git a/rust/kernel/non_empty.rs b/rust/kernel/non_empty.rs
new file mode 100644
index 000000000000..5eec67a4f8a7
--- /dev/null
+++ b/rust/kernel/non_empty.rs
@@ -0,0 +1,123 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Containers that are known not to be empty.
+
+use crate::build_assert::const_assert;
+use core::ops::{
+    Deref,
+    DerefMut, //
+};
+
+/// A container that can report whether it holds any element.
+///
+/// The result must not change while the container is only reachable through a shared reference.
+///
+/// Unsafe code may only rely on the result for a container type whose implementation it trusts,
+/// such as the ones in this module, since a safe implementation can return anything.
+pub trait IsEmpty {
+    /// Returns `true` if the container holds no element.
+    fn is_empty(&self) -> bool;
+}
+
+impl<T> IsEmpty for [T] {
+    #[inline]
+    fn is_empty(&self) -> bool {
+        <[T]>::is_empty(self)
+    }
+}
+
+/// A container that holds at least one element.
+///
+/// [`NonEmpty`] dereferences to the container it wraps, so the container's API stays available.
+/// Methods whose result is only optional because the container could be empty can be shadowed by
+/// versions that return the result directly. They are added as users need them. For now that is
+/// only [`NonEmpty::last_mut`] on a slice, which returns `&mut T` instead of `Option<&mut T>`.
+///
+/// # Invariants
+///
+/// The wrapped container is not empty. Unsafe code may rely on this only for arrays and for
+/// container types whose [`IsEmpty`] implementation is in this module.
+///
+/// # Examples
+///
+/// ```
+/// use kernel::non_empty::NonEmpty;
+///
+/// let mut buf = [1u8, 2, 3];
+/// let slice: &mut NonEmpty<[u8]> = NonEmpty::from_mut(&mut buf);
+/// *slice.last_mut() = 0;
+/// assert_eq!(slice.len(), 3);
+/// assert_eq!(buf, [1, 2, 0]);
+///
+/// let mut buf = [1u8, 2, 3];
+/// let slice = NonEmpty::try_from_mut(&mut buf[..2]).ok_or(EINVAL)?;
+/// *slice.last_mut() = 0;
+/// assert_eq!(buf, [1, 0, 3]);
+///
+/// let mut empty: [u8; 0] = [];
+/// assert!(NonEmpty::try_from_mut(&mut empty[..]).is_none());
+/// # Ok::<(), Error>(())
+/// ```
+#[repr(transparent)]
+#[derive(Debug)]
+pub struct NonEmpty<C: ?Sized>(C);
+
+impl<C: ?Sized + IsEmpty> NonEmpty<C> {
+    /// Creates a [`NonEmpty`] from a mutable reference to a container, or returns [`None`] if the
+    /// container is empty.
+    #[inline]
+    pub fn try_from_mut(container: &mut C) -> Option<&mut Self> {
+        if container.is_empty() {
+            return None;
+        }
+
+        // INVARIANT: `container` was checked to be non-empty above.
+        // SAFETY: `NonEmpty<C>` is transparent to `C`, so the cast keeps both the layout and the
+        // pointer metadata, and the pointer comes from a valid `&mut C` with the same lifetime.
+        Some(unsafe { &mut *(core::ptr::from_mut(container) as *mut Self) })
+    }
+}
+
+impl<T, const N: usize> NonEmpty<[T; N]> {
+    /// Creates a [`NonEmpty`] array from a mutable reference to an array.
+    ///
+    /// Unlike [`NonEmpty::try_from_mut`], this does not return an [`Option`]: the array length is
+    /// checked at compile time, and using it with an empty array is a build error. The result
+    /// coerces to a [`NonEmpty`] slice, which is where its accessors are.
+    #[inline]
+    pub const fn from_mut(array: &mut [T; N]) -> &mut Self {
+        const_assert!(N > 0);
+
+        // INVARIANT: `array` has `N` elements, and `N > 0` was checked at compile time.
+        // SAFETY: `NonEmpty<[T; N]>` is transparent to `[T; N]`, so the cast keeps the layout, and
+        // the pointer comes from a valid `&mut [T; N]` with the same lifetime.
+        unsafe { &mut *core::ptr::from_mut(array).cast::<Self>() }
+    }
+}
+
+impl<T> NonEmpty<[T]> {
+    /// Returns a mutable reference to the last element of the slice.
+    #[inline]
+    pub fn last_mut(&mut self) -> &mut T {
+        // SAFETY: By the type invariant, the slice is not empty, so it has a last element.
+        unsafe { self.0.last_mut().unwrap_unchecked() }
+    }
+}
+
+impl<C: ?Sized> Deref for NonEmpty<C> {
+    type Target = C;
+
+    #[inline]
+    fn deref(&self) -> &C {
+        &self.0
+    }
+}
+
+// `DerefMut` is only sound for containers whose length cannot change through a mutable
+// reference. For a growable container such as `KVec`, `clear()` would empty it.
+impl<T> DerefMut for NonEmpty<[T]> {
+    #[inline]
+    fn deref_mut(&mut self) -> &mut [T] {
+        &mut self.0
+    }
+}

-- 
2.55.0

[PATCH v3 2/2] rust: uaccess: use NonEmpty in strcpy_into_buf()

[Lorenzo Delgado]

`strcpy_into_buf()` returns `EINVAL` for an empty buffer and later
writes the NUL terminator through `last_mut().unwrap_unchecked()`,
relying on that earlier check.

Take the buffer as a `NonEmpty<[u8]>` instead, so the requirement is in
the signature. The `EINVAL` case goes away, and `last_mut()` returns
the element directly, which removes the `unsafe` block. There are no
callers to convert. A caller with an array buffer can pass it through
`NonEmpty::from_mut()` without a runtime check.

The `MaybeUninit` cast still starts from `[u8]`, so its SAFETY comment
does not depend on the layout of the wrapper.

Suggested-by: Gary Guo <gary@garyguo.net>
Link: https://lore.kernel.org/rust-for-linux/DM0D9TERQAM3.2Y3HV6P68E2N8@garyguo.net/
Link: https://github.com/Rust-for-Linux/linux/issues/1262
Signed-off-by: Lorenzo Delgado <lnsdev@proton.me>
---
 rust/kernel/uaccess.rs | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
index 5f6c4d7a1a51..1cc5c7580d3a 100644
--- a/rust/kernel/uaccess.rs
+++ b/rust/kernel/uaccess.rs
@@ -11,6 +11,7 @@
     error::Result,
     ffi::{c_char, c_void},
     fs::file,
+    non_empty::NonEmpty,
     prelude::*,
     ptr::KnownSize,
     transmute::{AsBytes, FromBytes},
@@ -383,20 +384,16 @@ pub fn read_all<A: Allocator>(mut self, buf: &mut Vec<u8, A>, flags: Flags) -> R
     /// Read a NUL-terminated string from userspace and return it.
     ///
     /// The string is read into `buf` and a NUL-terminator is added if the end of `buf` is reached.
-    /// Since there must be space to add a NUL-terminator, the buffer must not be empty. The
+    /// Since there must be space to add a NUL-terminator, `buf` is a [`NonEmpty`] slice. The
     /// returned `&CStr` points into `buf`.
     ///
     /// Fails with [`EFAULT`] if the read happens on a bad address (some data may have been
     /// copied).
     #[doc(alias = "strncpy_from_user")]
-    pub fn strcpy_into_buf<'buf>(self, buf: &'buf mut [u8]) -> Result<&'buf CStr> {
-        if buf.is_empty() {
-            return Err(EINVAL);
-        }
-
+    pub fn strcpy_into_buf<'buf>(self, buf: &'buf mut NonEmpty<[u8]>) -> Result<&'buf CStr> {
         // SAFETY: The types are compatible and `strncpy_from_user` doesn't write uninitialized
         // bytes to `buf`.
-        let mut dst = unsafe { &mut *(core::ptr::from_mut(buf) as *mut [MaybeUninit<u8>]) };
+        let mut dst = unsafe { &mut *(core::ptr::from_mut::<[u8]>(buf) as *mut [MaybeUninit<u8>]) };
 
         // We never read more than `self.length` bytes.
         if dst.len() > self.length {
@@ -422,9 +419,7 @@ pub fn strcpy_into_buf<'buf>(self, buf: &'buf mut [u8]) -> Result<&'buf CStr> {
             // This means that we filled the buffer exactly. In this case, we add a NUL-terminator
             // and return it. Unlike the `len < dst.len()` branch, don't modify `len` because it
             // already represents the length including the NUL-terminator.
-            //
-            // SAFETY: Due to the check at the beginning, the buffer is not empty.
-            unsafe { *buf.last_mut().unwrap_unchecked() = 0 };
+            *buf.last_mut() = 0;
         }
 
         // This method consumes `self`, so it can only be called once, thus we do not need to

-- 
2.55.0

Re: [PATCH v3 1/2] rust: non_empty: add NonEmpty

[Miguel Ojeda]

On Wed, Oct 14, 2026 at 10:30 AM Lorenzo Delgado <lnsdev@proton.me> wrote:
>
> +/// A container that can report whether it holds any element.
> +///
> +/// The result must not change while the container is only reachable through a shared reference.
> +///
> +/// Unsafe code may only rely on the result for a container type whose implementation it trusts,
> +/// such as the ones in this module, since a safe implementation can return anything.
> +pub trait IsEmpty {
> +    /// Returns `true` if the container holds no element.
> +    fn is_empty(&self) -> bool;
> +}

Do we need this to be a safe trait? If it is unsafe, then we can
simplify the invariant and more easily think about the type, like
other `Non*`s, no?

Thanks!

Cheers,
Miguel

Re: [PATCH v3 1/2] rust: non_empty: add NonEmpty

[Gary Guo]

On Wed Oct 14, 2026 at 12:55 PM BST, Miguel Ojeda wrote:
> On Wed, Oct 14, 2026 at 10:30 AM Lorenzo Delgado <lnsdev@proton.me> wrote:
>>
>> +/// A container that can report whether it holds any element.
>> +///
>> +/// The result must not change while the container is only reachable through a shared reference.
>> +///
>> +/// Unsafe code may only rely on the result for a container type whose implementation it trusts,
>> +/// such as the ones in this module, since a safe implementation can return anything.
>> +pub trait IsEmpty {
>> +    /// Returns `true` if the container holds no element.
>> +    fn is_empty(&self) -> bool;
>> +}
>
> Do we need this to be a safe trait? If it is unsafe, then we can
> simplify the invariant and more easily think about the type, like
> other `Non*`s, no?

I think in this case the meaning of "empty" depends on the container itself.
Unsafe code that relies on "is_empty" would be the `NonEmpty<Container>` itself,
so it just needs to trust itself (as boundary of unsafe is module level).

I think of almost all of the text above can be removed. The invariant of
`NonEmpty` can just be "`IsEmpty::is_empty` has returned true" and how it is
interpreted is up to the container type.

Best,
Gary

Re: [PATCH v3 1/2] rust: non_empty: add NonEmpty

[Alexandre Courbot]

On Wed Oct 14, 2026 at 11:25 PM JST, Gary Guo wrote:
> On Wed Oct 14, 2026 at 12:55 PM BST, Miguel Ojeda wrote:
>> On Wed, Oct 14, 2026 at 10:30 AM Lorenzo Delgado <lnsdev@proton.me> wrote:
>>>
>>> +/// A container that can report whether it holds any element.
>>> +///
>>> +/// The result must not change while the container is only reachable through a shared reference.
>>> +///
>>> +/// Unsafe code may only rely on the result for a container type whose implementation it trusts,
>>> +/// such as the ones in this module, since a safe implementation can return anything.
>>> +pub trait IsEmpty {
>>> +    /// Returns `true` if the container holds no element.
>>> +    fn is_empty(&self) -> bool;
>>> +}
>>
>> Do we need this to be a safe trait? If it is unsafe, then we can
>> simplify the invariant and more easily think about the type, like
>> other `Non*`s, no?
>
> I think in this case the meaning of "empty" depends on the container itself.
> Unsafe code that relies on "is_empty" would be the `NonEmpty<Container>` itself,
> so it just needs to trust itself (as boundary of unsafe is module level).
>
> I think of almost all of the text above can be removed. The invariant of
> `NonEmpty` can just be "`IsEmpty::is_empty` has returned true" and how it is
> interpreted is up to the container type.

Is `IsEmpty` actually useful? It needs to be implemented for every
container we want to wrap with essentially the same boilerplate code,
which looks a bit overkill to me. Couldn't we just do:

  impl<T, C: Deref<Target = [T]>> NonEmpty<C>

and have a single constructor method that supports all container types
by reusing the slice's `is_empty` method?

That way we could get rid of `IsEmpty` entirely. That would limit
`NonEmpty` to types that directly deref to `[T]`, but I guess we are
going to use it on slices most of the time, with the exception of when
we want to return an owned container that is not empty, and those we
have already deref to a slice.

Re: [PATCH v3 0/2] rust: add NonEmpty

[Alexandre Courbot]

On Wed Oct 14, 2026 at 5:30 PM JST, Lorenzo Delgado wrote:
> Some functions check that a container is not empty and later rely on it
> through `unwrap_unchecked()`, `unwrap()` or an error path that can't be
> taken. Gary suggested a non-empty type for this while reviewing Thorsten
> Blum's uaccess patch [1], and Miguel filed it as #1262 [2].
>
> Patch 1 adds `NonEmpty<C>` in `kernel::non_empty`, with only what the
> first user needs: slices, `try_from_mut()`, `from_mut()` for arrays
> and `last_mut()`. Patch 2 makes `strcpy_into_buf()` take a
> `NonEmpty<[u8]>`, which drops an `unsafe` block and the `EINVAL` case.
>
> Some notes:
>
> - `IsEmpty` is a safe trait. Unsafe code only relies on the
>   implementations in `kernel::non_empty`, and the docs require the
>   result not to change while the container is only reachable through a
>   shared reference. If a generic accessor ever needs to rely on it, it
>   can become an `unsafe trait` then.
>
> - Since APIs aren't added without users, other accessors (`first()`,
>   `try_from_ref()`, ...) and containers are left for later. I plan to
>   send follow-ups for `net/phy.rs` and `scatterlist.rs`, which both
>   reject an empty slice and then rely on it.

Having `last` without `first` is weird. Please add the minimal set that
makes sense from the get-go, these are `pub` anyway so clippy won't
complain if they are unused.

>
> - I put it in its own `kernel::non_empty` module. Would a broader home
>   like `kernel::collections` be better?

`kernel::types` sounds like a proper place? Or if we want a dedicated
module, `kernel::slice` to match the standard library? `non_empty` is
too ad-hoc and specific.

Also as a general comment: the commit messages and comments read as
LLM-generated and are insufferably mechanical and tedious. Please turn
the LLM dial down a bit and spend some time writing or at least editing
them yourself to save some time for your reviewers. The doccomment for
`from_mut` is a good example of something that could be reduced to 2
lines max.

Re: [PATCH v3 2/2] rust: uaccess: use NonEmpty in strcpy_into_buf()

[Alexandre Courbot]

On Wed Oct 14, 2026 at 5:30 PM JST, Lorenzo Delgado wrote:
<...>
> diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
> index 5f6c4d7a1a51..1cc5c7580d3a 100644
> --- a/rust/kernel/uaccess.rs
> +++ b/rust/kernel/uaccess.rs
> @@ -11,6 +11,7 @@
>      error::Result,
>      ffi::{c_char, c_void},
>      fs::file,
> +    non_empty::NonEmpty,
>      prelude::*,
>      ptr::KnownSize,
>      transmute::{AsBytes, FromBytes},
> @@ -383,20 +384,16 @@ pub fn read_all<A: Allocator>(mut self, buf: &mut Vec<u8, A>, flags: Flags) -> R
>      /// Read a NUL-terminated string from userspace and return it.
>      ///
>      /// The string is read into `buf` and a NUL-terminator is added if the end of `buf` is reached.
> -    /// Since there must be space to add a NUL-terminator, the buffer must not be empty. The
> +    /// Since there must be space to add a NUL-terminator, `buf` is a [`NonEmpty`] slice. The
>      /// returned `&CStr` points into `buf`.
>      ///
>      /// Fails with [`EFAULT`] if the read happens on a bad address (some data may have been
>      /// copied).
>      #[doc(alias = "strncpy_from_user")]
> -    pub fn strcpy_into_buf<'buf>(self, buf: &'buf mut [u8]) -> Result<&'buf CStr> {
> -        if buf.is_empty() {
> -            return Err(EINVAL);
> -        }
> -
> +    pub fn strcpy_into_buf<'buf>(self, buf: &'buf mut NonEmpty<[u8]>) -> Result<&'buf CStr> {

I understand this was done per the v1 feedback, but won't it make it
more tedious for users to call this method? (hard to tell as there are
no users at the moment :))

We could still benefit from `NonEmpty` if we replace the `buf.is_empty`
by an attempt to build it and return `EINVAL` if that fails (what v1 did
basically) and calling `last_mut` on it to remove the
`unwrap_unchecked`.

Of course if callers are expected to already work with a `NonEmpty` then
that's fine, but otherwise my concern is that every one of them would
have to manually build one to perform a check that this method can
easily do.

Re: [PATCH v3 1/2] rust: non_empty: add NonEmpty

[Gary Guo]

On Thu Oct 15, 2026 at 1:43 PM BST, Alexandre Courbot wrote:
> On Wed Oct 14, 2026 at 11:25 PM JST, Gary Guo wrote:
>> On Wed Oct 14, 2026 at 12:55 PM BST, Miguel Ojeda wrote:
>>> On Wed, Oct 14, 2026 at 10:30 AM Lorenzo Delgado <lnsdev@proton.me> wrote:
>>>>
>>>> +/// A container that can report whether it holds any element.
>>>> +///
>>>> +/// The result must not change while the container is only reachable through a shared reference.
>>>> +///
>>>> +/// Unsafe code may only rely on the result for a container type whose implementation it trusts,
>>>> +/// such as the ones in this module, since a safe implementation can return anything.
>>>> +pub trait IsEmpty {
>>>> +    /// Returns `true` if the container holds no element.
>>>> +    fn is_empty(&self) -> bool;
>>>> +}
>>>
>>> Do we need this to be a safe trait? If it is unsafe, then we can
>>> simplify the invariant and more easily think about the type, like
>>> other `Non*`s, no?
>>
>> I think in this case the meaning of "empty" depends on the container itself.
>> Unsafe code that relies on "is_empty" would be the `NonEmpty<Container>` itself,
>> so it just needs to trust itself (as boundary of unsafe is module level).
>>
>> I think of almost all of the text above can be removed. The invariant of
>> `NonEmpty` can just be "`IsEmpty::is_empty` has returned true" and how it is
>> interpreted is up to the container type.
>
> Is `IsEmpty` actually useful? It needs to be implemented for every
> container we want to wrap with essentially the same boilerplate code,
> which looks a bit overkill to me. Couldn't we just do:
>
>   impl<T, C: Deref<Target = [T]>> NonEmpty<C>

Slice does not dereference to a slice, and arrays doesn't dereference to slices
either.

Best,
Gary

Re: [PATCH v3 2/2] rust: uaccess: use NonEmpty in strcpy_into_buf()

[Gary Guo]

On Thu Oct 15, 2026 at 2:06 PM BST, Alexandre Courbot wrote:
> On Wed Oct 14, 2026 at 5:30 PM JST, Lorenzo Delgado wrote:
> <...>
>> diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
>> index 5f6c4d7a1a51..1cc5c7580d3a 100644
>> --- a/rust/kernel/uaccess.rs
>> +++ b/rust/kernel/uaccess.rs
>> @@ -11,6 +11,7 @@
>>      error::Result,
>>      ffi::{c_char, c_void},
>>      fs::file,
>> +    non_empty::NonEmpty,
>>      prelude::*,
>>      ptr::KnownSize,
>>      transmute::{AsBytes, FromBytes},
>> @@ -383,20 +384,16 @@ pub fn read_all<A: Allocator>(mut self, buf: &mut Vec<u8, A>, flags: Flags) -> R
>>      /// Read a NUL-terminated string from userspace and return it.
>>      ///
>>      /// The string is read into `buf` and a NUL-terminator is added if the end of `buf` is reached.
>> -    /// Since there must be space to add a NUL-terminator, the buffer must not be empty. The
>> +    /// Since there must be space to add a NUL-terminator, `buf` is a [`NonEmpty`] slice. The
>>      /// returned `&CStr` points into `buf`.
>>      ///
>>      /// Fails with [`EFAULT`] if the read happens on a bad address (some data may have been
>>      /// copied).
>>      #[doc(alias = "strncpy_from_user")]
>> -    pub fn strcpy_into_buf<'buf>(self, buf: &'buf mut [u8]) -> Result<&'buf CStr> {
>> -        if buf.is_empty() {
>> -            return Err(EINVAL);
>> -        }
>> -
>> +    pub fn strcpy_into_buf<'buf>(self, buf: &'buf mut NonEmpty<[u8]>) -> Result<&'buf CStr> {
>
> I understand this was done per the v1 feedback, but won't it make it
> more tedious for users to call this method? (hard to tell as there are
> no users at the moment :))
>
> We could still benefit from `NonEmpty` if we replace the `buf.is_empty`
> by an attempt to build it and return `EINVAL` if that fails (what v1 did
> basically) and calling `last_mut` on it to remove the
> `unwrap_unchecked`.
>
> Of course if callers are expected to already work with a `NonEmpty` then
> that's fine, but otherwise my concern is that every one of them would
> have to manually build one to perform a check that this method can
> easily do.

I think it might also be reasonable to invoke errorenous behaviour here, by do a
`WARN` and just return c""?

Best,
Gary

Re: [PATCH v3 0/2] rust: add NonEmpty

[Gary Guo]

On Thu Oct 15, 2026 at 1:57 PM BST, Alexandre Courbot wrote:
> On Wed Oct 14, 2026 at 5:30 PM JST, Lorenzo Delgado wrote:
>> Some functions check that a container is not empty and later rely on it
>> through `unwrap_unchecked()`, `unwrap()` or an error path that can't be
>> taken. Gary suggested a non-empty type for this while reviewing Thorsten
>> Blum's uaccess patch [1], and Miguel filed it as #1262 [2].
>>
>> Patch 1 adds `NonEmpty<C>` in `kernel::non_empty`, with only what the
>> first user needs: slices, `try_from_mut()`, `from_mut()` for arrays
>> and `last_mut()`. Patch 2 makes `strcpy_into_buf()` take a
>> `NonEmpty<[u8]>`, which drops an `unsafe` block and the `EINVAL` case.
>>
>> Some notes:
>>
>> - `IsEmpty` is a safe trait. Unsafe code only relies on the
>>   implementations in `kernel::non_empty`, and the docs require the
>>   result not to change while the container is only reachable through a
>>   shared reference. If a generic accessor ever needs to rely on it, it
>>   can become an `unsafe trait` then.
>>
>> - Since APIs aren't added without users, other accessors (`first()`,
>>   `try_from_ref()`, ...) and containers are left for later. I plan to
>>   send follow-ups for `net/phy.rs` and `scatterlist.rs`, which both
>>   reject an empty slice and then rely on it.
>
> Having `last` without `first` is weird. Please add the minimal set that
> makes sense from the get-go, these are `pub` anyway so clippy won't
> complain if they are unused.
>
>>
>> - I put it in its own `kernel::non_empty` module. Would a broader home
>>   like `kernel::collections` be better?
>
> `kernel::types` sounds like a proper place? Or if we want a dedicated
> module, `kernel::slice` to match the standard library? `non_empty` is
> too ad-hoc and specific.

`kernel::types` is too generic and becomes a dumping ground of things. I wonder
if we should group things that are new types with just range restrictions
(together things like NonZero, Bounded, and perhaps eventually Range, NonNeg and
others).

Best,
Gary

Re: [PATCH v3 1/2] rust: non_empty: add NonEmpty

[Miguel Ojeda]

On Wed, Oct 14, 2026 at 4:25 PM Gary Guo <gary@garyguo.net> wrote:
>
> I think in this case the meaning of "empty" depends on the container itself.
> Unsafe code that relies on "is_empty" would be the `NonEmpty<Container>` itself,
> so it just needs to trust itself (as boundary of unsafe is module level).
>
> I think of almost all of the text above can be removed. The invariant of
> `NonEmpty` can just be "`IsEmpty::is_empty` has returned true" and how it is
> interpreted is up to the container type.

That sounds good to me, i.e. my main concern was the invariant text.

(Though we still mark things as `unsafe` even if private and within a
module, so if we are actually trusting it, shouldn't we mark it?)

Cheers,
Miguel

Re: [PATCH v3 0/2] rust: add NonEmpty

[Miguel Ojeda]

On Thu, Oct 15, 2026 at 4:08 PM Gary Guo <gary@garyguo.net> wrote:
>
> `kernel::types` is too generic and becomes a dumping ground of things. I wonder
> if we should group things that are new types with just range restrictions
> (together things like NonZero, Bounded, and perhaps eventually Range, NonNeg and
> others).

Yeah, we indeed had that problem, which is why nowadays as you say we
prefer things to be more split than less, because going into the
"grouping" direction is usually easier (i.e. it is easier to comer up
with better hierarchies when we have more types around, but it is
annoying to have a big file growing with no boundaries meanwhile).

Cheers,
Miguel

Re: [PATCH v3 1/2] rust: non_empty: add NonEmpty

[Alexandre Courbot]

On Thu Oct 15, 2026 at 11:02 PM JST, Gary Guo wrote:
> On Thu Oct 15, 2026 at 1:43 PM BST, Alexandre Courbot wrote:
>> On Wed Oct 14, 2026 at 11:25 PM JST, Gary Guo wrote:
>>> On Wed Oct 14, 2026 at 12:55 PM BST, Miguel Ojeda wrote:
>>>> On Wed, Oct 14, 2026 at 10:30 AM Lorenzo Delgado <lnsdev@proton.me> wrote:
>>>>>
>>>>> +/// A container that can report whether it holds any element.
>>>>> +///
>>>>> +/// The result must not change while the container is only reachable through a shared reference.
>>>>> +///
>>>>> +/// Unsafe code may only rely on the result for a container type whose implementation it trusts,
>>>>> +/// such as the ones in this module, since a safe implementation can return anything.
>>>>> +pub trait IsEmpty {
>>>>> +    /// Returns `true` if the container holds no element.
>>>>> +    fn is_empty(&self) -> bool;
>>>>> +}
>>>>
>>>> Do we need this to be a safe trait? If it is unsafe, then we can
>>>> simplify the invariant and more easily think about the type, like
>>>> other `Non*`s, no?
>>>
>>> I think in this case the meaning of "empty" depends on the container itself.
>>> Unsafe code that relies on "is_empty" would be the `NonEmpty<Container>` itself,
>>> so it just needs to trust itself (as boundary of unsafe is module level).
>>>
>>> I think of almost all of the text above can be removed. The invariant of
>>> `NonEmpty` can just be "`IsEmpty::is_empty` has returned true" and how it is
>>> interpreted is up to the container type.
>>
>> Is `IsEmpty` actually useful? It needs to be implemented for every
>> container we want to wrap with essentially the same boilerplate code,
>> which looks a bit overkill to me. Couldn't we just do:
>>
>>   impl<T, C: Deref<Target = [T]>> NonEmpty<C>
>
> Slice does not dereference to a slice, and arrays doesn't dereference to slices
> either.

How about `C: AsRef<[T]>` (would require an implementation for `KVec`)
or `C: Borrow<[T]>`?

Re: [PATCH v3 0/2] rust: add NonEmpty

[Alexandre Courbot]

On Thu Oct 15, 2026 at 11:11 PM JST, Miguel Ojeda wrote:
> On Thu, Oct 15, 2026 at 4:08 PM Gary Guo <gary@garyguo.net> wrote:
>>
>> `kernel::types` is too generic and becomes a dumping ground of things. I wonder
>> if we should group things that are new types with just range restrictions
>> (together things like NonZero, Bounded, and perhaps eventually Range, NonNeg and
>> others).
>
> Yeah, we indeed had that problem, which is why nowadays as you say we
> prefer things to be more split than less, because going into the
> "grouping" direction is usually easier (i.e. it is easier to comer up
> with better hierarchies when we have more types around, but it is
> annoying to have a big file growing with no boundaries meanwhile).

In this case maybe we can resurrect the `kernel::slice` module for this?