Skip to content

Commit

Permalink
remove_dir_all(): try recursing first instead of trying to unlink()
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
hkratz committed Mar 4, 2022
1 parent 97cde9f commit e427333
Showing 1 changed file with 22 additions and 15 deletions.
37 changes: 22 additions & 15 deletions library/std/src/sys/unix/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1683,8 +1683,21 @@ mod remove_dir_impl {
fn remove_dir_all_recursive(parent_fd: Option<RawFd>, p: &Path) -> io::Result<()> {
let pcstr = cstr(p)?;

// entry is expected to be a directory, open as such
let fd = openat_nofollow_dironly(parent_fd, &pcstr)?;
// try opening as directory
let fd = match openat_nofollow_dironly(parent_fd, &pcstr) {
Err(err) if err.raw_os_error() == Some(libc::ENOTDIR) => {
// not a directory - don't traverse further
return match parent_fd {
// unlink...
Some(parent_fd) => {
cvt(unsafe { unlinkat(parent_fd, pcstr.as_ptr(), 0) }).map(drop)
}
// ...unless this was supposed to be the deletion root directory
None => Err(err),
};
}
result => result?,
};

// open the directory passing ownership of the fd
let (dir, fd) = fdreaddir(fd)?;
Expand All @@ -1697,19 +1710,13 @@ mod remove_dir_impl {
Some(false) => {
cvt(unsafe { unlinkat(fd, child.name_cstr().as_ptr(), 0) })?;
}
None => match cvt(unsafe { unlinkat(fd, child.name_cstr().as_ptr(), 0) }) {
// type unknown - try to unlink
Err(err)
if err.raw_os_error() == Some(libc::EISDIR)
|| err.raw_os_error() == Some(libc::EPERM) =>
{
// if the file is a directory unlink fails with EISDIR on Linux and EPERM everyhwere else
remove_dir_all_recursive(Some(fd), Path::new(&child.file_name()))?;
}
result => {
result?;
}
},
None => {
// POSIX specifies that calling unlink()/unlinkat(..., 0) on a directory can succeed
// if the process has the appropriate privileges. This however can causing orphaned
// directories requiring an fsck e.g. on Solaris and Illumos. So we try recursing
// into it first instead of trying to unlink() it.
remove_dir_all_recursive(Some(fd), Path::new(&child.file_name()))?;
}
}
}

Expand Down

0 comments on commit e427333

Please sign in to comment.