On Thu, Feb 8, 2024 at 1:35 AM Martin Rodriguez Reboredo <yakoyoku@xxxxxxxxx> wrote: > > On 2/2/24 07:55, Alice Ryhl wrote: > > From: Wedson Almeida Filho <wedsonaf@xxxxxxxxx> > > > > 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@xxxxxxxxx> > > 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> > > --- > > [...] > > +/// ## Rust references > > +/// > > +/// The reference type `&File` is similar to light refcounts: > > +/// > > +/// * `&File` references don't own a reference count. They can only exist as long as the reference > > +/// count stays positive, and can only be created when there is some mechanism in place to ensure > > +/// this. > > +/// > > +/// * The Rust borrow-checker normally ensures this by enforcing that the `ARef<File>` from which > > +/// a `&File` is created outlives the `&File`. > > +/// > > +/// * Using the unsafe [`File::from_ptr`] means that it is up to the caller to ensure that the > > +/// `&File` only exists while the reference count is positive. > > +/// > > +/// * You can think of `fdget` as using an fd to look up an `ARef<File>` in the `struct > > +/// files_struct` and create an `&File` from it. The "fd cannot be closed" rule is like the Rust > > +/// rule "the `ARef<File>` must outlive the `&File`". > > I find it kinda odd that this unordered list interspaces elements with > blank lines as opposed to the following one, though, I don't see it as > rather a big deal. Shrug. I did it here because I found it more readable. > > +/// > > +/// # Invariants > > +/// > > +/// * Instances of this type are refcounted using the `f_count` field. > > +/// * If an fd with active light refcounts is closed, then it must be the case that the file > > +/// refcount is positive until all light refcounts of the fd have been dropped. > > +/// * A light refcount must be dropped before returning to userspace. > > [...] > > Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@xxxxxxxxx>
