Re: [PATCH v5 6/7] rust: alloc: add Vec::remove

Benno Lossin
2025-05-07T11:44:33Z

On Fri May 2, 2025 at 3:19 PM CEST, Alice Ryhl wrote:
> This is needed by Rust Binder in the range allocator, and by upcoming
> GPU drivers during firmware initialization.
>
> Panics in the kernel are best avoided when possible, so an error is
> returned if the index is out of bounds. An error type is used rather
> than just returning Option<T> to let callers handle errors with ?.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

One follow-up comment below. With the `# Panics` section removed:

Reviewed-by: Benno Lossin <lossin@kernel.org>

[...]

> diff --git a/rust/kernel/alloc/kvec/errors.rs b/rust/kernel/alloc/kvec/errors.rs
> index 84c96ec5007ddc676283cbce07f4d670c3873c1e..06fe696e8bc6612a5e6aa2f6c28b685033acfa2f 100644
> --- a/rust/kernel/alloc/kvec/errors.rs
> +++ b/rust/kernel/alloc/kvec/errors.rs
> @@ -21,3 +21,18 @@ fn from(_: PushError<T>) -> Error {
> EINVAL
> }
> }
> +
> +/// Error type for [`Vec::remove`].
> +pub struct RemoveError;

Would it make sense as a follow-up to store the index that was accessed?

---
Cheers,
Benno

> +
> +impl Debug for RemoveError {
> + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
> + write!(f, "Index out of bounds")
> + }
> +}
> +
> +impl From<RemoveError> for Error {
> + fn from(_: RemoveError) -> Error {
> + EINVAL
> + }
> +}