Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: I/O safety for modules fcntl and dir #2434

Merged
merged 10 commits into from
Jun 9, 2024

Conversation

SteveLauC
Copy link
Member

@SteveLauC SteveLauC commented Jun 8, 2024

What does this PR do

This PR adds I/O safety to fcntl.rs and dir.rs.

Ref: #1750

xxat() interfaces

For those xxat() interfaces, I didn't introduce a new trait to represent directory file descriptors, instead, I took the way how rustix implements it:

  1. Define AT_FDCWD: BorrowedFd<'static>
  2. Change the signature to xxat<Fd: AsFd>(dirfd: Fd, ...)

because:

  1. rustix has been using this interface for a long time, which means it would work pretty well.
  2. It is indeed unfortunate that we cannot catch some errors at compile time, but we will eventually get notified at runtime, e.g., when passing a fd that is not a directory to dirfd, or using AT_FDCWD with non-xxat() functions.

Checklist:

  • I have read CONTRIBUTING.md
  • I have written necessary tests and rustdoc comments
  • A change log has been added if this PR modifies nix's API

since = "0.30.0",
note = "Deprecate this since it is not I/O-safe, use from_fd instead."
)]
pub unsafe fn from<F: IntoRawFd>(fd: F) -> Result<Self> {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this function should be removed given its IntoRawFd usage, so deprecate it in the next release and remove it in futhre versions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For all the owned file descriptors types provided by the std, they have Into<OwnedFd> implemented, so Dir::from_fd() would be a good replacement for this function.


#[cfg(not(target_os = "haiku"))]
#[test]
fn ebadf() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is removed because this should not happen after the adoption of I/O safety.

@SteveLauC SteveLauC marked this pull request as ready for review June 9, 2024 06:25
@SteveLauC SteveLauC force-pushed the refactor/io_safety_fcntl branch from 4004b87 to d6d7f40 Compare June 9, 2024 06:27
@SteveLauC SteveLauC added this pull request to the merge queue Jun 9, 2024
@SteveLauC SteveLauC changed the title refactor: I/O safety for module fcntl refactor: I/O safety for modules fcntl and dir Jun 9, 2024
@SteveLauC SteveLauC mentioned this pull request Jun 9, 2024
29 tasks
Merged via the queue into nix-rust:master with commit b2f34f7 Jun 9, 2024
36 checks passed
@SteveLauC SteveLauC deleted the refactor/io_safety_fcntl branch June 9, 2024 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant