Re: [PATCH v4 3/9] rust: file: add Rust abstraction for `struct file`

Benno Lossin
2024-02-05T12:33:48Z

On 2/2/24 11:55, Alice Ryhl wrote:
> From: Wedson Almeida Filho <wedsonaf@gmail.com>
>
> This abstraction makes it possible to manipulate the open files for a
> process. The new `File` struct wraps the C `struct file`. When accessing
> it using the smart pointer `ARef<File>`, the pointer will own a
> reference count to the file. When accessing it as `&File`, then the
> reference does not own a refcount, but the borrow checker will ensure
> that the reference count does not hit zero while the `&File` is live.
>
> Since this is intended to manipulate the open files of a process, we
> introduce an `fget` constructor that corresponds to the C `fget`
> method. In future patches, it will become possible to create a new fd in
> a process and bind it to a `File`. Rust Binder will use these to send
> fds from one process to another.
>
> We also provide a method for accessing the file's flags. Rust Binder
> will use this to access the flags of the Binder fd to check whether the
> non-blocking flag is set, which affects what the Binder ioctl does.
>
> This introduces a struct for the EBADF error type, rather than just
> using the Error type directly. This has two advantages:
> * `File::from_fd` returns a `Result<ARef<File>, BadFdError>`, which the
> compiler will represent as a single pointer, with null being an error.
> This is possible because the compiler understands that `BadFdError`
> has only one possible value, and it also understands that the
> `ARef<File>` smart pointer is guaranteed non-null.
> * Additionally, we promise to users of the method that the method can
> only fail with EBADF, which means that they can rely on this promise
> without having to inspect its implementation.
> That said, there are also two disadvantages:
> * Defining additional error types involves boilerplate.
> * The question mark operator will only utilize the `From` trait once,
> which prevents you from using the question mark operator on
> `BadFdError` in methods that return some third error type that the
> kernel `Error` is convertible into. (However, it works fine in methods
> that return `Error`.)
>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> Co-developed-by: Daniel Xu <dxu@dxuuu.xyz>
> Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
> Co-developed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> fs/file.c | 7 +
> rust/bindings/bindings_helper.h | 2 +
> rust/helpers.c | 7 +
> rust/kernel/file.rs | 249 ++++++++++++++++++++++++++++++++
> rust/kernel/lib.rs | 1 +
> 5 files changed, 266 insertions(+)
> create mode 100644 rust/kernel/file.rs

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

--
Cheers,
Benno