device: Use OwnedFd type and bump MSRV to 1.66
#75
Conversation
| #[derive(Debug, Clone)] | ||
| pub struct Handle(RawFd); | ||
| #[derive(Debug)] | ||
| pub struct Handle(OwnedFd); |
There was a problem hiding this comment.
This type always wrongly derived Clone, causing drop() close() the same fd multiple times...
Instead I'd argue this trait should be available on Device which holds on to an Arc<Handle> exactly to allow sharing of the fd without cloning it.
Additionally Handle could provide try_clone() to duplicate the file descriptor?
Also, Device currently has an fn handle() to clone this Arc<Handle> but thus far seems to mostly be used to distribute the Fd around. Hence:
- Should we implement
As(Raw)FdforDeviceso that it's easier to borrow its (raw) file descriptor? - Should various implementations like
io::Queuestore aDevicedirectly so that it's more obvious that it's not just a wrapper aroundOwnedFdbut really a wrapper around aDevice?
| /// Wraps an existing file descriptor | ||
| /// | ||
| /// The caller must ensure that `fd` is a valid, open file descriptor for a V4L device. | ||
| pub unsafe fn new(fd: RawFd) -> Self { | ||
| pub fn new(fd: OwnedFd) -> Self { |
There was a problem hiding this comment.
These docs, while still true, are a bit less relevant now (see also the removal of unsafe) because the caller already has to guarantee this when constructing an OwnedFd.
|
@raymanfx should I target this to |
|
@raymanfx ping? |
|
Hi guys, sorry for being MIA. I've been working on other projects and there's only so many hours in a day. I definately want to return to working on this crate and finish the overhaul of the I/O memory API. @MarijnS95 as you have been involved for quite a bit now, what do you think about taking over maintainership of this crate temporarily? I'm still available for reviews, but don't feel like I can dedicate enough time to API changes / architecture right now. |
|
@raymanfx I figured you'd ask; same problems for me, there's only so much time in a day. And I'm not even using this crate yet, that's still on a soon™ project list somewhere™. That said I do somehow have a sweet-spot for low-level (typically kernel/OS related) binding crates ( On that note, there's still an unfinished Media API in one of my branches :) |
The first new commit 22a2365 on `next` switches to the `RawFd` type from the `std::os::fd` module which is only available since Rust 1.66. This module also provides an `OwnedFd` type which already has a `close()` call in the `Drop` implementation, and implements the desired `AsRawFd` (and `AsFd` and `IntoRawFd`) traits. Note that these types were already stabilized in Rust 1.63 under `std::os::unix::io`, which is now merely a reexport of `std::os::fd`. This also replaces the forgotten `.fd()` calls with `.as_raw_fd()` from from commit 17c2871.
The first new commit 22a2365 on
nextswitches to theRawFdtype from thestd::os::fdmodule which is only available since Rust 1.66. This module also provides anOwnedFdtype which already has aclose()call in theDropimplementation, and implements the desiredAsRawFd(andAsFdandIntoRawFd) traits.Note that these types were already stabilized in Rust 1.63 under
std::os::unix::io, which is now merely a reexport ofstd::os::fd.This also replaces the forgotten
.fd()calls with.as_raw_fd()from from commit 17c2871.