T0b1/wasmtime
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

[wasi-common]: yanix now returns io::Error directly (#1242)

Browse Source 
* 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<ffi::NulError> 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
This commit is contained in:
Jakub Konka
2020-03-06 23:20:54 +01:00
committed by GitHub
parent 55337abd3f
commit 42fae4e3b8
25 changed files with 368 additions and 398 deletions
Download Patch File Download Diff File Expand all files Collapse all files

124
crates/wasi-common/src/old/snapshot_0/sys/unix/bsd/hostcalls_impl.rs
View File

@@ -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);
}
}
}

11
crates/wasi-common/src/old/snapshot_0/sys/unix/host_impl.rs
View File

@@ -12,17 +12,6 @@ use yanix::file::OFlag;
pub(crate) use sys_impl::host_impl::*;
impl From<yanix::Error> 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 {

88
crates/wasi-common/src/old/snapshot_0/sys/unix/hostcalls_impl/fs.rs
View File

@@ -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());

124
crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs
View File

@@ -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);
}
}
}

11
crates/wasi-common/src/sys/unix/host_impl.rs
View File

@@ -10,17 +10,6 @@ use yanix::file::OFlag;
pub(crate) use sys_impl::host_impl::*;
impl From<yanix::Error> 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 {

88
crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs
View File

@@ -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());