Skip to content

dirfd misc operations (4/4) #161994

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.

Sign up for GitHub

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

Open
Qelxiros wants to merge 3 commits into
base: main
Choose a base branch
from
Open

dirfd misc operations (4/4) #161994

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cab1e92
add symlink
Qelxiros Aug 10, 2026
12adeae
add direntry api
Qelxiros Aug 12, 2026
2e330df
fixes
Qelxiros Sep 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 117 additions & 0 deletions library/std/src/fs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1870,6 +1870,15 @@ impl Dir {
self.inner.remove_dir(path.as_ref())
}

/// Attempts to create a new symbolic link on the filesystem.
///
/// If `original` is a relative path, it is interpreted relative to the created link.
/// If `link` is a relative path, it is interpreted relative to `self`.
#[unstable(feature = "dirfd", issue = "120426")]
pub fn symlink<P: AsRef<Path>, Q: AsRef<Path>>(&self, original: P, link: Q) -> io::Result<()> {
self.inner.symlink(original.as_ref(), link.as_ref())
}

/// Creates a new `Dir` instance that shares the same underlying directory handle
/// as the existing `Dir` instance.
///
Expand Down Expand Up @@ -2885,6 +2894,114 @@ impl DirEntry {
pub fn file_name(&self) -> OsString {
self.0.file_name()
}

/// Opens the file represented by `self` in read-only mode.
///
/// # Errors
///
/// This function will return an error if `self` does not represent a regular file.
/// Other errors may also be returned according to [`OpenOptions::open`].
///
/// # Examples
///
/// ```
/// #![feature(dirfd)]
/// use std::{fs, io};
///
/// if let Ok(entries) = fs::read_dir(".") {
/// for entry in entries {
/// if let Ok(entry) = entry && entry.path().is_file() {
/// if let Ok(file) = entry.open() {
/// println!("{:?}", io::read_to_string(file));
/// }
/// }
/// }
/// }
/// ```
#[unstable(feature = "dirfd", issue = "120426")]
pub fn open(&self) -> io::Result<File> {
self.0.open_with(&OpenOptions::new().read(true).0).map(|inner| File { inner })
}

/// Opens the file represented by `self` according to `options`.
///
/// # Errors
///
/// Errors may be returned according to [`OpenOptions::open`].
///
/// # Examples
///
/// ```
/// #![feature(dirfd)]
/// use std::fs;
/// use std::io::Write;
///
/// if let Ok(entries) = fs::read_dir(".") {
/// for entry in entries {
/// if let Ok(entry) = entry && entry.path().is_file() {
/// if let Ok(mut file) = entry.open_with(&fs::OpenOptions::new().read(true).write(true)) {
/// let _ = file.write_all(b"foo");
/// }
/// }
/// }
/// }
/// ```
#[unstable(feature = "dirfd", issue = "120426")]
pub fn open_with(&self, options: &OpenOptions) -> io::Result<File> {
self.0.open_with(&options.0).map(|inner| File { inner })
}

/// Removes the file represented by `self`.
///
/// # Errors
///
/// This function returns an error if `self` isn't a file. Errors may also be returned for other
/// reasons such as incorrect permissions.
///
/// # Examples
///
/// ```
/// #![feature(dirfd)]
/// use std::fs;
///
/// if let Ok(entries) = fs::read_dir(".") {
/// for entry in entries {
/// if let Ok(entry) = entry && entry.path().is_file() {
/// let _ = entry.remove_file();
/// }
/// }
/// }
/// ```
#[unstable(feature = "dirfd", issue = "120426")]
pub fn remove_file(&self) -> io::Result<()> {
self.0.remove_file()
}

/// Removes the directory represented by `self`.
///
/// # Errors
///
/// This function returns an error if `self` isn't a directory. Errors may also be returned for other
/// reasons such as incorrect permissions or a non-empty directory.
///
/// # Examples
///
/// ```
/// #![feature(dirfd)]
/// use std::fs;
///
/// if let Ok(entries) = fs::read_dir(".") {
/// for entry in entries {
/// if let Ok(entry) = entry && entry.path().is_dir() {
/// let _ = entry.remove_dir();
/// }
/// }
/// }
/// ```
#[unstable(feature = "dirfd", issue = "120426")]
pub fn remove_dir(&self) -> io::Result<()> {
self.0.remove_dir()
}
}

#[stable(feature = "dir_entry_debug", since = "1.13.0")]
Expand Down
77 changes: 73 additions & 4 deletions library/std/src/fs/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use rand::RngCore;

