Re: [RFC PATCH v1 1/5] rust: file: add bindings for `struct file`

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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?



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux