On 7/20/23 12:28, Alice Ryhl wrote:
From: Wedson Almeida Filho <walmeida@xxxxxxxxxxxxx> Using these bindings it becomes possible to access files from drivers written in Rust. This patch only adds support for accessing the flags, and for managing the refcount of the file. Signed-off-by: Wedson Almeida Filho <walmeida@xxxxxxxxxxxxx> Co-Developed-by: Daniel Xu <dxu@xxxxxxxxx> Signed-off-by: Daniel Xu <dxu@xxxxxxxxx> Co-Developed-by: Alice Ryhl <aliceryhl@xxxxxxxxxx> Signed-off-by: Alice Ryhl <aliceryhl@xxxxxxxxxx> --- In this patch, I am defining an error type called `BadFdError`. I'd like your thoughts on doing it this way vs just using the normal `Error` type. Pros: * The type system makes it clear that the function can only fail with EBADF, and that no other errors are possible. * Since the compiler knows that `ARef<Self>` cannot be null and that `BadFdError` has only one possible value, the return type of `File::from_fd` is represented as a pointer with null being an error. Cons: * Defining additional error types involves boilerplate. * The return type becomes a tagged union, making it larger than a pointer.
These two are kinda passable, as a `impl_null_ptr_err` macro can be opted to implement error types from nulls. Also if we consider that `File::from_fd` isn't going to be called a gorillion times a second or a recursive call is done, then I'd say that the tagged union won't bring any other problems, except that a compiler optim is skipped.
* 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.
Although, I want this to be mentioned in the doc of `BadFdError` as this cannot be overlooked when said usage of `?` is done.
rust/bindings/bindings_helper.h | 2 + rust/helpers.c | 7 ++ rust/kernel/file.rs | 176 ++++++++++++++++++++++++++++++++ rust/kernel/lib.rs | 1 + 4 files changed, 186 insertions(+) create mode 100644 rust/kernel/file.rs [...] diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h [...] diff --git a/rust/helpers.c b/rust/helpers.c [...]
This one is making my head spin, shouldn't they go first?