use super::Dir;
use crate::fs::{self, File, FileTimes, OpenOptions, TryLockError, exists};
use crate::fs::{self, File, FileTimes, OpenOptions, TryLockError, exists, read_dir};
use crate::io::prelude::*;
use crate::io::{BorrowedBuf, ErrorKind, SeekFrom};
use crate::mem::MaybeUninit;
Expand Down Expand Up @@ -3027,7 +3027,7 @@ fn test_dir_write_file() {
let tmpdir = tmpdir();
let dir = check!(Dir::open(tmpdir.path()));
let mut f = check!(dir.open_file_with("foo.txt", &OpenOptions::new().write(true).create(true)));
check!(f.write(b"bar"));
check!(f.write_all(b"bar"));
check!(f.flush());
drop(f);
let mut f = check!(File::open(tmpdir.join("foo.txt")));
Expand All @@ -3040,7 +3040,7 @@ fn test_dir_write_file() {
fn test_dir_remove_file() {
let tmpdir = tmpdir();
let mut f = check!(File::create(tmpdir.join("foo.txt")));
check!(f.write(b"bar"));
check!(f.write_all(b"bar"));
check!(f.flush());
drop(f);
let dir = check!(Dir::open(tmpdir.path()));
Expand Down Expand Up @@ -3097,7 +3097,7 @@ fn test_dir_open_dir() {
let dir2 = check!(Dir::open(tmpdir.path().join("foo")));
let mut f =
check!(dir2.open_file_with("bar.txt", &OpenOptions::new().create(true).write(true)));
check!(f.write(b"baz"));
check!(f.write_all(b"baz"));
check!(f.flush());
drop(f);
let dir3 = check!(dir1.open_dir("foo"));
Expand All @@ -3106,3 +3106,72 @@ fn test_dir_open_dir() {
check!(f.read_exact(&mut buf));
assert_eq!(b"baz", &buf);
}

#[test]
fn test_dir_symlink() {
let tmpdir = tmpdir();
if !got_symlink_permission(&tmpdir) {
return;
};

let dir = check!(Dir::open(tmpdir.path()));
let mut f = check!(dir.open_file_with("foo.txt", &OpenOptions::new().write(true).create(true)));
check!(f.write(b"quux"));
check!(f.flush());
drop(f);
check!(dir.symlink("foo.txt", "bar.txt"));
let mut f = check!(dir.open_file("bar.txt"));
let mut buf = [0u8; 4];
check!(f.read_exact(&mut buf));
assert_eq!(b"quux", &buf);
}

#[test]
fn test_dir_direntry_open() {
let tmpdir = tmpdir();
let mut file1 = check!(File::create(tmpdir.path().join("foo.txt")));
let mut file2 = check!(File::create(tmpdir.path().join("bar.txt")));
check!(file1.write_all(b"baz"));
check!(file2.write_all(b"baz"));

for dirent in check!(read_dir(tmpdir.path())) {
let mut file = check!(check!(dirent).open());
let mut buf = [0u8; 3];
check!(file.read_exact(&mut buf));
assert_eq!(b"baz", &buf);
}
}

#[test]
fn test_dir_direntry_open_with() {
let tmpdir = tmpdir();
check!(File::create(tmpdir.path().join("foo.txt")));

for dirent in check!(read_dir(tmpdir.path())) {
let dirent = check!(dirent);
let mut file = check!(dirent.open_with(&OpenOptions::new().read(true).write(true)));
check!(file.write_all(b"baz"));
let contents = check!(fs::read_to_string(dirent.path()));
assert_eq!("baz", contents);
}
}

#[test]
fn test_dir_direntry_remove() {
let tmpdir = tmpdir();
check!(File::create(tmpdir.path().join("foo.txt")));
check!(File::create(tmpdir.path().join("bar.txt")));
check!(fs::create_dir(tmpdir.path().join("baz")));

for dirent in check!(read_dir(tmpdir.path())) {
let dirent = check!(dirent);
if dirent.path().is_file() {
check!(dirent.remove_file());
}
if dirent.path().is_dir() {
check!(dirent.remove_dir());
}
}

assert!(fs::read_dir(tmpdir.path()).is_ok_and(|i| i.count() == 0))
}
28 changes: 27 additions & 1 deletion library/std/src/sys/fs/common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::fs::{create_dir, remove_dir, remove_file, rename};
use crate::io::{self, Error, ErrorKind};
use crate::path::{Path, PathBuf};
use crate::sys::IntoInner;
use crate::sys::fs::{File, FileAttr, OpenOptions};
use crate::sys::fs::{File, FileAttr, OpenOptions, symlink};
use crate::sys::helpers::ignore_notfound;
use crate::{fmt, fs};

Expand Down Expand Up @@ -108,10 +108,36 @@ impl Dir {
pub fn remove_dir(&self, path: &Path) -> io::Result<()> {
remove_dir(self.path.join(path))
}

pub fn symlink(&self, original: &Path, link: &Path) -> io::Result<()> {
symlink(original, &self.path.join(link))
}
}

impl fmt::Debug for Dir {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("Dir").field("path", &self.path).finish()
}
}

#[cfg(not(any(
all(target_os = "linux", not(target_env = "musl")),
target_os = "android",
target_os = "fuchsia",
target_os = "hurd",
target_os = "illumos",
target_vendor = "apple",
)))]
Comment on lines +123 to +130

