From d458fb68153f34d3cb71f57db4ad568fec5b5f9e Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Wed, 16 Oct 2019 11:04:39 +0200 Subject: [PATCH] Clean up BSD vs Linux implementation details This commit moves a couple of things around: * separates the logic of `path_unlink_file` into separate impls for linux and BSD-style nixes * moves implementation consts into appropriate impl modules: linux or bsd * cleans up `utime_now` and `utime_omit` for BSD-style nixes --- src/sys/unix/bsd/hostcalls_impl.rs | 41 +++++++++++++++ src/sys/unix/bsd/mod.rs | 61 +++++++++++++++++++++++ src/sys/unix/host_impl.rs | 51 ++++++------------- src/sys/unix/hostcalls_impl/fs.rs | 44 ---------------- src/sys/unix/hostcalls_impl/fs_helpers.rs | 36 ++++++------- src/sys/unix/linux/hostcalls_impl.rs | 15 ++++++ src/sys/unix/linux/mod.rs | 35 +++++++++++++ 7 files changed, 183 insertions(+), 100 deletions(-) diff --git a/src/sys/unix/bsd/hostcalls_impl.rs b/src/sys/unix/bsd/hostcalls_impl.rs index 35a57a03b1..d2cbb9b5c7 100644 --- a/src/sys/unix/bsd/hostcalls_impl.rs +++ b/src/sys/unix/bsd/hostcalls_impl.rs @@ -8,6 +8,47 @@ use std::ffi::CString; use std::fs::File; use std::os::unix::prelude::AsRawFd; +pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> { + use nix::errno; + use nix::libc::unlinkat; + + let path_cstr = CString::new(resolved.path().as_bytes()).map_err(|_| Error::EILSEQ)?; + + // nix doesn't expose unlinkat() yet + match unsafe { unlinkat(resolved.dirfd().as_raw_fd(), path_cstr.as_ptr(), 0) } { + 0 => Ok(()), + _ => { + let mut e = errno::Errno::last(); + + // Non-Linux implementations may return EPERM when attempting to remove a + // directory without REMOVEDIR. While that's what POSIX specifies, it's + // less useful. Adjust this to EISDIR. It doesn't matter that this is not + // atomic with the unlinkat, because if the file is removed and a directory + // is created before fstatat sees it, we're racing with that change anyway + // and unlinkat could have legitimately seen the directory if the race had + // turned out differently. + use nix::fcntl::AtFlags; + use nix::sys::stat::{fstatat, SFlag}; + + if e == errno::Errno::EPERM { + if let Ok(stat) = fstatat( + resolved.dirfd().as_raw_fd(), + resolved.path(), + AtFlags::AT_SYMLINK_NOFOLLOW, + ) { + if SFlag::from_bits_truncate(stat.st_mode).contains(SFlag::S_IFDIR) { + e = errno::Errno::EISDIR; + } + } else { + e = errno::Errno::last(); + } + } + + Err(host_impl::errno_from_nix(e)) + } + } +} + pub(crate) fn path_rename(resolved_old: PathGet, resolved_new: PathGet) -> Result<()> { use nix::{errno::Errno, fcntl::AtFlags, libc::renameat, sys::stat::fstatat}; let old_path_cstr = CString::new(resolved_old.path().as_bytes()).map_err(|_| Error::EILSEQ)?; diff --git a/src/sys/unix/bsd/mod.rs b/src/sys/unix/bsd/mod.rs index 0ca462eda3..4161742883 100644 --- a/src/sys/unix/bsd/mod.rs +++ b/src/sys/unix/bsd/mod.rs @@ -17,3 +17,64 @@ pub(crate) mod fdentry_impl { } } } + +pub(crate) mod host_impl { + use super::super::host_impl::dirent_filetype_from_host; + use crate::{host, memory, Result}; + + pub(crate) const O_RSYNC: nix::fcntl::OFlag = nix::fcntl::OFlag::O_SYNC; + + pub(crate) fn dirent_from_host( + host_entry: &nix::libc::dirent, + ) -> Result { + let mut entry = unsafe { std::mem::zeroed::() }; + let d_type = dirent_filetype_from_host(host_entry)?; + entry.d_ino = memory::enc_inode(host_entry.d_ino); + entry.d_next = memory::enc_dircookie(host_entry.d_seekoff); + entry.d_namlen = memory::enc_u32(u32::from(host_entry.d_namlen)); + entry.d_type = memory::enc_filetype(d_type); + Ok(entry) + } +} + +pub(crate) mod fs_helpers { + use cfg_if::cfg_if; + + pub(crate) fn utime_now() -> libc::c_long { + cfg_if! { + if #[cfg(any( + target_os = "macos", + target_os = "freebsd", + target_os = "ios", + target_os = "dragonfly" + ))] { + -1 + } else if #[cfg(target_os = "openbsd")] { + // https://github.com/openbsd/src/blob/master/sys/sys/stat.h#L187 + -2 + } else if #[cfg(target_os = "netbsd" )] { + // http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/sys/stat.h?rev=1.69&content-type=text/x-cvsweb-markup&only_with_tag=MAIN + 1_073_741_823 + } + } + } + + pub(crate) fn utime_omit() -> libc::c_long { + cfg_if! { + if #[cfg(any( + target_os = "macos", + target_os = "freebsd", + target_os = "ios", + target_os = "dragonfly" + ))] { + -2 + } else if #[cfg(target_os = "openbsd")] { + // https://github.com/openbsd/src/blob/master/sys/sys/stat.h#L187 + -1 + } else if #[cfg(target_os = "netbsd")] { + // http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/sys/stat.h?rev=1.69&content-type=text/x-cvsweb-markup&only_with_tag=MAIN + 1_073_741_822 + } + } + } +} diff --git a/src/sys/unix/host_impl.rs b/src/sys/unix/host_impl.rs index f355002d3a..d46a3e2f3c 100644 --- a/src/sys/unix/host_impl.rs +++ b/src/sys/unix/host_impl.rs @@ -3,11 +3,26 @@ #![allow(non_snake_case)] #![allow(dead_code)] use crate::hostcalls_impl::FileType; -use crate::{host, memory, Error, Result}; +use crate::{host, Error, Result}; use log::warn; use std::ffi::OsStr; use std::os::unix::prelude::OsStrExt; +cfg_if::cfg_if! { + if #[cfg(target_os = "linux")] { + pub(crate) use super::linux::host_impl::*; + } else if #[cfg(any( + target_os = "macos", + target_os = "netbsd", + target_os = "freebsd", + target_os = "openbsd", + target_os = "ios", + target_os = "dragonfly" + ))] { + pub(crate) use super::bsd::host_impl::*; + } +} + pub(crate) fn errno_from_nix(errno: nix::errno::Errno) -> Error { match errno { nix::errno::Errno::EPERM => Error::EPERM, @@ -91,12 +106,6 @@ pub(crate) fn errno_from_nix(errno: nix::errno::Errno) -> Error { } } -#[cfg(target_os = "linux")] -pub(crate) const O_RSYNC: nix::fcntl::OFlag = nix::fcntl::OFlag::O_RSYNC; - -#[cfg(not(target_os = "linux"))] -pub(crate) const O_RSYNC: nix::fcntl::OFlag = nix::fcntl::OFlag::O_SYNC; - pub(crate) fn nix_from_fdflags(fdflags: host::__wasi_fdflags_t) -> nix::fcntl::OFlag { use nix::fcntl::OFlag; let mut nix_flags = OFlag::empty(); @@ -228,34 +237,6 @@ pub(crate) fn dirent_filetype_from_host( } } -#[cfg(target_os = "linux")] -pub(crate) fn dirent_from_host(host_entry: &nix::libc::dirent) -> Result { - let mut entry = unsafe { std::mem::zeroed::() }; - let d_namlen = unsafe { std::ffi::CStr::from_ptr(host_entry.d_name.as_ptr()) } - .to_bytes() - .len(); - if d_namlen > u32::max_value() as usize { - return Err(Error::EIO); - } - let d_type = dirent_filetype_from_host(host_entry)?; - entry.d_ino = memory::enc_inode(host_entry.d_ino); - entry.d_next = memory::enc_dircookie(host_entry.d_off as u64); - entry.d_namlen = memory::enc_u32(d_namlen as u32); - entry.d_type = memory::enc_filetype(d_type); - Ok(entry) -} - -#[cfg(not(target_os = "linux"))] -pub(crate) fn dirent_from_host(host_entry: &nix::libc::dirent) -> Result { - let mut entry = unsafe { std::mem::zeroed::() }; - let d_type = dirent_filetype_from_host(host_entry)?; - entry.d_ino = memory::enc_inode(host_entry.d_ino); - entry.d_next = memory::enc_dircookie(host_entry.d_seekoff); - entry.d_namlen = memory::enc_u32(u32::from(host_entry.d_namlen)); - entry.d_type = memory::enc_filetype(d_type); - Ok(entry) -} - /// Creates owned WASI path from OS string. /// /// NB WASI spec requires OS string to be valid UTF-8. Otherwise, diff --git a/src/sys/unix/hostcalls_impl/fs.rs b/src/sys/unix/hostcalls_impl/fs.rs index 420d20eefa..24a33155a3 100644 --- a/src/sys/unix/hostcalls_impl/fs.rs +++ b/src/sys/unix/hostcalls_impl/fs.rs @@ -357,50 +357,6 @@ pub(crate) fn path_symlink(old_path: &str, resolved: PathGet) -> Result<()> { } } -pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> { - use nix::errno; - use nix::libc::unlinkat; - - let path_cstr = CString::new(resolved.path().as_bytes()).map_err(|_| Error::EILSEQ)?; - - // nix doesn't expose unlinkat() yet - match unsafe { unlinkat(resolved.dirfd().as_raw_fd(), path_cstr.as_ptr(), 0) } { - 0 => Ok(()), - _ => { - let mut e = errno::Errno::last(); - - #[cfg(not(linux))] - { - // Non-Linux implementations may return EPERM when attempting to remove a - // directory without REMOVEDIR. While that's what POSIX specifies, it's - // less useful. Adjust this to EISDIR. It doesn't matter that this is not - // atomic with the unlinkat, because if the file is removed and a directory - // is created before fstatat sees it, we're racing with that change anyway - // and unlinkat could have legitimately seen the directory if the race had - // turned out differently. - use nix::fcntl::AtFlags; - use nix::sys::stat::{fstatat, SFlag}; - - if e == errno::Errno::EPERM { - if let Ok(stat) = fstatat( - resolved.dirfd().as_raw_fd(), - resolved.path(), - AtFlags::AT_SYMLINK_NOFOLLOW, - ) { - if SFlag::from_bits_truncate(stat.st_mode).contains(SFlag::S_IFDIR) { - e = errno::Errno::EISDIR; - } - } else { - e = errno::Errno::last(); - } - } - } - - Err(host_impl::errno_from_nix(e)) - } - } -} - pub(crate) fn path_remove_directory(resolved: PathGet) -> Result<()> { use nix::errno; use nix::libc::{unlinkat, AT_REMOVEDIR}; diff --git a/src/sys/unix/hostcalls_impl/fs_helpers.rs b/src/sys/unix/hostcalls_impl/fs_helpers.rs index 31cd355ba5..aa1097a18e 100644 --- a/src/sys/unix/hostcalls_impl/fs_helpers.rs +++ b/src/sys/unix/hostcalls_impl/fs_helpers.rs @@ -2,9 +2,23 @@ #![allow(unused_unsafe)] use crate::sys::host_impl; use crate::{host, Result}; -use nix::libc::{self, c_long}; use std::fs::File; +cfg_if::cfg_if! { + if #[cfg(target_os = "linux")] { + pub(crate) use super::super::linux::fs_helpers::*; + } else if #[cfg(any( + target_os = "macos", + target_os = "netbsd", + target_os = "freebsd", + target_os = "openbsd", + target_os = "ios", + target_os = "dragonfly" + ))] { + pub(crate) use super::super::bsd::fs_helpers::*; + } +} + pub(crate) fn path_open_rights( rights_base: host::__wasi_rights_t, rights_inheriting: host::__wasi_rights_t, @@ -63,23 +77,3 @@ pub(crate) fn readlinkat(dirfd: &File, path: &str) -> Result { .map_err(Into::into) .and_then(host_impl::path_from_host) } - -#[cfg(not(target_os = "macos"))] -pub(crate) fn utime_now() -> c_long { - libc::UTIME_NOW -} - -#[cfg(target_os = "macos")] -pub(crate) fn utime_now() -> c_long { - -1 -} - -#[cfg(not(target_os = "macos"))] -pub(crate) fn utime_omit() -> c_long { - libc::UTIME_OMIT -} - -#[cfg(target_os = "macos")] -pub(crate) fn utime_omit() -> c_long { - -2 -} diff --git a/src/sys/unix/linux/hostcalls_impl.rs b/src/sys/unix/linux/hostcalls_impl.rs index 13952d01b0..3d0d3ed109 100644 --- a/src/sys/unix/linux/hostcalls_impl.rs +++ b/src/sys/unix/linux/hostcalls_impl.rs @@ -9,6 +9,21 @@ use std::fs::File; use std::mem::MaybeUninit; use std::os::unix::prelude::AsRawFd; +pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> { + use nix::errno; + use nix::libc::unlinkat; + + let path_cstr = CString::new(resolved.path().as_bytes()).map_err(|_| Error::EILSEQ)?; + + // nix doesn't expose unlinkat() yet + let res = unsafe { unlinkat(resolved.dirfd().as_raw_fd(), path_cstr.as_ptr(), 0) }; + if res == 0 { + Ok(()) + } else { + Err(host_impl::errno_from_nix(errno::Errno::last())) + } +} + pub(crate) fn path_rename(resolved_old: PathGet, resolved_new: PathGet) -> Result<()> { use nix::libc::renameat; let old_path_cstr = CString::new(resolved_old.path().as_bytes()).map_err(|_| Error::EILSEQ)?; diff --git a/src/sys/unix/linux/mod.rs b/src/sys/unix/linux/mod.rs index e751ff010d..a435cf7165 100644 --- a/src/sys/unix/linux/mod.rs +++ b/src/sys/unix/linux/mod.rs @@ -24,3 +24,38 @@ pub(crate) mod fdentry_impl { } } } + +pub(crate) mod host_impl { + use super::super::host_impl::dirent_filetype_from_host; + use crate::{host, memory, Error, Result}; + + pub(crate) const O_RSYNC: nix::fcntl::OFlag = nix::fcntl::OFlag::O_RSYNC; + + pub(crate) fn dirent_from_host( + host_entry: &nix::libc::dirent, + ) -> Result { + let mut entry = unsafe { std::mem::zeroed::() }; + let d_namlen = unsafe { std::ffi::CStr::from_ptr(host_entry.d_name.as_ptr()) } + .to_bytes() + .len(); + if d_namlen > u32::max_value() as usize { + return Err(Error::EIO); + } + let d_type = dirent_filetype_from_host(host_entry)?; + entry.d_ino = memory::enc_inode(host_entry.d_ino); + entry.d_next = memory::enc_dircookie(host_entry.d_off as u64); + entry.d_namlen = memory::enc_u32(d_namlen as u32); + entry.d_type = memory::enc_filetype(d_type); + Ok(entry) + } +} + +pub(crate) mod fs_helpers { + pub(crate) fn utime_now() -> libc::c_long { + libc::UTIME_NOW + } + + pub(crate) fn utime_omit() -> libc::c_long { + libc::UTIME_OMIT + } +}