Re: [PATCH 1/2] rust: alloc: extend safety requirements of Vec::set_len()

[Danilo Krummrich <dakr@kernel.org>]

On Sat, Mar 15, 2025 at 05:44:29PM +0000, Benno Lossin wrote:
> On Sat Mar 15, 2025 at 4:43 PM CET, Danilo Krummrich wrote:
> > Extend the safety requirements of set_len() to consider the case when
> > the new length is smaller than the old length.
> >
> > Fixes: 2aac4cd7dae3 ("rust: alloc: implement kernel `Vec` type")
> > Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> > ---
> >  rust/kernel/alloc/kvec.rs | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs
> > index ae9d072741ce..8540d9e2b717 100644
> > --- a/rust/kernel/alloc/kvec.rs
> > +++ b/rust/kernel/alloc/kvec.rs
> > @@ -189,7 +189,9 @@ pub fn len(&self) -> usize {
> >      ///
> >      /// - `new_len` must be less than or equal to [`Self::capacity`].
> >      /// - If `new_len` is greater than `self.len`, all elements within the interval
> > -    ///   [`self.len`,`new_len`) must be initialized.
> > +    ///   [`self.len`,`new_len`) must be initialized,
> > +    /// - if `new_len` is smaller than `self.len`, all elements within the interval
> > +    ///   [`new_len`, `self.len`) must either be dropped or copied and taken ownership of.
> 
> What is meant by "copied"? I agree with Tamir, this can probably just be
> "taken ownership of".

If not dropped in place, the only way to take ownership of the object is to copy
the corresponding memory, i.e. the caller can not take ownership of the object
at its current memory location, since this memory is owned by the vector.

AFAIU, when we speak of ownership, we mean ownership over a value or object, but
not ownership over the underlying memory.

Hence, I think it's important to mention that it needs to be either dropped in
place or copied in order to take ownership.

Even if we agree that "drop in place" is considered as "take ownership", we need
to differentiate between the special case of taking ownership where its dropped
in place and the requirement to copy the value and then take ownership.

> 
> With that change:
> 
> Reviewed-by: Benno Lossin <benno.lossin@proton.me>
> 
> ---
> Cheers,
> Benno
> 
> >      #[inline]
> >      pub unsafe fn set_len(&mut self, new_len: usize) {
> >          debug_assert!(new_len <= self.capacity());
> >
> > base-commit: 80e54e84911a923c40d7bee33a34c1b4be148d7a
> 
>