Skip to content

Commit e427333

Browse files
committed
remove_dir_all(): try recursing first instead of trying to unlink()
This only affects the `slow` code path, if there is no `dirent.d_type` or if the type is `DT_UNKNOWN`. POSIX specifies that calling `unlink()` or `unlinkat(..., 0)` on a directory can succeed: > "The _path_ argument shall not name a directory unless the process has > appropriate privileges and the implementation supports using _unlink()_ on > directories." This however can cause orphaned directories requiring an fsck e.g. on Illumos UFS, so we have to avoid that in the common case. We now just try to recurse into it first and unlink() if we can't open it as a directory.
1 parent 97cde9f commit e427333

File tree

1 file changed

+22
-15
lines changed
  • library/std/src/sys/unix

1 file changed

+22
-15
lines changed

library/std/src/sys/unix/fs.rs

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1683,8 +1683,21 @@ mod remove_dir_impl {
16831683
fn remove_dir_all_recursive(parent_fd: Option<RawFd>, p: &Path) -> io::Result<()> {
16841684
let pcstr = cstr(p)?;
16851685

1686-
// entry is expected to be a directory, open as such
1687-
let fd = openat_nofollow_dironly(parent_fd, &pcstr)?;
1686+
// try opening as directory
1687+
let fd = match openat_nofollow_dironly(parent_fd, &pcstr) {
1688+
Err(err) if err.raw_os_error() == Some(libc::ENOTDIR) => {
1689+
// not a directory - don't traverse further
1690+
return match parent_fd {
1691+
// unlink...
1692+
Some(parent_fd) => {
1693+
cvt(unsafe { unlinkat(parent_fd, pcstr.as_ptr(), 0) }).map(drop)
1694+
}
1695+
// ...unless this was supposed to be the deletion root directory
1696+
None => Err(err),
1697+
};
1698+
}
1699+
result => result?,
1700+
};
16881701

16891702
// open the directory passing ownership of the fd
16901703
let (dir, fd) = fdreaddir(fd)?;
@@ -1697,19 +1710,13 @@ mod remove_dir_impl {
16971710
Some(false) => {
16981711
cvt(unsafe { unlinkat(fd, child.name_cstr().as_ptr(), 0) })?;
16991712
}
1700-
None => match cvt(unsafe { unlinkat(fd, child.name_cstr().as_ptr(), 0) }) {
1701-
// type unknown - try to unlink
1702-
Err(err)
1703-
if err.raw_os_error() == Some(libc::EISDIR)
1704-
|| err.raw_os_error() == Some(libc::EPERM) =>
1705-
{
1706-
// if the file is a directory unlink fails with EISDIR on Linux and EPERM everyhwere else
1707-
remove_dir_all_recursive(Some(fd), Path::new(&child.file_name()))?;
1708-
}
1709-
result => {
1710-
result?;
1711-
}
1712-
},
1713+
None => {
1714+
// POSIX specifies that calling unlink()/unlinkat(..., 0) on a directory can succeed
1715+
// if the process has the appropriate privileges. This however can causing orphaned
1716+
// directories requiring an fsck e.g. on Solaris and Illumos. So we try recursing
1717+
// into it first instead of trying to unlink() it.
1718+
remove_dir_all_recursive(Some(fd), Path::new(&child.file_name()))?;
1719+
}
17131720
}
17141721
}
17151722

0 commit comments

Comments
 (0)