Re: [PATCH v3 07/10] rust: alloc: update `VecExt` to take allocation flags

[Benno Lossin]

On 28.03.24 02:36, Wedson Almeida Filho wrote:
> From: Wedson Almeida Filho <walmeida@xxxxxxxxxxxxx>
> 
> We also rename the methods by removing the `try_` prefix since the names
> are available due to our usage of the `no_global_oom_handling` config
> when building the `alloc` crate.
> 
> Reviewed-by: Boqun Feng <boqun.feng@xxxxxxxxx>
> Signed-off-by: Wedson Almeida Filho <walmeida@xxxxxxxxxxxxx>
> ---
>   rust/kernel/alloc/vec_ext.rs | 158 +++++++++++++++++++++++++++++++----
>   rust/kernel/error.rs         |  11 +--
>   rust/kernel/lib.rs           |   1 -
>   rust/kernel/str.rs           |   6 +-
>   rust/kernel/types.rs         |   4 +-
>   samples/rust/rust_minimal.rs |   6 +-
>   6 files changed, 152 insertions(+), 34 deletions(-)

Reviewed-by: Benno Lossin <benno.lossin@xxxxxxxxx>

[...]

>   impl<T> VecExt<T> for Vec<T> {
> -    fn try_with_capacity(capacity: usize) -> Result<Self, TryReserveError> {
> +    fn with_capacity(capacity: usize, flags: Flags) -> Result<Self, AllocError> {
>           let mut v = Vec::new();
> -        v.try_reserve(capacity)?;
> +        <Self as VecExt<_>>::reserve(&mut v, capacity, flags)?;
>           Ok(v)
>       }
> 
> -    fn try_push(&mut self, v: T) -> Result<(), TryReserveError> {
> -        if let Err(retry) = self.push_within_capacity(v) {
> -            self.try_reserve(1)?;
> -            let _ = self.push_within_capacity(retry);
> -        }
> +    fn push(&mut self, v: T, flags: Flags) -> Result<(), AllocError> {
> +        <Self as VecExt<_>>::reserve(self, 1, flags)?;
> +        let s = self.spare_capacity_mut();
> +        s[0].write(v);

I haven't checked the codegen, but this could result in an additional
bound check. We don't need to care about this now, but at some point we
should do a performance test. There are probably other things that I
have missed.

-- 
Cheers,
Benno

> +
> +        // SAFETY: We just initialised the first spare entry, so it is safe to increase the length
> +        // by 1. We also know that the new length is <= capacity because of the previous call to
> +        // `reserve` above.
> +        unsafe { self.set_len(self.len() + 1) };
>           Ok(())
>       }

[...]