Skip to content

Commit b2f34f7

Browse files
authored
refactor: I/O safety for module fcntl (#2434)
* refactor: I/O safety for module fcntl * fix: move the ownership from OwnedFd to Dir * style: format code * style: format code * chore: fix imports * chore: fix imports * chore: fix imports * chore: remove a redundant dot from FreeBSD example * docs: more examples * chore: changelog entry
1 parent d029c24 commit b2f34f7

File tree

10 files changed

+437
-218
lines changed

10 files changed

+437
-218
lines changed

changelog/2434.changed.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Public interfaces in `fcntl.rs` and `dir.rs` now use I/O-safe types.

src/dir.rs

Lines changed: 78 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
use crate::errno::Errno;
44
use crate::fcntl::{self, OFlag};
55
use crate::sys;
6-
use crate::{Error, NixPath, Result};
6+
use crate::{NixPath, Result};
77
use cfg_if::cfg_if;
88
use std::ffi;
99
use std::os::unix::io::{AsRawFd, IntoRawFd, RawFd};
@@ -17,17 +17,38 @@ use libc::{dirent, readdir_r};
1717

1818
/// An open directory.
1919
///
20-
/// This is a lower-level interface than `std::fs::ReadDir`. Notable differences:
21-
/// * can be opened from a file descriptor (as returned by `openat`, perhaps before knowing
22-
/// if the path represents a file or directory).
23-
/// * implements `AsRawFd`, so it can be passed to `fstat`, `openat`, etc.
24-
/// The file descriptor continues to be owned by the `Dir`, so callers must not keep a `RawFd`
25-
/// after the `Dir` is dropped.
20+
/// This is a lower-level interface than [`std::fs::ReadDir`]. Notable differences:
21+
/// * can be opened from a file descriptor (as returned by [`openat`][openat],
22+
/// perhaps before knowing if the path represents a file or directory).
23+
/// * implements [`AsFd`][AsFd], so it can be passed to [`fstat`][fstat],
24+
/// [`openat`][openat], etc. The file descriptor continues to be owned by the
25+
/// `Dir`, so callers must not keep a `RawFd` after the `Dir` is dropped.
2626
/// * can be iterated through multiple times without closing and reopening the file
2727
/// descriptor. Each iteration rewinds when finished.
2828
/// * returns entries for `.` (current directory) and `..` (parent directory).
29-
/// * returns entries' names as a `CStr` (no allocation or conversion beyond whatever libc
29+
/// * returns entries' names as a [`CStr`][cstr] (no allocation or conversion beyond whatever libc
3030
/// does).
31+
///
32+
/// [AsFd]: std::os::fd::AsFd
33+
/// [fstat]: crate::sys::stat::fstat
34+
/// [openat]: crate::fcntl::openat
35+
/// [cstr]: std::ffi::CStr
36+
///
37+
/// # Examples
38+
///
39+
/// Traverse the current directory, and print entries' names:
40+
///
41+
/// ```
42+
/// use nix::dir::Dir;
43+
/// use nix::fcntl::OFlag;
44+
/// use nix::sys::stat::Mode;
45+
///
46+
/// let mut cwd = Dir::open(".", OFlag::O_RDONLY | OFlag::O_CLOEXEC, Mode::empty()).unwrap();
47+
/// for res_entry in cwd.iter() {
48+
/// let entry = res_entry.unwrap();
49+
/// println!("File name: {}", entry.file_name().to_str().unwrap());
50+
/// }
51+
/// ```
3152
#[derive(Debug, Eq, Hash, PartialEq)]
3253
pub struct Dir(ptr::NonNull<libc::DIR>);
3354

@@ -43,8 +64,8 @@ impl Dir {
4364
}
4465

4566
/// Opens the given path as with `fcntl::openat`.
46-
pub fn openat<P: ?Sized + NixPath>(
47-
dirfd: Option<RawFd>,
67+
pub fn openat<Fd: std::os::fd::AsFd, P: ?Sized + NixPath>(
68+
dirfd: Fd,
4869
path: &P,
4970
oflag: OFlag,
5071
mode: sys::stat::Mode,
@@ -54,21 +75,46 @@ impl Dir {
5475
}
5576

5677
/// Converts from a descriptor-based object, closing the descriptor on success or failure.
78+
///
79+
/// # Safety
80+
///
81+
/// It is only safe if `fd` is an owned file descriptor.
5782
#[inline]
58-
pub fn from<F: IntoRawFd>(fd: F) -> Result<Self> {
59-
Dir::from_fd(fd.into_raw_fd())
83+
#[deprecated(
84+
since = "0.30.0",
85+
note = "Deprecate this since it is not I/O-safe, use from_fd instead."
86+
)]
87+
pub unsafe fn from<F: IntoRawFd>(fd: F) -> Result<Self> {
88+
use std::os::fd::FromRawFd;
89+
use std::os::fd::OwnedFd;
90+
91+
// SAFETY:
92+
//
93+
// This is indeed unsafe is `fd` it not an owned fd.
94+
let owned_fd = unsafe { OwnedFd::from_raw_fd(fd.into_raw_fd()) };
95+
Dir::from_fd(owned_fd)
6096
}
6197

6298
/// Converts from a file descriptor, closing it on failure.
99+
///
100+
/// # Examples
101+
///
102+
/// `ENOTDIR` would be returned if `fd` does not refer to a directory:
103+
///
104+
/// ```should_panic
105+
/// use std::os::fd::OwnedFd;
106+
/// use nix::dir::Dir;
107+
///
108+
/// let temp_file = tempfile::tempfile().unwrap();
109+
/// let temp_file_fd: OwnedFd = temp_file.into();
110+
/// let never = Dir::from_fd(temp_file_fd).unwrap();
111+
/// ```
63112
#[doc(alias("fdopendir"))]
64-
pub fn from_fd(fd: RawFd) -> Result<Self> {
65-
let d = ptr::NonNull::new(unsafe { libc::fdopendir(fd) }).ok_or_else(
66-
|| {
67-
let e = Error::last();
68-
unsafe { libc::close(fd) };
69-
e
70-
},
71-
)?;
113+
pub fn from_fd(fd: std::os::fd::OwnedFd) -> Result<Self> {
114+
// take the ownership as the constructed `Dir` is now the owner
115+
let raw_fd = fd.into_raw_fd();
116+
let d = ptr::NonNull::new(unsafe { libc::fdopendir(raw_fd) })
117+
.ok_or(Errno::last())?;
72118
Ok(Dir(d))
73119
}
74120

@@ -86,6 +132,18 @@ impl Dir {
86132
// `Dir` is safe to pass from one thread to another, as it's not reference-counted.
87133
unsafe impl Send for Dir {}
88134

135+
impl std::os::fd::AsFd for Dir {
136+
fn as_fd(&self) -> std::os::fd::BorrowedFd {
137+
let raw_fd = self.as_raw_fd();
138+
139+
// SAFETY:
140+
//
141+
// `raw_fd` should be open and valid for the lifetime of the returned
142+
// `BorrowedFd` as it is extracted from `&self`.
143+
unsafe { std::os::fd::BorrowedFd::borrow_raw(raw_fd) }
144+
}
145+
}
146+
89147
impl AsRawFd for Dir {
90148
fn as_raw_fd(&self) -> RawFd {
91149
unsafe { libc::dirfd(self.0.as_ptr()) }

0 commit comments

Comments
 (0)