mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [RFC PATCH v1 1/2] rust: add NonEmptySlice
       [not found] <20261011191540.56199-1-laniel_francis@npnm.eu>
@ 2026-10-11 19:15 ` Francis Laniel
  2026-10-12 14:36   ` Alexandre Courbot
  2026-10-11 19:15 ` [RFC PATCH v1 2/2] rust: uaccess: Use slice::NonEmptySlice in strcpy_into_buf() Francis Laniel
  1 sibling, 1 reply; 4+ messages in thread
From: Francis Laniel @ 2026-10-11 19:15 UTC (permalink / raw)
  To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
	Alexandre Courbot, Onur Özkan, Asahi Lina, Joel Fernandes,
	Alvin Sun, Francis Laniel
  Cc: Thorsten Blum, linux-kernel, rust-for-linux

Add NonEmptySlice<T>, a [T] wrapper that is guaranteed to contain at
least one element.
This lets methods such as last_mut() return a plain reference instead
of an Option, so callers that have already checked for emptiness no
longer need unwrap() or unwrap_unchecked().

Suggested-by: Gary Guo <gary@garyguo.net>
Signed-off-by: Francis Laniel <laniel_francis@npnm.eu>
---
 rust/kernel/lib.rs   |  1 +
 rust/kernel/slice.rs | 76 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+)
 create mode 100644 rust/kernel/slice.rs

diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 7225abc64084..8cc01aceb3b2 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -130,6 +130,7 @@
 #[cfg(CONFIG_RUST_SERIAL_DEV_BUS_ABSTRACTIONS)]
 pub mod serdev;
 pub mod sizes;
+pub mod slice;
 #[cfg(CONFIG_SOC_BUS)]
 pub mod soc;
 #[doc(hidden)]
diff --git a/rust/kernel/slice.rs b/rust/kernel/slice.rs
new file mode 100644
index 000000000000..7e467cb952c3
--- /dev/null
+++ b/rust/kernel/slice.rs
@@ -0,0 +1,76 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Slice abstractions to ease development.
+
+use core::ops::{Deref, DerefMut};
+
+/// A slice that is guaranteed to contain at least one element.
+///
+/// # Invariants
+///
+/// `self.0` is not empty.
+#[repr(transparent)]
+pub struct NonEmptySlice<T>([T]);
+
+impl<T> NonEmptySlice<T> {
+    /// Returns `None` if `s` is empty.
+    pub fn new(s: &mut [T]) -> Option<&Self> {
+        if s.is_empty() {
+            return None;
+        }
+        // SAFETY: `Self` is `repr(transparent)` over `[T]`, so the cast keeps the slice length.
+        // INVARIANT: `s` is not empty, checked above.
+        Some(unsafe { &*(core::ptr::from_mut(s) as *const Self) })
+    }
+
+    /// Returns `None` if `s` is empty.
+    pub fn new_mut(s: &mut [T]) -> Option<&mut Self> {
+        if s.is_empty() {
+            return None;
+        }
+        // SAFETY: `Self` is `repr(transparent)` over `[T]`, so the cast keeps the slice length.
+        // INVARIANT: `s` is not empty, checked above.
+        Some(unsafe { &mut *(core::ptr::from_mut(s) as *mut Self) })
+    }
+
+    /// Returns a reference to the first element.
+    pub fn first(&mut self) -> &T {
+        // SAFETY: By the type invariant, `len >= 1`, so `0` is in bounds.
+        unsafe { self.0.get_unchecked(0) }
+    }
+
+    /// Returns a mutable reference to the first element.
+    pub fn first_mut(&mut self) -> &mut T {
+        // SAFETY: By the type invariant, `len >= 1`, so `0` is in bounds.
+        unsafe { self.0.get_unchecked_mut(0) }
+    }
+
+    /// Returns a reference to the last element.
+    pub fn last(&mut self) -> &T {
+        let last = self.0.len() - 1;
+        // SAFETY: By the type invariant, `len >= 1`, so `last` is in bounds.
+        unsafe { self.0.get_unchecked(last) }
+    }
+
+    /// Returns a mutable reference to the last element.
+    pub fn last_mut(&mut self) -> &mut T {
+        let last = self.0.len() - 1;
+        // SAFETY: By the type invariant, `len >= 1`, so `last` is in bounds.
+        unsafe { self.0.get_unchecked_mut(last) }
+    }
+}
+
+impl<T> Deref for NonEmptySlice<T> {
+    type Target = [T];
+
+    fn deref(&self) -> &[T] {
+        &self.0
+    }
+}
+
+impl<T> DerefMut for NonEmptySlice<T> {
+    // Mutating elements through `&mut [T]` can't change the length, so the invariant holds.
+    fn deref_mut(&mut self) -> &mut [T] {
+        &mut self.0
+    }
+}
-- 
2.47.3


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [RFC PATCH v1 2/2] rust: uaccess: Use slice::NonEmptySlice in strcpy_into_buf()
       [not found] <20261011191540.56199-1-laniel_francis@npnm.eu>
  2026-10-11 19:15 ` [RFC PATCH v1 1/2] rust: add NonEmptySlice Francis Laniel
@ 2026-10-11 19:15 ` Francis Laniel
  1 sibling, 0 replies; 4+ messages in thread
From: Francis Laniel @ 2026-10-11 19:15 UTC (permalink / raw)
  To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
	Alexandre Courbot, Onur Özkan, Greg Kroah-Hartman,
	Francis Laniel, Timur Tabi
  Cc: Thorsten Blum, rust-for-linux, linux-kernel

This removes an unsafe block in strcpy_into_buf(), the unsafe
block now lies in last_mut() in NonEmptySlice.

Signed-off-by: Francis Laniel <laniel_francis@npnm.eu>
Suggested-by: Gary Guo <gary@garyguo.net>
---
 rust/kernel/uaccess.rs | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
index 5f6c4d7a1a51..b6bf9074d67a 100644
--- a/rust/kernel/uaccess.rs
+++ b/rust/kernel/uaccess.rs
@@ -13,6 +13,7 @@
     fs::file,
     prelude::*,
     ptr::KnownSize,
+    slice::NonEmptySlice,
     transmute::{AsBytes, FromBytes},
 };
 use core::mem::{size_of, MaybeUninit};
@@ -390,13 +391,11 @@ pub fn read_all<A: Allocator>(mut self, buf: &mut Vec<u8, A>, flags: Flags) -> R
     /// 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);
-        }
+        let buf = NonEmptySlice::new_mut(buf).ok_or(EINVAL)?;
 
         // 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(&mut **buf) as *mut [MaybeUninit<u8>]) };
 
         // We never read more than `self.length` bytes.
         if dst.len() > self.length {
@@ -422,9 +421,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.47.3


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC PATCH v1 1/2] rust: add NonEmptySlice
  2026-10-11 19:15 ` [RFC PATCH v1 1/2] rust: add NonEmptySlice Francis Laniel
@ 2026-10-12 14:36   ` Alexandre Courbot
  2026-10-12 15:11     ` Francis Laniel
  0 siblings, 1 reply; 4+ messages in thread
From: Alexandre Courbot @ 2026-10-12 14:36 UTC (permalink / raw)
  To: Francis Laniel
  Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
	Onur Özkan, Asahi Lina, Joel Fernandes, Alvin Sun,
	Thorsten Blum, linux-kernel, rust-for-linux

Looks like we are converging towards the series sent by Lorenzo, but
wanted to share a couple a quick comments nonetheless:

On Mon Oct 12, 2026 at 4:15 AM JST, Francis Laniel wrote:
> Add NonEmptySlice<T>, a [T] wrapper that is guaranteed to contain at
> least one element.
> This lets methods such as last_mut() return a plain reference instead
> of an Option, so callers that have already checked for emptiness no
> longer need unwrap() or unwrap_unchecked().
>
> Suggested-by: Gary Guo <gary@garyguo.net>
> Signed-off-by: Francis Laniel <laniel_francis@npnm.eu>
> ---
>  rust/kernel/lib.rs   |  1 +
>  rust/kernel/slice.rs | 76 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 77 insertions(+)
>  create mode 100644 rust/kernel/slice.rs
>
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 7225abc64084..8cc01aceb3b2 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -130,6 +130,7 @@
>  #[cfg(CONFIG_RUST_SERIAL_DEV_BUS_ABSTRACTIONS)]
>  pub mod serdev;
>  pub mod sizes;
> +pub mod slice;
>  #[cfg(CONFIG_SOC_BUS)]
>  pub mod soc;
>  #[doc(hidden)]
> diff --git a/rust/kernel/slice.rs b/rust/kernel/slice.rs
> new file mode 100644
> index 000000000000..7e467cb952c3
> --- /dev/null
> +++ b/rust/kernel/slice.rs
> @@ -0,0 +1,76 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Slice abstractions to ease development.
> +
> +use core::ops::{Deref, DerefMut};
> +
> +/// A slice that is guaranteed to contain at least one element.
> +///
> +/// # Invariants
> +///
> +/// `self.0` is not empty.
> +#[repr(transparent)]
> +pub struct NonEmptySlice<T>([T]);
> +
> +impl<T> NonEmptySlice<T> {
> +    /// Returns `None` if `s` is empty.

This is not a standard doccomment. Doccomments first start by stating
what the function does before explaining the return value.

> +    pub fn new(s: &mut [T]) -> Option<&Self> {

It looks strange to take a `&mut` and return a `&Self`. There should be
no reason to take a mutable reference here, and you can use
`ptr::from_ref` to create the reference.

> +        if s.is_empty() {
> +            return None;
> +        }
> +        // SAFETY: `Self` is `repr(transparent)` over `[T]`, so the cast keeps the slice length.
> +        // INVARIANT: `s` is not empty, checked above.
> +        Some(unsafe { &*(core::ptr::from_mut(s) as *const Self) })
> +    }
> +
> +    /// Returns `None` if `s` is empty.
> +    pub fn new_mut(s: &mut [T]) -> Option<&mut Self> {
> +        if s.is_empty() {
> +            return None;
> +        }
> +        // SAFETY: `Self` is `repr(transparent)` over `[T]`, so the cast keeps the slice length.
> +        // INVARIANT: `s` is not empty, checked above.
> +        Some(unsafe { &mut *(core::ptr::from_mut(s) as *mut Self) })
> +    }
> +
> +    /// Returns a reference to the first element.
> +    pub fn first(&mut self) -> &T {

Same here, `first` and `last` should take a non-mutable reference as
their standard library counterparts do. They do not allow to mutate the
slice.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC PATCH v1 1/2] rust: add NonEmptySlice
  2026-10-12 14:36   ` Alexandre Courbot
@ 2026-10-12 15:11     ` Francis Laniel
  0 siblings, 0 replies; 4+ messages in thread
From: Francis Laniel @ 2026-10-12 15:11 UTC (permalink / raw)
  To: Alexandre Courbot
  Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
	Onur Özkan, Asahi Lina, Joel Fernandes, Alvin Sun,
	Thorsten Blum, linux-kernel, rust-for-linux

Hi,

Le lundi 12 octobre 2026, 17:36:41 heure d’été d’Europe de l’Est Alexandre Courbot a écrit :
> Looks like we are converging towards the series sent by Lorenzo, but
> wanted to share a couple a quick comments nonetheless:

Thanks for the feedback!

> On Mon Oct 12, 2026 at 4:15 AM JST, Francis Laniel wrote:
> > Add NonEmptySlice<T>, a [T] wrapper that is guaranteed to contain at
> > least one element.
> > This lets methods such as last_mut() return a plain reference instead
> > of an Option, so callers that have already checked for emptiness no
> > longer need unwrap() or unwrap_unchecked().
> >
> > Suggested-by: Gary Guo <gary@garyguo.net>
> > Signed-off-by: Francis Laniel <laniel_francis@npnm.eu>
> > ---
> >  rust/kernel/lib.rs   |  1 +
> >  rust/kernel/slice.rs | 76 ++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 77 insertions(+)
> >  create mode 100644 rust/kernel/slice.rs
> >
> > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> > index 7225abc64084..8cc01aceb3b2 100644
> > --- a/rust/kernel/lib.rs
> > +++ b/rust/kernel/lib.rs
> > @@ -130,6 +130,7 @@
> >  #[cfg(CONFIG_RUST_SERIAL_DEV_BUS_ABSTRACTIONS)]
> >  pub mod serdev;
> >  pub mod sizes;
> > +pub mod slice;
> >  #[cfg(CONFIG_SOC_BUS)]
> >  pub mod soc;
> >  #[doc(hidden)]
> > diff --git a/rust/kernel/slice.rs b/rust/kernel/slice.rs
> > new file mode 100644
> > index 000000000000..7e467cb952c3
> > --- /dev/null
> > +++ b/rust/kernel/slice.rs
> > @@ -0,0 +1,76 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +//! Slice abstractions to ease development.
> > +
> > +use core::ops::{Deref, DerefMut};
> > +
> > +/// A slice that is guaranteed to contain at least one element.
> > +///
> > +/// # Invariants
> > +///
> > +/// `self.0` is not empty.
> > +#[repr(transparent)]
> > +pub struct NonEmptySlice<T>([T]);
> > +
> > +impl<T> NonEmptySlice<T> {
> > +    /// Returns `None` if `s` is empty.
> 
> This is not a standard doccomment. Doccomments first start by stating
> what the function does before explaining the return value.
> 
> > +    pub fn new(s: &mut [T]) -> Option<&Self> {
> 
> It looks strange to take a `&mut` and return a `&Self`. There should be
> no reason to take a mutable reference here, and you can use
> `ptr::from_ref` to create the reference.

Indeed, this sounds weird! I will keep it consistant for future contributions.

> > +        if s.is_empty() {
> > +            return None;
> > +        }
> > +        // SAFETY: `Self` is `repr(transparent)` over `[T]`, so the cast keeps the slice length.
> > +        // INVARIANT: `s` is not empty, checked above.
> > +        Some(unsafe { &*(core::ptr::from_mut(s) as *const Self) })
> > +    }
> > +
> > +    /// Returns `None` if `s` is empty.
> > +    pub fn new_mut(s: &mut [T]) -> Option<&mut Self> {
> > +        if s.is_empty() {
> > +            return None;
> > +        }
> > +        // SAFETY: `Self` is `repr(transparent)` over `[T]`, so the cast keeps the slice length.
> > +        // INVARIANT: `s` is not empty, checked above.
> > +        Some(unsafe { &mut *(core::ptr::from_mut(s) as *mut Self) })
> > +    }
> > +
> > +    /// Returns a reference to the first element.
> > +    pub fn first(&mut self) -> &T {
> 
> Same here, `first` and `last` should take a non-mutable reference as
> their standard library counterparts do. They do not allow to mutate the
> slice.
> 

Best regards,



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-12 15:28 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20261011191540.56199-1-laniel_francis@npnm.eu>
2026-10-11 19:15 ` [RFC PATCH v1 1/2] rust: add NonEmptySlice Francis Laniel
2026-10-12 14:36   ` Alexandre Courbot
2026-10-12 15:11     ` Francis Laniel
2026-10-11 19:15 ` [RFC PATCH v1 2/2] rust: uaccess: Use slice::NonEmptySlice in strcpy_into_buf() Francis Laniel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®