From 42fae4e3b8cdb168e241444a68a7a1c3ebe8db4a Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Fri, 6 Mar 2020 23:20:54 +0100 Subject: [PATCH] [wasi-common]: yanix now returns io::Error directly (#1242) * Yanix now returns io::Error This commit may seem somewhat controversial at first, but hear me out first. Currently, Yanix would return a custom error that's a wrapper around three other error types returned by various entities inside Rust's `libstd`. In particular, Yanix's error type would wrap `io::Error`, `num::TryFromIntError` and `ffi::NulError`. It turns out that there is a natural conversion between the first and the last and provided by the standard library, i.e., `From for io::Error` is provided. So at the surface it may seem that only the first two wrapped error types are worth keeping. Digging a little bit deeper into `libstd`, `num::TryFromIntError` is essentially speaking only a marker that the integral conversion went wrong. The struct implementing this error stores a unit type, and nothing more. It therefore seems like a waste to wrap this particular error when we could unify everything under `io::Error`. And so, whenever we perform an int conversion, I suggest we simply remap the error to `io::Error::from_raw_os_error(libc::EOVERFLOW)` since this carries a comparable amount of information. As a result of completely discarding `yanix::Error` custom error type, we are invariably simplifying `yanix` itself, but also allowing `wasi-common` to simplify in several places as well. * Adapt wasi-common to changes in yanix * Add Cargo.lock * Unwrap try_into's where possible * Remove unnecessary type annotation --- Cargo.lock | 1 - .../snapshot_0/sys/unix/bsd/hostcalls_impl.rs | 124 +++++++++--------- .../src/old/snapshot_0/sys/unix/host_impl.rs | 11 -- .../snapshot_0/sys/unix/hostcalls_impl/fs.rs | 88 ++++++------- .../src/sys/unix/bsd/hostcalls_impl.rs | 124 +++++++++--------- crates/wasi-common/src/sys/unix/host_impl.rs | 11 -- .../src/sys/unix/hostcalls_impl/fs.rs | 88 ++++++------- crates/wasi-common/yanix/Cargo.toml | 1 - crates/wasi-common/yanix/src/clock.rs | 11 +- crates/wasi-common/yanix/src/dir.rs | 2 +- crates/wasi-common/yanix/src/error.rs | 61 --------- crates/wasi-common/yanix/src/fcntl.rs | 13 +- crates/wasi-common/yanix/src/file.rs | 47 ++++--- crates/wasi-common/yanix/src/lib.rs | 48 ++++++- crates/wasi-common/yanix/src/poll.rs | 12 +- crates/wasi-common/yanix/src/socket.rs | 5 +- crates/wasi-common/yanix/src/sys/bsd/dir.rs | 14 +- .../wasi-common/yanix/src/sys/bsd/fadvise.rs | 21 ++- crates/wasi-common/yanix/src/sys/bsd/file.rs | 18 ++- .../yanix/src/sys/emscripten/mod.rs | 12 +- crates/wasi-common/yanix/src/sys/linux/dir.rs | 14 +- .../yanix/src/sys/linux/fadvise.rs | 5 +- .../wasi-common/yanix/src/sys/linux/file.rs | 29 ++-- crates/wasi-common/yanix/src/sys/linux/mod.rs | 3 +- crates/wasi-common/yanix/src/sys/mod.rs | 3 +- 25 files changed, 368 insertions(+), 398 deletions(-) delete mode 100644 crates/wasi-common/yanix/src/error.rs diff --git a/Cargo.lock b/Cargo.lock index 79865b4ccb..8f99fb1591 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2607,7 +2607,6 @@ dependencies = [ "cfg-if", "libc", "log", - "thiserror", ] [[package]] diff --git a/crates/wasi-common/src/old/snapshot_0/sys/unix/bsd/hostcalls_impl.rs b/crates/wasi-common/src/old/snapshot_0/sys/unix/bsd/hostcalls_impl.rs index e6c62860d5..bffb9c6b10 100644 --- a/crates/wasi-common/src/old/snapshot_0/sys/unix/bsd/hostcalls_impl.rs +++ b/crates/wasi-common/src/old/snapshot_0/sys/unix/bsd/hostcalls_impl.rs @@ -1,6 +1,6 @@ use crate::old::snapshot_0::hostcalls_impl::PathGet; use crate::old::snapshot_0::{Error, Result}; -use std::{io, os::unix::prelude::AsRawFd}; +use std::os::unix::prelude::AsRawFd; pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> { use yanix::file::{unlinkat, AtFlag}; @@ -12,34 +12,32 @@ pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> { ) } { Err(err) => { - if let yanix::Error::Io(ref errno) = err { - let raw_errno = errno.raw_os_error().unwrap(); - // 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 yanix::file::{fstatat, FileType}; + let raw_errno = err.raw_os_error().unwrap(); + // 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 yanix::file::{fstatat, FileType}; - if raw_errno == libc::EPERM { - match unsafe { - fstatat( - resolved.dirfd().as_raw_fd(), - resolved.path(), - AtFlag::SYMLINK_NOFOLLOW, - ) - } { - Ok(stat) => { - if FileType::from_stat_st_mode(stat.st_mode) == FileType::Directory { - return Err(io::Error::from_raw_os_error(libc::EISDIR).into()); - } - } - Err(err) => { - log::debug!("path_unlink_file fstatat error: {:?}", err); + if raw_errno == libc::EPERM { + match unsafe { + fstatat( + resolved.dirfd().as_raw_fd(), + resolved.path(), + AtFlag::SYMLINK_NOFOLLOW, + ) + } { + Ok(stat) => { + if FileType::from_stat_st_mode(stat.st_mode) == FileType::Directory { + return Err(Error::EISDIR); } } + Err(err) => { + log::debug!("path_unlink_file fstatat error: {:?}", err); + } } } @@ -57,25 +55,23 @@ pub(crate) fn path_symlink(old_path: &str, resolved: PathGet) -> Result<()> { match unsafe { symlinkat(old_path, resolved.dirfd().as_raw_fd(), resolved.path()) } { Err(err) => { - if let yanix::Error::Io(ref errno) = err { - if errno.raw_os_error().unwrap() == libc::ENOTDIR { - // On BSD, symlinkat returns ENOTDIR when it should in fact - // return a EEXIST. It seems that it gets confused with by - // the trailing slash in the target path. Thus, we strip - // the trailing slash and check if the path exists, and - // adjust the error code appropriately. - let new_path = resolved.path().trim_end_matches('/'); - match unsafe { - fstatat( - resolved.dirfd().as_raw_fd(), - new_path, - AtFlag::SYMLINK_NOFOLLOW, - ) - } { - Ok(_) => return Err(Error::EEXIST), - Err(err) => { - log::debug!("path_symlink fstatat error: {:?}", err); - } + if err.raw_os_error().unwrap() == libc::ENOTDIR { + // On BSD, symlinkat returns ENOTDIR when it should in fact + // return a EEXIST. It seems that it gets confused with by + // the trailing slash in the target path. Thus, we strip + // the trailing slash and check if the path exists, and + // adjust the error code appropriately. + let new_path = resolved.path().trim_end_matches('/'); + match unsafe { + fstatat( + resolved.dirfd().as_raw_fd(), + new_path, + AtFlag::SYMLINK_NOFOLLOW, + ) + } { + Ok(_) => return Err(Error::EEXIST), + Err(err) => { + log::debug!("path_symlink fstatat error: {:?}", err); } } } @@ -105,28 +101,26 @@ pub(crate) fn path_rename(resolved_old: PathGet, resolved_new: PathGet) -> Resul // // TODO // Verify on other BSD-based OSes. - if let yanix::Error::Io(ref errno) = err { - if errno.raw_os_error().unwrap() == libc::ENOENT { - // check if the source path exists - match unsafe { - fstatat( - resolved_old.dirfd().as_raw_fd(), - resolved_old.path(), - AtFlag::SYMLINK_NOFOLLOW, - ) - } { - Ok(_) => { - // check if destination contains a trailing slash - if resolved_new.path().contains('/') { - return Err(Error::ENOTDIR); - } else { - return Err(Error::ENOENT); - } - } - Err(err) => { - log::debug!("path_rename fstatat error: {:?}", err); + if err.raw_os_error().unwrap() == libc::ENOENT { + // check if the source path exists + match unsafe { + fstatat( + resolved_old.dirfd().as_raw_fd(), + resolved_old.path(), + AtFlag::SYMLINK_NOFOLLOW, + ) + } { + Ok(_) => { + // check if destination contains a trailing slash + if resolved_new.path().contains('/') { + return Err(Error::ENOTDIR); + } else { + return Err(Error::ENOENT); } } + Err(err) => { + log::debug!("path_rename fstatat error: {:?}", err); + } } } diff --git a/crates/wasi-common/src/old/snapshot_0/sys/unix/host_impl.rs b/crates/wasi-common/src/old/snapshot_0/sys/unix/host_impl.rs index d10e092d06..688fcc63e2 100644 --- a/crates/wasi-common/src/old/snapshot_0/sys/unix/host_impl.rs +++ b/crates/wasi-common/src/old/snapshot_0/sys/unix/host_impl.rs @@ -12,17 +12,6 @@ use yanix::file::OFlag; pub(crate) use sys_impl::host_impl::*; -impl From for Error { - fn from(err: yanix::Error) -> Self { - use yanix::Error::*; - match err { - Io(err) => err.into(), - Nul(err) => err.into(), - IntConversion(err) => err.into(), - } - } -} - impl FromRawOsError for Error { fn from_raw_os_error(code: i32) -> Self { match code { diff --git a/crates/wasi-common/src/old/snapshot_0/sys/unix/hostcalls_impl/fs.rs b/crates/wasi-common/src/old/snapshot_0/sys/unix/hostcalls_impl/fs.rs index 3e73a42c29..cace76529e 100644 --- a/crates/wasi-common/src/old/snapshot_0/sys/unix/hostcalls_impl/fs.rs +++ b/crates/wasi-common/src/old/snapshot_0/sys/unix/hostcalls_impl/fs.rs @@ -124,56 +124,54 @@ pub(crate) fn path_open( } { Ok(fd) => fd, Err(e) => { - if let yanix::Error::Io(ref err) = e { - match err.raw_os_error().unwrap() { - // Linux returns ENXIO instead of EOPNOTSUPP when opening a socket - libc::ENXIO => { - match unsafe { - fstatat( - resolved.dirfd().as_raw_fd(), - resolved.path(), - AtFlag::SYMLINK_NOFOLLOW, - ) - } { - Ok(stat) => { - if FileType::from_stat_st_mode(stat.st_mode) == FileType::Socket { - return Err(Error::ENOTSUP); - } - } - Err(err) => { - log::debug!("path_open fstatat error: {:?}", err); + match e.raw_os_error().unwrap() { + // Linux returns ENXIO instead of EOPNOTSUPP when opening a socket + libc::ENXIO => { + match unsafe { + fstatat( + resolved.dirfd().as_raw_fd(), + resolved.path(), + AtFlag::SYMLINK_NOFOLLOW, + ) + } { + Ok(stat) => { + if FileType::from_stat_st_mode(stat.st_mode) == FileType::Socket { + return Err(Error::ENOTSUP); } } - } - // Linux returns ENOTDIR instead of ELOOP when using O_NOFOLLOW|O_DIRECTORY - // on a symlink. - libc::ENOTDIR - if !(nix_all_oflags & (OFlag::NOFOLLOW | OFlag::DIRECTORY)).is_empty() => - { - match unsafe { - fstatat( - resolved.dirfd().as_raw_fd(), - resolved.path(), - AtFlag::SYMLINK_NOFOLLOW, - ) - } { - Ok(stat) => { - if FileType::from_stat_st_mode(stat.st_mode) == FileType::Symlink { - return Err(Error::ELOOP); - } - } - Err(err) => { - log::debug!("path_open fstatat error: {:?}", err); - } + Err(err) => { + log::debug!("path_open fstatat error: {:?}", err); } } - // FreeBSD returns EMLINK instead of ELOOP when using O_NOFOLLOW on - // a symlink. - libc::EMLINK if !(nix_all_oflags & OFlag::NOFOLLOW).is_empty() => { - return Err(Error::ELOOP); - } - _ => {} } + // Linux returns ENOTDIR instead of ELOOP when using O_NOFOLLOW|O_DIRECTORY + // on a symlink. + libc::ENOTDIR + if !(nix_all_oflags & (OFlag::NOFOLLOW | OFlag::DIRECTORY)).is_empty() => + { + match unsafe { + fstatat( + resolved.dirfd().as_raw_fd(), + resolved.path(), + AtFlag::SYMLINK_NOFOLLOW, + ) + } { + Ok(stat) => { + if FileType::from_stat_st_mode(stat.st_mode) == FileType::Symlink { + return Err(Error::ELOOP); + } + } + Err(err) => { + log::debug!("path_open fstatat error: {:?}", err); + } + } + } + // FreeBSD returns EMLINK instead of ELOOP when using O_NOFOLLOW on + // a symlink. + libc::EMLINK if !(nix_all_oflags & OFlag::NOFOLLOW).is_empty() => { + return Err(Error::ELOOP); + } + _ => {} } return Err(e.into()); diff --git a/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs b/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs index 207b2f4712..b82b917c1c 100644 --- a/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs +++ b/crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs @@ -1,6 +1,6 @@ use crate::hostcalls_impl::PathGet; use crate::{Error, Result}; -use std::{io, os::unix::prelude::AsRawFd}; +use std::os::unix::prelude::AsRawFd; pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> { use yanix::file::{unlinkat, AtFlag}; @@ -12,34 +12,32 @@ pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> { ) } { Err(err) => { - if let yanix::Error::Io(ref errno) = err { - let raw_errno = errno.raw_os_error().unwrap(); - // 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 yanix::file::{fstatat, FileType}; + let raw_errno = err.raw_os_error().unwrap(); + // 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 yanix::file::{fstatat, FileType}; - if raw_errno == libc::EPERM { - match unsafe { - fstatat( - resolved.dirfd().as_raw_fd(), - resolved.path(), - AtFlag::SYMLINK_NOFOLLOW, - ) - } { - Ok(stat) => { - if FileType::from_stat_st_mode(stat.st_mode) == FileType::Directory { - return Err(io::Error::from_raw_os_error(libc::EISDIR).into()); - } - } - Err(err) => { - log::debug!("path_unlink_file fstatat error: {:?}", err); + if raw_errno == libc::EPERM { + match unsafe { + fstatat( + resolved.dirfd().as_raw_fd(), + resolved.path(), + AtFlag::SYMLINK_NOFOLLOW, + ) + } { + Ok(stat) => { + if FileType::from_stat_st_mode(stat.st_mode) == FileType::Directory { + return Err(Error::EISDIR); } } + Err(err) => { + log::debug!("path_unlink_file fstatat error: {:?}", err); + } } } @@ -57,25 +55,23 @@ pub(crate) fn path_symlink(old_path: &str, resolved: PathGet) -> Result<()> { match unsafe { symlinkat(old_path, resolved.dirfd().as_raw_fd(), resolved.path()) } { Err(err) => { - if let yanix::Error::Io(ref errno) = err { - if errno.raw_os_error().unwrap() == libc::ENOTDIR { - // On BSD, symlinkat returns ENOTDIR when it should in fact - // return a EEXIST. It seems that it gets confused with by - // the trailing slash in the target path. Thus, we strip - // the trailing slash and check if the path exists, and - // adjust the error code appropriately. - let new_path = resolved.path().trim_end_matches('/'); - match unsafe { - fstatat( - resolved.dirfd().as_raw_fd(), - new_path, - AtFlag::SYMLINK_NOFOLLOW, - ) - } { - Ok(_) => return Err(Error::EEXIST), - Err(err) => { - log::debug!("path_symlink fstatat error: {:?}", err); - } + if err.raw_os_error().unwrap() == libc::ENOTDIR { + // On BSD, symlinkat returns ENOTDIR when it should in fact + // return a EEXIST. It seems that it gets confused with by + // the trailing slash in the target path. Thus, we strip + // the trailing slash and check if the path exists, and + // adjust the error code appropriately. + let new_path = resolved.path().trim_end_matches('/'); + match unsafe { + fstatat( + resolved.dirfd().as_raw_fd(), + new_path, + AtFlag::SYMLINK_NOFOLLOW, + ) + } { + Ok(_) => return Err(Error::EEXIST), + Err(err) => { + log::debug!("path_symlink fstatat error: {:?}", err); } } } @@ -105,28 +101,26 @@ pub(crate) fn path_rename(resolved_old: PathGet, resolved_new: PathGet) -> Resul // // TODO // Verify on other BSD-based OSes. - if let yanix::Error::Io(ref errno) = err { - if errno.raw_os_error().unwrap() == libc::ENOENT { - // check if the source path exists - match unsafe { - fstatat( - resolved_old.dirfd().as_raw_fd(), - resolved_old.path(), - AtFlag::SYMLINK_NOFOLLOW, - ) - } { - Ok(_) => { - // check if destination contains a trailing slash - if resolved_new.path().contains('/') { - return Err(Error::ENOTDIR); - } else { - return Err(Error::ENOENT); - } - } - Err(err) => { - log::debug!("path_rename fstatat error: {:?}", err); + if err.raw_os_error().unwrap() == libc::ENOENT { + // check if the source path exists + match unsafe { + fstatat( + resolved_old.dirfd().as_raw_fd(), + resolved_old.path(), + AtFlag::SYMLINK_NOFOLLOW, + ) + } { + Ok(_) => { + // check if destination contains a trailing slash + if resolved_new.path().contains('/') { + return Err(Error::ENOTDIR); + } else { + return Err(Error::ENOENT); } } + Err(err) => { + log::debug!("path_rename fstatat error: {:?}", err); + } } } diff --git a/crates/wasi-common/src/sys/unix/host_impl.rs b/crates/wasi-common/src/sys/unix/host_impl.rs index c88bd2198a..dea0b106eb 100644 --- a/crates/wasi-common/src/sys/unix/host_impl.rs +++ b/crates/wasi-common/src/sys/unix/host_impl.rs @@ -10,17 +10,6 @@ use yanix::file::OFlag; pub(crate) use sys_impl::host_impl::*; -impl From for Error { - fn from(err: yanix::Error) -> Self { - use yanix::Error::*; - match err { - Io(err) => err.into(), - Nul(err) => err.into(), - IntConversion(err) => err.into(), - } - } -} - impl FromRawOsError for Error { fn from_raw_os_error(code: i32) -> Self { match code { diff --git a/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs b/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs index ea01628b87..2a7c01db74 100644 --- a/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs +++ b/crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs @@ -124,56 +124,54 @@ pub(crate) fn path_open( let new_fd = match fd_no { Ok(fd) => fd, Err(e) => { - if let yanix::Error::Io(ref err) = e { - match err.raw_os_error().unwrap() { - // Linux returns ENXIO instead of EOPNOTSUPP when opening a socket - libc::ENXIO => { - match unsafe { - fstatat( - resolved.dirfd().as_raw_fd(), - resolved.path(), - AtFlag::SYMLINK_NOFOLLOW, - ) - } { - Ok(stat) => { - if FileType::from_stat_st_mode(stat.st_mode) == FileType::Socket { - return Err(Error::ENOTSUP); - } - } - Err(err) => { - log::debug!("path_open fstatat error: {:?}", err); + match e.raw_os_error().unwrap() { + // Linux returns ENXIO instead of EOPNOTSUPP when opening a socket + libc::ENXIO => { + match unsafe { + fstatat( + resolved.dirfd().as_raw_fd(), + resolved.path(), + AtFlag::SYMLINK_NOFOLLOW, + ) + } { + Ok(stat) => { + if FileType::from_stat_st_mode(stat.st_mode) == FileType::Socket { + return Err(Error::ENOTSUP); } } - } - // Linux returns ENOTDIR instead of ELOOP when using O_NOFOLLOW|O_DIRECTORY - // on a symlink. - libc::ENOTDIR - if !(nix_all_oflags & (OFlag::NOFOLLOW | OFlag::DIRECTORY)).is_empty() => - { - match unsafe { - fstatat( - resolved.dirfd().as_raw_fd(), - resolved.path(), - AtFlag::SYMLINK_NOFOLLOW, - ) - } { - Ok(stat) => { - if FileType::from_stat_st_mode(stat.st_mode) == FileType::Symlink { - return Err(Error::ELOOP); - } - } - Err(err) => { - log::debug!("path_open fstatat error: {:?}", err); - } + Err(err) => { + log::debug!("path_open fstatat error: {:?}", err); } } - // FreeBSD returns EMLINK instead of ELOOP when using O_NOFOLLOW on - // a symlink. - libc::EMLINK if !(nix_all_oflags & OFlag::NOFOLLOW).is_empty() => { - return Err(Error::ELOOP); - } - _ => {} } + // Linux returns ENOTDIR instead of ELOOP when using O_NOFOLLOW|O_DIRECTORY + // on a symlink. + libc::ENOTDIR + if !(nix_all_oflags & (OFlag::NOFOLLOW | OFlag::DIRECTORY)).is_empty() => + { + match unsafe { + fstatat( + resolved.dirfd().as_raw_fd(), + resolved.path(), + AtFlag::SYMLINK_NOFOLLOW, + ) + } { + Ok(stat) => { + if FileType::from_stat_st_mode(stat.st_mode) == FileType::Symlink { + return Err(Error::ELOOP); + } + } + Err(err) => { + log::debug!("path_open fstatat error: {:?}", err); + } + } + } + // FreeBSD returns EMLINK instead of ELOOP when using O_NOFOLLOW on + // a symlink. + libc::EMLINK if !(nix_all_oflags & OFlag::NOFOLLOW).is_empty() => { + return Err(Error::ELOOP); + } + _ => {} } return Err(e.into()); diff --git a/crates/wasi-common/yanix/Cargo.toml b/crates/wasi-common/yanix/Cargo.toml index c1e77c9459..e5e30f7d72 100644 --- a/crates/wasi-common/yanix/Cargo.toml +++ b/crates/wasi-common/yanix/Cargo.toml @@ -10,7 +10,6 @@ edition = "2018" [dependencies] log = "0.4" libc = { version = "0.2", features = ["extra_traits"] } -thiserror = "1.0" bitflags = "1.2" cfg-if = "0.1.9" diff --git a/crates/wasi-common/yanix/src/clock.rs b/crates/wasi-common/yanix/src/clock.rs index 4d1e105b4e..34e0b9947b 100644 --- a/crates/wasi-common/yanix/src/clock.rs +++ b/crates/wasi-common/yanix/src/clock.rs @@ -1,4 +1,5 @@ -use crate::{Error, Result}; +use crate::from_success_code; +use std::io::Result; use std::mem::MaybeUninit; #[derive(Debug, Copy, Clone)] @@ -22,16 +23,12 @@ impl ClockId { pub fn clock_getres(clock_id: ClockId) -> Result { let mut timespec = MaybeUninit::::uninit(); - Error::from_success_code(unsafe { - libc::clock_getres(clock_id.as_raw(), timespec.as_mut_ptr()) - })?; + from_success_code(unsafe { libc::clock_getres(clock_id.as_raw(), timespec.as_mut_ptr()) })?; Ok(unsafe { timespec.assume_init() }) } pub fn clock_gettime(clock_id: ClockId) -> Result { let mut timespec = MaybeUninit::::uninit(); - Error::from_success_code(unsafe { - libc::clock_gettime(clock_id.as_raw(), timespec.as_mut_ptr()) - })?; + from_success_code(unsafe { libc::clock_gettime(clock_id.as_raw(), timespec.as_mut_ptr()) })?; Ok(unsafe { timespec.assume_init() }) } diff --git a/crates/wasi-common/yanix/src/dir.rs b/crates/wasi-common/yanix/src/dir.rs index 55ef823a81..00b2b40898 100644 --- a/crates/wasi-common/yanix/src/dir.rs +++ b/crates/wasi-common/yanix/src/dir.rs @@ -1,8 +1,8 @@ use crate::{ file::FileType, sys::dir::{iter_impl, EntryImpl}, - Result, }; +use std::io::Result; use std::os::unix::io::{AsRawFd, IntoRawFd, RawFd}; use std::{ffi::CStr, io, ops::Deref, ptr}; diff --git a/crates/wasi-common/yanix/src/error.rs b/crates/wasi-common/yanix/src/error.rs deleted file mode 100644 index f59c046aee..0000000000 --- a/crates/wasi-common/yanix/src/error.rs +++ /dev/null @@ -1,61 +0,0 @@ -//! Error type -use crate::Result; -use std::{ffi, io, num}; - -#[derive(Debug, thiserror::Error)] -pub enum Error { - #[error("I/O error {0}")] - Io(#[from] io::Error), - #[error("a nul byte was not found in the expected position")] - Nul(#[from] ffi::NulError), - #[error("integral type conversion failed")] - IntConversion(#[from] num::TryFromIntError), -} - -impl Error { - pub fn from_success_code(t: T) -> Result<()> { - if t.is_zero() { - Ok(()) - } else { - Err(Self::from(io::Error::last_os_error())) - } - } - - pub fn from_result(t: T) -> Result { - if t.is_minus_one() { - Err(Self::from(io::Error::last_os_error())) - } else { - Ok(t) - } - } -} - -#[doc(hidden)] -pub trait IsZero { - fn is_zero(&self) -> bool; -} - -macro_rules! impl_is_zero { - ($($t:ident)*) => ($(impl IsZero for $t { - fn is_zero(&self) -> bool { - *self == 0 - } - })*) -} - -impl_is_zero! { i32 i64 isize } - -#[doc(hidden)] -pub trait IsMinusOne { - fn is_minus_one(&self) -> bool; -} - -macro_rules! impl_is_minus_one { - ($($t:ident)*) => ($(impl IsMinusOne for $t { - fn is_minus_one(&self) -> bool { - *self == -1 - } - })*) -} - -impl_is_minus_one! { i32 i64 isize } diff --git a/crates/wasi-common/yanix/src/fcntl.rs b/crates/wasi-common/yanix/src/fcntl.rs index 3b95cbffe9..b82e41cfc8 100644 --- a/crates/wasi-common/yanix/src/fcntl.rs +++ b/crates/wasi-common/yanix/src/fcntl.rs @@ -1,7 +1,8 @@ use crate::{ file::{FdFlag, OFlag}, - Error, Result, + from_result, from_success_code, }; +use std::io::Result; use std::os::unix::prelude::*; pub unsafe fn dup_fd(fd: RawFd, close_on_exec: bool) -> Result { @@ -9,7 +10,7 @@ pub unsafe fn dup_fd(fd: RawFd, close_on_exec: bool) -> Result { // the minimum duplicated RawFd number. In our case, I don't // think we have to worry about this that much, so passing in // the RawFd descriptor we want duplicated - Error::from_result(if close_on_exec { + from_result(if close_on_exec { libc::fcntl(fd, libc::F_DUPFD_CLOEXEC, fd) } else { libc::fcntl(fd, libc::F_DUPFD, fd) @@ -17,17 +18,17 @@ pub unsafe fn dup_fd(fd: RawFd, close_on_exec: bool) -> Result { } pub unsafe fn get_fd_flags(fd: RawFd) -> Result { - Error::from_result(libc::fcntl(fd, libc::F_GETFD)).map(FdFlag::from_bits_truncate) + from_result(libc::fcntl(fd, libc::F_GETFD)).map(FdFlag::from_bits_truncate) } pub unsafe fn set_fd_flags(fd: RawFd, flags: FdFlag) -> Result<()> { - Error::from_success_code(libc::fcntl(fd, libc::F_SETFD, flags.bits())) + from_success_code(libc::fcntl(fd, libc::F_SETFD, flags.bits())) } pub unsafe fn get_status_flags(fd: RawFd) -> Result { - Error::from_result(libc::fcntl(fd, libc::F_GETFL)).map(OFlag::from_bits_truncate) + from_result(libc::fcntl(fd, libc::F_GETFL)).map(OFlag::from_bits_truncate) } pub unsafe fn set_status_flags(fd: RawFd, flags: OFlag) -> Result<()> { - Error::from_success_code(libc::fcntl(fd, libc::F_SETFL, flags.bits())) + from_success_code(libc::fcntl(fd, libc::F_SETFL, flags.bits())) } diff --git a/crates/wasi-common/yanix/src/file.rs b/crates/wasi-common/yanix/src/file.rs index 397740cea4..c16c4ac8bd 100644 --- a/crates/wasi-common/yanix/src/file.rs +++ b/crates/wasi-common/yanix/src/file.rs @@ -1,9 +1,10 @@ -use crate::{Error, Result}; +use crate::{from_result, from_success_code}; use bitflags::bitflags; use cfg_if::cfg_if; use std::{ convert::TryInto, ffi::{CString, OsStr, OsString}, + io::Result, os::unix::prelude::*, }; @@ -137,7 +138,7 @@ pub unsafe fn openat>( mode: Mode, ) -> Result { let path = CString::new(path.as_ref().as_bytes())?; - Error::from_result(libc::openat( + from_result(libc::openat( dirfd, path.as_ptr(), oflag.bits(), @@ -148,21 +149,23 @@ pub unsafe fn openat>( pub unsafe fn readlinkat>(dirfd: RawFd, path: P) -> Result { let path = CString::new(path.as_ref().as_bytes())?; let buffer = &mut [0u8; libc::PATH_MAX as usize + 1]; - Error::from_result(libc::readlinkat( + let nread = from_result(libc::readlinkat( dirfd, path.as_ptr(), buffer.as_mut_ptr() as *mut _, buffer.len(), - )) - .and_then(|nread| { - let link = OsStr::from_bytes(&buffer[0..nread.try_into()?]); - Ok(link.into()) - }) + ))?; + // We can just unwrap() this, because readlinkat returns an ssize_t which is either -1 + // (handled above) or non-negative and will fit in a size_t/usize, which is what we're + // converting it to here. + let nread = nread.try_into().unwrap(); + let link = OsStr::from_bytes(&buffer[0..nread]); + Ok(link.into()) } pub unsafe fn mkdirat>(dirfd: RawFd, path: P, mode: Mode) -> Result<()> { let path = CString::new(path.as_ref().as_bytes())?; - Error::from_success_code(libc::mkdirat(dirfd, path.as_ptr(), mode.bits())) + from_success_code(libc::mkdirat(dirfd, path.as_ptr(), mode.bits())) } pub unsafe fn linkat>( @@ -174,7 +177,7 @@ pub unsafe fn linkat>( ) -> Result<()> { let old_path = CString::new(old_path.as_ref().as_bytes())?; let new_path = CString::new(new_path.as_ref().as_bytes())?; - Error::from_success_code(libc::linkat( + from_success_code(libc::linkat( old_dirfd, old_path.as_ptr(), new_dirfd, @@ -185,7 +188,7 @@ pub unsafe fn linkat>( pub unsafe fn unlinkat>(dirfd: RawFd, path: P, flags: AtFlag) -> Result<()> { let path = CString::new(path.as_ref().as_bytes())?; - Error::from_success_code(libc::unlinkat(dirfd, path.as_ptr(), flags.bits())) + from_success_code(libc::unlinkat(dirfd, path.as_ptr(), flags.bits())) } pub unsafe fn renameat>( @@ -196,7 +199,7 @@ pub unsafe fn renameat>( ) -> Result<()> { let old_path = CString::new(old_path.as_ref().as_bytes())?; let new_path = CString::new(new_path.as_ref().as_bytes())?; - Error::from_success_code(libc::renameat( + from_success_code(libc::renameat( old_dirfd, old_path.as_ptr(), new_dirfd, @@ -207,7 +210,7 @@ pub unsafe fn renameat>( pub unsafe fn symlinkat>(old_path: P, new_dirfd: RawFd, new_path: P) -> Result<()> { let old_path = CString::new(old_path.as_ref().as_bytes())?; let new_path = CString::new(new_path.as_ref().as_bytes())?; - Error::from_success_code(libc::symlinkat( + from_success_code(libc::symlinkat( old_path.as_ptr(), new_dirfd, new_path.as_ptr(), @@ -218,7 +221,7 @@ pub unsafe fn fstatat>(dirfd: RawFd, path: P, flags: AtFlag) -> use std::mem::MaybeUninit; let path = CString::new(path.as_ref().as_bytes())?; let mut filestat = MaybeUninit::::uninit(); - Error::from_result(libc::fstatat( + from_result(libc::fstatat( dirfd, path.as_ptr(), filestat.as_mut_ptr(), @@ -230,20 +233,26 @@ pub unsafe fn fstatat>(dirfd: RawFd, path: P, flags: AtFlag) -> pub unsafe fn fstat(fd: RawFd) -> Result { use std::mem::MaybeUninit; let mut filestat = MaybeUninit::::uninit(); - Error::from_result(libc::fstat(fd, filestat.as_mut_ptr()))?; + from_result(libc::fstat(fd, filestat.as_mut_ptr()))?; Ok(filestat.assume_init()) } /// `fionread()` function, equivalent to `ioctl(fd, FIONREAD, *bytes)`. pub unsafe fn fionread(fd: RawFd) -> Result { let mut nread: libc::c_int = 0; - Error::from_result(libc::ioctl(fd, libc::FIONREAD, &mut nread as *mut _))?; - Ok(nread.try_into()?) + from_result(libc::ioctl(fd, libc::FIONREAD, &mut nread as *mut _))?; + // FIONREAD returns a non-negative int if it doesn't fail, or it'll fit in a u32 if it does. + // + // For the future, if we want to be super cautious and avoid assuming int is 32-bit, we could + // widen fionread's return type here, since the one place that calls it wants a u64 anyway. + Ok(nread.try_into().unwrap()) } /// This function is unsafe because it operates on a raw file descriptor. /// It's provided, because std::io::Seek requires a mutable borrow. pub unsafe fn tell(fd: RawFd) -> Result { - let offset: i64 = Error::from_result(libc::lseek(fd, 0, libc::SEEK_CUR))?; - Ok(offset.try_into()?) + let offset = from_result(libc::lseek(fd, 0, libc::SEEK_CUR))?; + // lseek returns an off_t, which we can assume is a non-negative i64 if it doesn't fail. + // So we can unwrap() this conversion. + Ok(offset.try_into().unwrap()) } diff --git a/crates/wasi-common/yanix/src/lib.rs b/crates/wasi-common/yanix/src/lib.rs index 7f9361a4e9..fded31d6cd 100644 --- a/crates/wasi-common/yanix/src/lib.rs +++ b/crates/wasi-common/yanix/src/lib.rs @@ -16,12 +16,54 @@ pub mod file; pub mod poll; pub mod socket; -mod error; mod sys; pub mod fadvise { pub use super::sys::fadvise::*; } -pub use error::Error; -pub type Result = std::result::Result; +use std::io::{Error, Result}; + +fn from_success_code(t: T) -> Result<()> { + if t.is_zero() { + Ok(()) + } else { + Err(Error::last_os_error()) + } +} + +fn from_result(t: T) -> Result { + if t.is_minus_one() { + Err(Error::last_os_error()) + } else { + Ok(t) + } +} + +trait IsZero { + fn is_zero(&self) -> bool; +} + +macro_rules! impl_is_zero { + ($($t:ident)*) => ($(impl IsZero for $t { + fn is_zero(&self) -> bool { + *self == 0 + } + })*) +} + +impl_is_zero! { i32 i64 isize } + +trait IsMinusOne { + fn is_minus_one(&self) -> bool; +} + +macro_rules! impl_is_minus_one { + ($($t:ident)*) => ($(impl IsMinusOne for $t { + fn is_minus_one(&self) -> bool { + *self == -1 + } + })*) +} + +impl_is_minus_one! { i32 i64 isize } diff --git a/crates/wasi-common/yanix/src/poll.rs b/crates/wasi-common/yanix/src/poll.rs index 315956bc3e..838c7ad331 100644 --- a/crates/wasi-common/yanix/src/poll.rs +++ b/crates/wasi-common/yanix/src/poll.rs @@ -1,6 +1,6 @@ -use crate::{Error, Result}; +use crate::from_result; use bitflags::bitflags; -use std::{convert::TryInto, os::unix::prelude::*}; +use std::{convert::TryInto, io::Result, os::unix::prelude::*}; bitflags! { pub struct PollFlags: libc::c_short { @@ -36,12 +36,14 @@ impl PollFd { } pub fn poll(fds: &mut [PollFd], timeout: libc::c_int) -> Result { - Error::from_result(unsafe { + let nready = from_result(unsafe { libc::poll( fds.as_mut_ptr() as *mut libc::pollfd, fds.len() as libc::nfds_t, timeout, ) - }) - .and_then(|nready| nready.try_into().map_err(Into::into)) + })?; + // When poll doesn't fail, its return value is a non-negative int, which will + // always be convertable to usize, so we can unwrap() here. + Ok(nready.try_into().unwrap()) } diff --git a/crates/wasi-common/yanix/src/socket.rs b/crates/wasi-common/yanix/src/socket.rs index 22e5d488f1..f5de22decd 100644 --- a/crates/wasi-common/yanix/src/socket.rs +++ b/crates/wasi-common/yanix/src/socket.rs @@ -1,4 +1,5 @@ -use crate::{Error, Result}; +use crate::from_success_code; +use std::io::Result; use std::os::unix::prelude::*; #[derive(Debug, Clone, Copy)] @@ -15,7 +16,7 @@ pub unsafe fn get_socket_type(fd: RawFd) -> Result { use std::mem::{self, MaybeUninit}; let mut buffer = MaybeUninit::::zeroed().assume_init(); let mut out_len = mem::size_of::() as libc::socklen_t; - Error::from_success_code(libc::getsockopt( + from_success_code(libc::getsockopt( fd, libc::SOL_SOCKET, libc::SO_TYPE, diff --git a/crates/wasi-common/yanix/src/sys/bsd/dir.rs b/crates/wasi-common/yanix/src/sys/bsd/dir.rs index adc55ce2a2..6d1b75cc29 100644 --- a/crates/wasi-common/yanix/src/sys/bsd/dir.rs +++ b/crates/wasi-common/yanix/src/sys/bsd/dir.rs @@ -1,8 +1,8 @@ -use crate::{ - dir::{Dir, Entry, EntryExt, SeekLoc}, - Result, +use crate::dir::{Dir, Entry, EntryExt, SeekLoc}; +use std::{ + io::{Error, Result}, + ops::Deref, }; -use std::{io, ops::Deref}; #[derive(Copy, Clone, Debug)] pub(crate) struct EntryImpl { @@ -19,17 +19,17 @@ impl Deref for EntryImpl { } pub(crate) fn iter_impl(dir: &Dir) -> Option> { - let errno = io::Error::last_os_error(); + let errno = Error::last_os_error(); let dirent = unsafe { libc::readdir(dir.as_raw().as_ptr()) }; if dirent.is_null() { - let curr_errno = io::Error::last_os_error(); + let curr_errno = Error::last_os_error(); if errno.raw_os_error() != curr_errno.raw_os_error() { // TODO This should be verified on different BSD-flavours. // // According to 4.3BSD/POSIX.1-2001 man pages, there was an error // if the errno value has changed at some point during the sequence // of readdir calls. - Some(Err(curr_errno.into())) + Some(Err(curr_errno)) } else { // Not an error. We've simply reached the end of the stream. None diff --git a/crates/wasi-common/yanix/src/sys/bsd/fadvise.rs b/crates/wasi-common/yanix/src/sys/bsd/fadvise.rs index 97baca65a3..d1fa29d9d9 100644 --- a/crates/wasi-common/yanix/src/sys/bsd/fadvise.rs +++ b/crates/wasi-common/yanix/src/sys/bsd/fadvise.rs @@ -1,5 +1,5 @@ -use crate::{Error, Result}; -use std::{convert::TryInto, os::unix::prelude::*}; +use crate::from_success_code; +use std::{convert::TryInto, io::Result, os::unix::prelude::*}; #[cfg(not(any(target_os = "freebsd", target_os = "netbsd")))] #[derive(Debug, Copy, Clone)] @@ -44,11 +44,22 @@ pub unsafe fn posix_fadvise( // off_t ra_offset; /* offset into the file */ // int ra_count; /* size of the read */ // }; + let ra_count = match len.try_into() { + Ok(ra_count) => ra_count, + Err(_) => { + // This conversion can fail, because it's converting into int. But in that case, the user + // is providing a dubiously large hint. This is not confirmed (no helpful info in the man + // pages), but offhand, a 2+ GiB advisory read async seems unlikely to help with any kind + // of performance, so we log and exit early with a no-op. + log::warn!("`len` too big to fit in the host's command. Returning early with no-op!"); + return Ok(()); + } + }; let advisory = libc::radvisory { ra_offset: offset, - ra_count: len.try_into()?, + ra_count, }; - Error::from_success_code(libc::fcntl(fd, libc::F_RDADVISE, &advisory)) + from_success_code(libc::fcntl(fd, libc::F_RDADVISE, &advisory)) } #[cfg(any(target_os = "freebsd", target_os = "netbsd"))] @@ -58,7 +69,7 @@ pub unsafe fn posix_fadvise( len: libc::off_t, advice: PosixFadviseAdvice, ) -> Result<()> { - Error::from_success_code(libc::posix_fadvise(fd, offset, len, advice as libc::c_int)) + from_success_code(libc::posix_fadvise(fd, offset, len, advice as libc::c_int)) } // On BSDs without support we leave it as no-op diff --git a/crates/wasi-common/yanix/src/sys/bsd/file.rs b/crates/wasi-common/yanix/src/sys/bsd/file.rs index e17355d968..142c51aa1c 100644 --- a/crates/wasi-common/yanix/src/sys/bsd/file.rs +++ b/crates/wasi-common/yanix/src/sys/bsd/file.rs @@ -1,5 +1,7 @@ -use crate::Result; -use std::{io, os::unix::prelude::*}; +use std::{ + io::{Error, Result}, + os::unix::prelude::*, +}; pub unsafe fn isatty(fd: RawFd) -> Result { let res = libc::isatty(fd); @@ -8,15 +10,11 @@ pub unsafe fn isatty(fd: RawFd) -> Result { Ok(true) } else { // ... otherwise 0 is returned, and errno is set to indicate the error. - let errno = io::Error::last_os_error(); - if let Some(raw_errno) = errno.raw_os_error() { - if raw_errno == libc::ENOTTY { - Ok(false) - } else { - Err(errno.into()) - } + let errno = Error::last_os_error(); + if errno.raw_os_error().unwrap() == libc::ENOTTY { + Ok(false) } else { - Err(errno.into()) + Err(errno) } } } diff --git a/crates/wasi-common/yanix/src/sys/emscripten/mod.rs b/crates/wasi-common/yanix/src/sys/emscripten/mod.rs index b9edc97e32..1ee0bdbda4 100644 --- a/crates/wasi-common/yanix/src/sys/emscripten/mod.rs +++ b/crates/wasi-common/yanix/src/sys/emscripten/mod.rs @@ -5,12 +5,20 @@ pub(crate) mod fadvise; #[path = "../linux/file.rs"] pub(crate) mod file; -use crate::{dir::SeekLoc, Result}; +use crate::dir::SeekLoc; use std::convert::TryInto; +use std::io::{Error, Result}; impl SeekLoc { pub unsafe fn from_raw(loc: i64) -> Result { - let loc = loc.try_into()?; + // The cookie (or `loc`) is an opaque value, and applications aren't supposed to do + // arithmetic on them or pick their own values or have any awareness of the numeric + // range of the values. They're just supposed to pass back in the values that we + // give them. And any value we give them will be convertable back to `long`, + // because that's the type the OS gives them to us in. So return an `EINVAL`. + let loc = loc + .try_into() + .map_err(|_| Error::from_raw_os_error(libc::EINVAL))?; Ok(Self(loc)) } } diff --git a/crates/wasi-common/yanix/src/sys/linux/dir.rs b/crates/wasi-common/yanix/src/sys/linux/dir.rs index 9fc3ce1bab..d7f25f43f0 100644 --- a/crates/wasi-common/yanix/src/sys/linux/dir.rs +++ b/crates/wasi-common/yanix/src/sys/linux/dir.rs @@ -1,8 +1,8 @@ -use crate::{ - dir::{Dir, Entry, EntryExt, SeekLoc}, - Result, +use crate::dir::{Dir, Entry, EntryExt, SeekLoc}; +use std::{ + io::{Error, Result}, + ops::Deref, }; -use std::{io, ops::Deref}; #[derive(Copy, Clone, Debug)] pub(crate) struct EntryImpl(libc::dirent64); @@ -26,17 +26,17 @@ impl EntryExt for Entry { } pub(crate) fn iter_impl(dir: &Dir) -> Option> { - let errno = io::Error::last_os_error(); + let errno = Error::last_os_error(); let dirent = unsafe { libc::readdir64(dir.as_raw().as_ptr()) }; if dirent.is_null() { - let curr_errno = io::Error::last_os_error(); + let curr_errno = Error::last_os_error(); if errno.raw_os_error() != curr_errno.raw_os_error() { // TODO This should be verified on different BSD-flavours. // // According to 4.3BSD/POSIX.1-2001 man pages, there was an error // if the errno value has changed at some point during the sequence // of readdir calls. - Some(Err(curr_errno.into())) + Some(Err(curr_errno)) } else { // Not an error. We've simply reached the end of the stream. None diff --git a/crates/wasi-common/yanix/src/sys/linux/fadvise.rs b/crates/wasi-common/yanix/src/sys/linux/fadvise.rs index fc2c60c70e..c3e2354862 100644 --- a/crates/wasi-common/yanix/src/sys/linux/fadvise.rs +++ b/crates/wasi-common/yanix/src/sys/linux/fadvise.rs @@ -1,4 +1,5 @@ -use crate::{Error, Result}; +use crate::from_success_code; +use std::io::Result; use std::os::unix::prelude::*; #[derive(Debug, Copy, Clone)] @@ -18,5 +19,5 @@ pub unsafe fn posix_fadvise( len: libc::off_t, advice: PosixFadviseAdvice, ) -> Result<()> { - Error::from_success_code(libc::posix_fadvise(fd, offset, len, advice as libc::c_int)) + from_success_code(libc::posix_fadvise(fd, offset, len, advice as libc::c_int)) } diff --git a/crates/wasi-common/yanix/src/sys/linux/file.rs b/crates/wasi-common/yanix/src/sys/linux/file.rs index 98798b7999..e039f7bb84 100644 --- a/crates/wasi-common/yanix/src/sys/linux/file.rs +++ b/crates/wasi-common/yanix/src/sys/linux/file.rs @@ -1,5 +1,7 @@ -use crate::Result; -use std::{io, os::unix::prelude::*}; +use std::{ + io::{Error, Result}, + os::unix::prelude::*, +}; pub unsafe fn isatty(fd: RawFd) -> Result { let res = libc::isatty(fd); @@ -8,20 +10,17 @@ pub unsafe fn isatty(fd: RawFd) -> Result { Ok(true) } else { // ... otherwise 0 is returned, and errno is set to indicate the error. - let errno = io::Error::last_os_error(); - if let Some(raw_errno) = errno.raw_os_error() { - // While POSIX specifies ENOTTY if the passed - // fd is *not* a tty, on Linux, some implementations - // may return EINVAL instead. - // - // https://linux.die.net/man/3/isatty - if raw_errno == libc::ENOTTY || raw_errno == libc::EINVAL { - Ok(false) - } else { - Err(errno.into()) - } + let errno = Error::last_os_error(); + let raw_errno = errno.raw_os_error().unwrap(); + // While POSIX specifies ENOTTY if the passed + // fd is *not* a tty, on Linux, some implementations + // may return EINVAL instead. + // + // https://linux.die.net/man/3/isatty + if raw_errno == libc::ENOTTY || raw_errno == libc::EINVAL { + Ok(false) } else { - Err(errno.into()) + Err(errno) } } } diff --git a/crates/wasi-common/yanix/src/sys/linux/mod.rs b/crates/wasi-common/yanix/src/sys/linux/mod.rs index d7beaa7cbe..e386fccad2 100644 --- a/crates/wasi-common/yanix/src/sys/linux/mod.rs +++ b/crates/wasi-common/yanix/src/sys/linux/mod.rs @@ -2,7 +2,8 @@ pub(crate) mod dir; pub(crate) mod fadvise; pub(crate) mod file; -use crate::{dir::SeekLoc, Result}; +use crate::dir::SeekLoc; +use std::io::Result; impl SeekLoc { pub unsafe fn from_raw(loc: i64) -> Result { diff --git a/crates/wasi-common/yanix/src/sys/mod.rs b/crates/wasi-common/yanix/src/sys/mod.rs index 3743f6f190..e6bd0e36dc 100644 --- a/crates/wasi-common/yanix/src/sys/mod.rs +++ b/crates/wasi-common/yanix/src/sys/mod.rs @@ -1,5 +1,6 @@ -use crate::{dir::SeekLoc, Result}; +use crate::dir::SeekLoc; use cfg_if::cfg_if; +use std::io::Result; cfg_if! { if #[cfg(any(target_os = "linux",