linux-kernel - Re: [PATCH v2] rust: alloc: Add doctest for `ArrayLayout`

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20241207013741.351172-1-jtostler1@gmail.com>
Date: Fri,  6 Dec 2024 17:37:38 -0800
From: Jimmy Ostler <jtostler1@...il.com>
To: dakr@...nel.org
Cc: a.hindborg@...nel.org,
	alex.gaynor@...il.com,
	aliceryhl@...gle.com,
	benno.lossin@...ton.me,
	bjorn3_gh@...tonmail.com,
	boqun.feng@...il.com,
	felipe_life@...e.com,
	gary@...yguo.net,
	jtostler1@...il.com,
	kernel@...entinobst.de,
	linux-kernel@...r.kernel.org,
	ojeda@...nel.org,
	rust-for-linux@...r.kernel.org,
	tmgross@...ch.edu
Subject: Re: [PATCH v2] rust: alloc: Add doctest for `ArrayLayout`

On Fri, Dec 6, 2024 at 2:57 AM Danilo Krummrich <dakr@...nel.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@...nel.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