* [PATCH v2] rust: alloc: Add doctest for `ArrayLayout` @ 2024-12-05 10:56 Jimmy Ostler 2024-12-05 11:35 ` Danilo Krummrich 0 siblings, 1 reply; 5+ messages in thread From: Jimmy Ostler @ 2024-12-05 10:56 UTC (permalink / raw) To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Danilo Krummrich, Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross, Filipe Xavier, Valentin Obst Cc: rust-for-linux, linux-kernel, Jimmy Ostler Add a rustdoc example and Kunit test to the `ArrayLayout` struct's `ArrayLayout::new()` function. Add an implementation of `From<LayoutError> for Error` for the `kernel::alloc::LayoutError`. This is necessary for the new test to compile. Change the `From` implementation on `core::alloc::LayoutError` to avoid collisions with `kernel::alloc::LayoutError`, and modify imports to explicitly import `kernel::alloc::LayoutError` instead. Suggested-by: Boqun Feng <boqun.feng@gmail.com> Link: https://github.com/Rust-for-Linux/linux/issues/1131 Signed-off-by: Jimmy Ostler <jtostler1@gmail.com> --- v1: https://lore.kernel.org/lkml/20241203051843.291729-1-jtostler1@gmail.com/T/#u v1 -> v2 changes: - Add third assert where length is smaller but still overflows - Remove rustdoc markdown codeblock languge signifier - Change tests to return results using `?` instead of panic - Remove `#[derive(Debug)]` for `LayoutError` - Add `From<LayoutError> for Error` implementation --- rust/kernel/alloc/layout.rs | 19 +++++++++++++++++++ rust/kernel/error.rs | 13 ++++++++++--- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/rust/kernel/alloc/layout.rs b/rust/kernel/alloc/layout.rs index 4b3cd7fdc816..0e053dcc7941 100644 --- a/rust/kernel/alloc/layout.rs +++ b/rust/kernel/alloc/layout.rs @@ -43,6 +43,25 @@ pub const fn empty() -> Self { /// # Errors /// /// When `len * size_of::<T>()` overflows or when `len * size_of::<T>() > isize::MAX`. + /// + /// # Examples + /// + /// ``` + /// # use kernel::alloc::layout::{ArrayLayout, LayoutError}; + /// let layout = ArrayLayout::<i32>::new(15)?; + /// assert_eq!(layout.len(), 15); + /// + /// // Errors because `len * size_of::<T>()` overflows + /// let layout = ArrayLayout::<i32>::new(isize::MAX as usize); + /// assert!(layout.is_err()); + /// + /// // Errors because `len * size_of::<i32>() > isize::MAX`, + /// // even though `len < isize::MAX` + /// let layout = ArrayLayout::<i32>::new(isize::MAX as usize / 2); + /// assert!(layout.is_err()); + /// + /// # Ok::<(), Error>(()) + /// ``` pub const fn new(len: usize) -> Result<Self, LayoutError> { match len.checked_mul(core::mem::size_of::<T>()) { Some(size) if size <= ISIZE_MAX => { diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs index 52c502432447..ac8526140d7a 100644 --- a/rust/kernel/error.rs +++ b/rust/kernel/error.rs @@ -4,9 +4,10 @@ //! //! C header: [`include/uapi/asm-generic/errno-base.h`](srctree/include/uapi/asm-generic/errno-base.h) -use crate::{alloc::AllocError, str::CStr}; - -use core::alloc::LayoutError; +use crate::{ + alloc::{layout::LayoutError, AllocError}, + str::CStr, +}; use core::fmt; use core::num::NonZeroI32; @@ -223,6 +224,12 @@ fn from(_: LayoutError) -> Error { } } +impl From<core::alloc::LayoutError> for Error { + fn from(_: core::alloc::LayoutError) -> Error { + code::ENOMEM + } +} + impl From<core::fmt::Error> for Error { fn from(_: core::fmt::Error) -> Error { code::EINVAL base-commit: 1dc707e647bc919834eff9636c8d00b78c782545 -- 2.47.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] rust: alloc: Add doctest for `ArrayLayout` 2024-12-05 10:56 [PATCH v2] rust: alloc: Add doctest for `ArrayLayout` Jimmy Ostler @ 2024-12-05 11:35 ` Danilo Krummrich 2024-12-05 23:04 ` Jimmy Ostler 0 siblings, 1 reply; 5+ messages in thread From: Danilo Krummrich @ 2024-12-05 11:35 UTC (permalink / raw) To: Jimmy Ostler Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross, Filipe Xavier, Valentin Obst, rust-for-linux, linux-kernel Hi Jimmy, Thanks for the patch! On Thu, Dec 05, 2024 at 02:56:27AM -0800, Jimmy Ostler wrote: > Add a rustdoc example and Kunit test to the `ArrayLayout` struct's > `ArrayLayout::new()` function. > > Add an implementation of `From<LayoutError> for Error` for the > `kernel::alloc::LayoutError`. This is necessary for the new test to > compile. Please split this into a separate patch. > > Change the `From` implementation on `core::alloc::LayoutError` to avoid > collisions with `kernel::alloc::LayoutError`, and modify imports to > explicitly import `kernel::alloc::LayoutError` instead. > > Suggested-by: Boqun Feng <boqun.feng@gmail.com> > Link: https://github.com/Rust-for-Linux/linux/issues/1131 > Signed-off-by: Jimmy Ostler <jtostler1@gmail.com> > --- > v1: https://lore.kernel.org/lkml/20241203051843.291729-1-jtostler1@gmail.com/T/#u > v1 -> v2 changes: > - Add third assert where length is smaller but still overflows > - Remove rustdoc markdown codeblock languge signifier > - Change tests to return results using `?` instead of panic > - Remove `#[derive(Debug)]` for `LayoutError` > - Add `From<LayoutError> for Error` implementation > --- > rust/kernel/alloc/layout.rs | 19 +++++++++++++++++++ > rust/kernel/error.rs | 13 ++++++++++--- > 2 files changed, 29 insertions(+), 3 deletions(-) > > diff --git a/rust/kernel/alloc/layout.rs b/rust/kernel/alloc/layout.rs > index 4b3cd7fdc816..0e053dcc7941 100644 > --- a/rust/kernel/alloc/layout.rs > +++ b/rust/kernel/alloc/layout.rs > @@ -43,6 +43,25 @@ pub const fn empty() -> Self { > /// # Errors > /// > /// When `len * size_of::<T>()` overflows or when `len * size_of::<T>() > isize::MAX`. > + /// > + /// # Examples > + /// > + /// ``` > + /// # use kernel::alloc::layout::{ArrayLayout, LayoutError}; > + /// let layout = ArrayLayout::<i32>::new(15)?; > + /// assert_eq!(layout.len(), 15); > + /// > + /// // Errors because `len * size_of::<T>()` overflows > + /// let layout = ArrayLayout::<i32>::new(isize::MAX as usize); > + /// assert!(layout.is_err()); > + /// > + /// // Errors because `len * size_of::<i32>() > isize::MAX`, > + /// // even though `len < isize::MAX` > + /// let layout = ArrayLayout::<i32>::new(isize::MAX as usize / 2); > + /// assert!(layout.is_err()); > + /// > + /// # Ok::<(), Error>(()) > + /// ``` > pub const fn new(len: usize) -> Result<Self, LayoutError> { > match len.checked_mul(core::mem::size_of::<T>()) { > Some(size) if size <= ISIZE_MAX => { > diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs > index 52c502432447..ac8526140d7a 100644 > --- a/rust/kernel/error.rs > +++ b/rust/kernel/error.rs > @@ -4,9 +4,10 @@ > //! > //! C header: [`include/uapi/asm-generic/errno-base.h`](srctree/include/uapi/asm-generic/errno-base.h) > > -use crate::{alloc::AllocError, str::CStr}; > - > -use core::alloc::LayoutError; > +use crate::{ > + alloc::{layout::LayoutError, AllocError}, > + str::CStr, > +}; I think this part of the change would be enough, since we don't make use of the `From` implementation of `core::alloc::LayoutError` anywhere. I think we can add it (again), once it's needed. > > use core::fmt; > use core::num::NonZeroI32; > @@ -223,6 +224,12 @@ fn from(_: LayoutError) -> Error { > } > } > > +impl From<core::alloc::LayoutError> for Error { > + fn from(_: core::alloc::LayoutError) -> Error { > + code::ENOMEM > + } > +} > + > impl From<core::fmt::Error> for Error { > fn from(_: core::fmt::Error) -> Error { > code::EINVAL > > base-commit: 1dc707e647bc919834eff9636c8d00b78c782545 > -- > 2.47.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] rust: alloc: Add doctest for `ArrayLayout` 2024-12-05 11:35 ` Danilo Krummrich @ 2024-12-05 23:04 ` Jimmy Ostler 2024-12-06 10:57 ` Danilo Krummrich 0 siblings, 1 reply; 5+ messages in thread From: Jimmy Ostler @ 2024-12-05 23:04 UTC (permalink / raw) To: dakr Cc: a.hindborg, alex.gaynor, aliceryhl, benno.lossin, bjorn3_gh, boqun.feng, felipe_life, gary, jtostler1, kernel, linux-kernel, ojeda, rust-for-linux, tmgross On Thu, Dec 5, 2024 at 3:35 AM Danilo Krummrich <dakr@kernel.org> wrote: > Hi Jimmy, > > Thanks for the patch! > > On Thu, Dec 05, 2024 at 02:56:27AM -0800, Jimmy Ostler wrote: > > Add a rustdoc example and Kunit test to the `ArrayLayout` struct's > > `ArrayLayout::new()` function. > > > > Add an implementation of `From<LayoutError> for Error` for the > > `kernel::alloc::LayoutError`. This is necessary for the new test to > > compile. > > Please split this into a separate patch. Got it, the next version will be split into separate patches. > > diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs > > index 52c502432447..ac8526140d7a 100644 > > --- a/rust/kernel/error.rs > > +++ b/rust/kernel/error.rs > > @@ -4,9 +4,10 @@ > > //! > > //! C header: [`include/uapi/asm-generic/errno-base.h`](srctree/include/uapi/asm-generic/errno-base.h) > > > > -use crate::{alloc::AllocError, str::CStr}; > > - > > -use core::alloc::LayoutError; > > +use crate::{ > > + alloc::{layout::LayoutError, AllocError}, > > + str::CStr, > > +}; > > I think this part of the change would be enough, since we don't make use of the > `From` implementation of `core::alloc::LayoutError` anywhere. > > I think we can add it (again), once it's needed. Okay, that makes sense. It is still used in the documentation for the macro `stack_try_pin_init`, and it is hidden and not used as a test, but it would probably be prudent to change that for consistency, as `Box::new` no longer returns `core::alloc::AllocError`. I can add that into the v3 patchset, unless there's some reason we should leave it. > > > > use core::fmt; > > use core::num::NonZeroI32; > > @@ -223,6 +224,12 @@ fn from(_: LayoutError) -> Error { > > } > > } > > > > +impl From<core::alloc::LayoutError> for Error { > > + fn from(_: core::alloc::LayoutError) -> Error { > > + code::ENOMEM > > + } > > +} > > + > > impl From<core::fmt::Error> for Error { > > fn from(_: core::fmt::Error) -> Error { > > code::EINVAL > > > > base-commit: 1dc707e647bc919834eff9636c8d00b78c782545 > > -- > > 2.47.1 > > Thanks! Jimmy Ostler ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] rust: alloc: Add doctest for `ArrayLayout` 2024-12-05 23:04 ` Jimmy Ostler @ 2024-12-06 10:57 ` Danilo Krummrich 2024-12-07 1:37 ` Jimmy Ostler 0 siblings, 1 reply; 5+ messages in thread From: Danilo Krummrich @ 2024-12-06 10:57 UTC (permalink / raw) To: Jimmy Ostler Cc: a.hindborg, alex.gaynor, aliceryhl, benno.lossin, bjorn3_gh, boqun.feng, felipe_life, gary, kernel, linux-kernel, ojeda, rust-for-linux, tmgross On Thu, Dec 05, 2024 at 03:04:28PM -0800, Jimmy Ostler wrote: > On Thu, Dec 5, 2024 at 3:35 AM Danilo Krummrich <dakr@kernel.org> wrote: > > > Hi Jimmy, > > > > Thanks for the patch! > > > > On Thu, Dec 05, 2024 at 02:56:27AM -0800, Jimmy Ostler wrote: > > > Add a rustdoc example and Kunit test to the `ArrayLayout` struct's > > > `ArrayLayout::new()` function. > > > > > > Add an implementation of `From<LayoutError> for Error` for the > > > `kernel::alloc::LayoutError`. This is necessary for the new test to > > > compile. > > > > Please split this into a separate patch. > > Got it, the next version will be split into separate patches. > > > > diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs > > > index 52c502432447..ac8526140d7a 100644 > > > --- a/rust/kernel/error.rs > > > +++ b/rust/kernel/error.rs > > > @@ -4,9 +4,10 @@ > > > //! > > > //! C header: [`include/uapi/asm-generic/errno-base.h`](srctree/include/uapi/asm-generic/errno-base.h) > > > > > > -use crate::{alloc::AllocError, str::CStr}; > > > - > > > -use core::alloc::LayoutError; > > > +use crate::{ > > > + alloc::{layout::LayoutError, AllocError}, > > > + str::CStr, > > > +}; > > > > I think this part of the change would be enough, since we don't make use of the > > `From` implementation of `core::alloc::LayoutError` anywhere. > > > > I think we can add it (again), once it's needed. > > Okay, that makes sense. It is still used in the documentation for the > macro `stack_try_pin_init`, and it is hidden and not used as a test, but > it would probably be prudent to change that for consistency, as > `Box::new` no longer returns `core::alloc::AllocError`. It seems you're confusing `LayoutError` and `AllocError` here. This is about the former. But you're right that `AllocError` can be fixed up in a few places too. Do you plan to send a patch for this as well? > I can add that into the v3 patchset, unless there's some reason we > should leave it. > > > > > > > use core::fmt; > > > use core::num::NonZeroI32; > > > @@ -223,6 +224,12 @@ fn from(_: LayoutError) -> Error { > > > } > > > } > > > > > > +impl From<core::alloc::LayoutError> for Error { > > > + fn from(_: core::alloc::LayoutError) -> Error { > > > + code::ENOMEM > > > + } > > > +} > > > + > > > impl From<core::fmt::Error> for Error { > > > fn from(_: core::fmt::Error) -> Error { > > > code::EINVAL > > > > > > base-commit: 1dc707e647bc919834eff9636c8d00b78c782545 > > > -- > > > 2.47.1 > > > > > Thanks! > > Jimmy Ostler ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] rust: alloc: Add doctest for `ArrayLayout` 2024-12-06 10:57 ` Danilo Krummrich @ 2024-12-07 1:37 ` Jimmy Ostler 0 siblings, 0 replies; 5+ messages in thread From: Jimmy Ostler @ 2024-12-07 1:37 UTC (permalink / raw) To: dakr Cc: a.hindborg, alex.gaynor, aliceryhl, benno.lossin, bjorn3_gh, boqun.feng, felipe_life, gary, jtostler1, kernel, linux-kernel, ojeda, rust-for-linux, tmgross On Fri, Dec 6, 2024 at 2:57 AM Danilo Krummrich <dakr@kernel.org> wrote: > > On Thu, Dec 05, 2024 at 03:04:28PM -0800, Jimmy Ostler wrote: > > On Thu, Dec 5, 2024 at 3:35 AM Danilo Krummrich <dakr@kernel.org> wrote: > > > > > Hi Jimmy, > > > > > > Thanks for the patch! > > > > > > On Thu, Dec 05, 2024 at 02:56:27AM -0800, Jimmy Ostler wrote: > > > > Add a rustdoc example and Kunit test to the `ArrayLayout` struct's > > > > `ArrayLayout::new()` function. > > > > > > > > Add an implementation of `From<LayoutError> for Error` for the > > > > `kernel::alloc::LayoutError`. This is necessary for the new test to > > > > compile. > > > > > > Please split this into a separate patch. > > > > Got it, the next version will be split into separate patches. > > > > > > diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs > > > > index 52c502432447..ac8526140d7a 100644 > > > > --- a/rust/kernel/error.rs > > > > +++ b/rust/kernel/error.rs > > > > @@ -4,9 +4,10 @@ > > > > //! > > > > //! C header: [`include/uapi/asm-generic/errno-base.h`](srctree/include/uapi/asm-generic/errno-base.h) > > > > > > > > -use crate::{alloc::AllocError, str::CStr}; > > > > - > > > > -use core::alloc::LayoutError; > > > > +use crate::{ > > > > + alloc::{layout::LayoutError, AllocError}, > > > > + str::CStr, > > > > +}; > > > > > > I think this part of the change would be enough, since we don't make use of the > > > `From` implementation of `core::alloc::LayoutError` anywhere. > > > > > > I think we can add it (again), once it's needed. > > > > Okay, that makes sense. It is still used in the documentation for the > > macro `stack_try_pin_init`, and it is hidden and not used as a test, but > > it would probably be prudent to change that for consistency, as > > `Box::new` no longer returns `core::alloc::AllocError`. > > It seems you're confusing `LayoutError` and `AllocError` here. > > This is about the former. But you're right that `AllocError` can be fixed up in > a few places too. Oops, you're totally right, I noticed it when grepping through error types in documentation, and mixed them up. > Do you plan to send a patch for this as well? Yes, I've already got a fix for that locally. > > I can add that into the v3 patchset, unless there's some reason we > > should leave it. > > > > > > > > > > use core::fmt; > > > > use core::num::NonZeroI32; > > > > @@ -223,6 +224,12 @@ fn from(_: LayoutError) -> Error { > > > > } > > > > } > > > > > > > > +impl From<core::alloc::LayoutError> for Error { > > > > + fn from(_: core::alloc::LayoutError) -> Error { > > > > + code::ENOMEM > > > > + } > > > > +} > > > > + > > > > impl From<core::fmt::Error> for Error { > > > > fn from(_: core::fmt::Error) -> Error { > > > > code::EINVAL > > > > > > > > base-commit: 1dc707e647bc919834eff9636c8d00b78c782545 > > > > -- > > > > 2.47.1 > > > > > > > > Thanks! > > > > Jimmy Ostler ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-12-07 1:37 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-12-05 10:56 [PATCH v2] rust: alloc: Add doctest for `ArrayLayout` Jimmy Ostler 2024-12-05 11:35 ` Danilo Krummrich 2024-12-05 23:04 ` Jimmy Ostler 2024-12-06 10:57 ` Danilo Krummrich 2024-12-07 1:37 ` Jimmy Ostler
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox