Re: [PATCH WIP 6/8] rust: alloc: implement BoxExtAlloc

[Danilo Krummrich <dakr@redhat.com>]

On Wed, May 01, 2024 at 08:53:51AM +0000, Benno Lossin wrote:
> On 29.04.24 22:11, Danilo Krummrich wrote:
> > Signed-off-by: Danilo Krummrich <dakr@redhat.com>
> > ---
> >  rust/kernel/alloc/box_ext.rs | 40 +++++++++++++++++++++++++++++++++++-
> >  1 file changed, 39 insertions(+), 1 deletion(-)
> > 
> > diff --git a/rust/kernel/alloc/box_ext.rs b/rust/kernel/alloc/box_ext.rs
> > index 76653d6f4257..74d6bb24e965 100644
> > --- a/rust/kernel/alloc/box_ext.rs
> > +++ b/rust/kernel/alloc/box_ext.rs
> > @@ -2,7 +2,7 @@
> > 
> >  //! Extensions to [`Box`] for fallible allocations.
> > 
> > -use super::Flags;
> > +use super::{AllocatorWithFlags, Flags};
> >  use alloc::boxed::Box;
> >  use core::alloc::AllocError;
> >  use core::mem::MaybeUninit;
> > @@ -21,6 +21,19 @@ pub trait BoxExt<T>: Sized {
> >      fn new_uninit(flags: Flags) -> Result<Box<MaybeUninit<T>>, AllocError>;
> >  }
> > 
> > +/// Extensions to [`Box`].
> > +pub trait BoxExtAlloc<T, A: AllocatorWithFlags>: Sized {
> 
> I think it would be better to put all of this stuff into the `BoxExt`
> trait. Otherwise we will end up with even more extension traits...

That was my inital idea as well, but it wasn't quite obvious for me how to make
this happen.

I think for BoxExt<T>, it's rather easy, since we don't need an implementation
for Box<T, A>. What do you think about this [1]?

For VecExt<T>, I'm not quite sure how this can work though, since we'd need to
implement VecExt<T> for both Vec<T> and Vec<T, A>.

For instance, VecExt<T>::reserve() needs to be implemented for both Vec<T> and
Vec<T, A>, but VecExt<T>::with_capacity() can only be implemented for Vec<T> and
VecExt<T>::with_capacity_in() can only be implemented for Vec<T, A>. But we have
to implement all trait functions for both types.

Do I miss something here?

[1] https://gitlab.freedesktop.org/drm/nova/-/snippets/7784

> Ditto for `VecExtAlloc`.
> 
> -- 
> Cheers,
> Benno
> 
> > +    /// Allocates a new box using a custom allocator.
> > +    ///
> > +    /// The allocation may fail, in which case an error is returned.
> > +    fn new_in(x: T, alloc: A, flags: Flags) -> Result<Self, AllocError>;
> > +
> > +    /// Allocates a new uninitialised box using a custom allocator..
> > +    ///
> > +    /// The allocation may fail, in which case an error is returned.
> > +    fn new_uninit_in(alloc: A, flags: Flags) -> Result<Box<MaybeUninit<T>>, AllocError>;
> > +}
> > +
> 
>