On 12/6/23 12:59, Alice Ryhl wrote:
> +impl File {
> + /// Constructs a new `struct file` wrapper from a file descriptor.
> + ///
> + /// The file descriptor belongs to the current process.
> + pub fn fget(fd: u32) -> Result<ARef<Self>, BadFdError> {
> + // SAFETY: FFI call, there are no requirements on `fd`.
> + let ptr = ptr::NonNull::new(unsafe { bindings::fget(fd) }).ok_or(BadFdError)?;
> +
> + // SAFETY: `fget` either returns null or a valid pointer to a file, and we checked for null
> + // above.
Since now both the Rust and C functions are called `fget`, I think you
should refer to `bindings::fget`.
> + //
> + // INVARIANT: `fget` increments the refcount before returning.
> + Ok(unsafe { ARef::from_raw(ptr.cast()) })
> + }
[...]
> +// SAFETY: The type invariants guarantee that `File` is always ref-counted.
> +unsafe impl AlwaysRefCounted for File {
> + fn inc_ref(&self) {
> + // SAFETY: The existence of a shared reference means that the refcount is nonzero.
> + unsafe { bindings::get_file(self.as_ptr()) };
> + }
> +
> + unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
> + // SAFETY: The safety requirements guarantee that the refcount is nonzero.
> + unsafe { bindings::fput(obj.cast().as_ptr()) }
The comment should also justify the cast.
--
Cheers,
Benno
> + }
> +}