@RalfJung RalfJung Sep 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This cfg seems to be carefully synced with cfg elsewhere, especially in unix/dir.rs. That's going to be a pain to maintain and does not follow the usual pattern of platform & fallback code in sys.

Also I think this will not even work since we're not importing this file at all on Unix or Windows. I suggested moving things here before I realized that you're using this fallback even on Windows and some Unixes. That will have to be handled differently. I am not sure what the best way to handle this is.

View changes since the review

@RalfJung RalfJung Sep 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also I think this will not even work since we're not importing this file at all on Unix or Windows.

Oh, the file always gets imported.

Still, I think the unix and windows dir files should explicitly invoke the fallback rather than relying on non-trivial cfg coordination across different files. The current code introduces very hard to maintain spaghetti logic. We've had this before e.g. in the thread_local implementation and a lot of effort was spent to clean it up.

impl crate::sys::fs::DirEntry {
pub fn open_with(&self, opts: &OpenOptions) -> io::Result<File> {
File::open(&self.path(), opts)
}

pub fn remove_file(&self) -> io::Result<()> {
remove_file(&self.path())
}

pub fn remove_dir(&self) -> io::Result<()> {
remove_dir(&self.path())
}
}
45 changes: 45 additions & 0 deletions library/std/src/sys/fs/unix.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1108,6 +1108,51 @@ impl DirEntry {
pub fn file_name_os_str(&self) -> &OsStr {
OsStr::from_bytes(self.name.as_bytes())
}

#[cfg(any(
all(target_os = "linux", not(target_env = "musl")),
target_os = "android",
target_os = "fuchsia",
target_os = "hurd",
target_os = "illumos",
target_vendor = "apple",
))]
pub fn open_with(&self, opts: &OpenOptions) -> io::Result<File> {
let dir = unsafe {
mem::ManuallyDrop::new(Dir(OwnedFd::from_raw_fd(cvt(dirfd(self.dir.dirp.0))?)))
};
dir.open_file_c(&self.name, opts, 0).map(FileDesc::from_inner).map(File)

@RalfJung RalfJung Sep 19, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is still racy in terms of self.name, right? A DirEntry doesn't have a handle that would ensure we're still referring to the same entry.

That seems worth mentioning in the docs, at least. It makes me wonder why we even have this operation, but I guess it was in the ACP.

View changes since the review

@the8472 the8472 Sep 19, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It still guards against the directory being renamed but not against the file inside the directory being renamed.

We can additionally cross-check the ino after opening the file with the one in the direntry, that should catch most TOCTOU issues, I guess bindmounts could still be a source of confusion (that'd require checking dev+ino, which we don't have) but at least it'd make attacks more difficult.

@the8472 the8472 Sep 19, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

And I'm not even sure how much attack surface this is. You're iterating a directory and looking for names that exist. Even if someone pulls a switcheroo you only knew that there is something there, which continues to be the case.
The only additional information that can be obtained from the DirEntry is ino and FileType, and only sometimes.

So we could just advise users to not rely on dirent.file_type() and instead check after they opened it.

}
Comment on lines +1112 to +1125

@RalfJung RalfJung Oct 3, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This could be done with less cfg repetition by having a single method on DirEntry that returns a Dir, and then using that everywhere. Only that method needs to be in OS-specific code.

EDIT: Ah no that would cause trouble with dropping. So it would have to be more like a with_dir method that takes an impl FnOnce(&Dir).

View changes since the review


#[cfg(any(
all(target_os = "linux", not(target_env = "musl")),
target_os = "android",
target_os = "fuchsia",
target_os = "hurd",
target_os = "illumos",
target_vendor = "apple",
))]
pub fn remove_file(&self) -> io::Result<()> {
let dir = unsafe {
mem::ManuallyDrop::new(Dir(OwnedFd::from_raw_fd(cvt(dirfd(self.dir.dirp.0))?)))
};
dir.remove_c(&self.name, false)
}

#[cfg(any(
all(target_os = "linux", not(target_env = "musl")),
target_os = "android",
target_os = "fuchsia",
target_os = "hurd",
target_os = "illumos",
target_vendor = "apple",
))]
pub fn remove_dir(&self) -> io::Result<()> {
let dir = unsafe {
mem::ManuallyDrop::new(Dir(OwnedFd::from_raw_fd(cvt(dirfd(self.dir.dirp.0))?)))
};
dir.remove_c(&self.name, true)
}
}

impl OpenOptions {
Expand Down
Loading
Loading