Skip to content

Commit

Permalink
Refactor and combine all FileType structs in yanix
Browse files Browse the repository at this point in the history
This commit does a bit of everything: refactors bits here and there,
fixes a bug discovered in another #701, and combines all structs that
we used in `yanix` and `wasi-common` crates to represent file types
on *nix into one struct, `yanix::file::FileType`.

Up until now, in `yanix`, we've had two separate structs used to
represent file types on the host: `yanix::dir::FileType` and
`yanix::file::SFlags` (well, not quite, but that was its main use).
They both were used in different context (the former when parsing
`dirent` struct, and the latter when parsing `stat` struct), they
were C-compatible (as far as their representation goes), and as it
turns out, they shared possible enumeration values. This commit
combines them both into an idiomatic Rust enum with the caveat that
it is now *not* C-compatible, however, I couldn't find a single use
where that would actually matter, and even if it does in the future,
we can simply add appropriate impl methods.

The combine `yanix::file::FileType` struct can be constructed in two
ways: 1) either from `stat.st_mode` value (and while we're here,
now it's done correctly according to POSIX which fixes the bug mentioned
in VFS impl PR #701), or 2) from `dirent.d_type` value. Also, since we now
have one struct for representing both contexts, this cleans up nicely
a lot of duplicated code in `host` module.
  • Loading branch information
Jakub Konka authored and kubkon committed Feb 24, 2020
1 parent aa78d49 commit 4fe397e
Show file tree
Hide file tree
Showing 8 changed files with 75 additions and 164 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> {
// 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, SFlag};
use yanix::file::{fstatat, FileType};

if errno == Errno::EPERM {
if let Ok(stat) = unsafe {
Expand All @@ -33,7 +33,7 @@ pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> {
AtFlag::SYMLINK_NOFOLLOW,
)
} {
if SFlag::from_bits_truncate(stat.st_mode).contains(SFlag::IFDIR) {
if FileType::from_stat_st_mode(stat.st_mode) == FileType::Directory {
errno = Errno::EISDIR;
}
} else {
Expand Down
64 changes: 13 additions & 51 deletions crates/wasi-common/src/old/snapshot_0/sys/unix/host_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,7 @@ use crate::old::snapshot_0::{
};
use std::ffi::OsStr;
use std::os::unix::prelude::OsStrExt;
use yanix::{
file::{OFlag, SFlag},
Errno,
};
use yanix::{file::OFlag, Errno};

pub(crate) use sys_impl::host_impl::*;

Expand Down Expand Up @@ -159,24 +156,6 @@ pub(crate) fn nix_from_oflags(oflags: wasi::__wasi_oflags_t) -> OFlag {
nix_flags
}

pub(crate) fn filetype_from_nix(sflags: SFlag) -> FileType {
if sflags.contains(SFlag::IFCHR) {
FileType::CharacterDevice
} else if sflags.contains(SFlag::IFBLK) {
FileType::BlockDevice
} else if sflags.contains(SFlag::IFSOCK) {
FileType::SocketStream
} else if sflags.contains(SFlag::IFDIR) {
FileType::Directory
} else if sflags.contains(SFlag::IFREG) {
FileType::RegularFile
} else if sflags.contains(SFlag::IFLNK) {
FileType::Symlink
} else {
FileType::Unknown
}
}

pub(crate) fn filestat_from_nix(filestat: libc::stat) -> Result<wasi::__wasi_filestat_t> {
use std::convert::TryInto;

Expand All @@ -186,7 +165,7 @@ pub(crate) fn filestat_from_nix(filestat: libc::stat) -> Result<wasi::__wasi_fil
.ok_or(Error::EOVERFLOW)
}

let filetype = SFlag::from_bits_truncate(filestat.st_mode);
let filetype = yanix::file::FileType::from_stat_st_mode(filestat.st_mode);
let dev = stdev_from_nix(filestat.st_dev)?;
let ino = stino_from_nix(filestat.st_ino)?;
let atim = filestat_to_timestamp(
Expand All @@ -210,33 +189,10 @@ pub(crate) fn filestat_from_nix(filestat: libc::stat) -> Result<wasi::__wasi_fil
atim,
ctim,
mtim,
filetype: filetype_from_nix(filetype).to_wasi(),
filetype: FileType::from(filetype).to_wasi(),
})
}

pub(crate) fn dirent_filetype_from_host(
host_entry: &libc::dirent,
) -> Result<wasi::__wasi_filetype_t> {
match host_entry.d_type {
libc::DT_FIFO => Ok(wasi::__WASI_FILETYPE_UNKNOWN),
libc::DT_CHR => Ok(wasi::__WASI_FILETYPE_CHARACTER_DEVICE),
libc::DT_DIR => Ok(wasi::__WASI_FILETYPE_DIRECTORY),
libc::DT_BLK => Ok(wasi::__WASI_FILETYPE_BLOCK_DEVICE),
libc::DT_REG => Ok(wasi::__WASI_FILETYPE_REGULAR_FILE),
libc::DT_LNK => Ok(wasi::__WASI_FILETYPE_SYMBOLIC_LINK),
libc::DT_SOCK => {
// TODO how to discriminate between STREAM and DGRAM?
// Perhaps, we should create a more general WASI filetype
// such as __WASI_FILETYPE_SOCKET, and then it would be
// up to the client to check whether it's actually
// STREAM or DGRAM?
Ok(wasi::__WASI_FILETYPE_UNKNOWN)
}
libc::DT_UNKNOWN => Ok(wasi::__WASI_FILETYPE_UNKNOWN),
_ => Err(Error::EINVAL),
}
}

/// Creates owned WASI path from OS string.
///
/// NB WASI spec requires OS string to be valid UTF-8. Otherwise,
Expand All @@ -245,16 +201,22 @@ pub(crate) fn path_from_host<S: AsRef<OsStr>>(s: S) -> Result<String> {
helpers::path_from_slice(s.as_ref().as_bytes()).map(String::from)
}

impl From<yanix::dir::FileType> for FileType {
fn from(ft: yanix::dir::FileType) -> Self {
use yanix::dir::FileType::*;
impl From<yanix::file::FileType> for FileType {
fn from(ft: yanix::file::FileType) -> Self {
use yanix::file::FileType::*;
match ft {
RegularFile => Self::RegularFile,
Symlink => Self::Symlink,
Directory => Self::Directory,
BlockDevice => Self::BlockDevice,
CharacterDevice => Self::CharacterDevice,
/* Unknown | Socket | Fifo */ _ => Self::Unknown,
/* Unknown | Socket | Fifo */
_ => Self::Unknown,
// TODO how to discriminate between STREAM and DGRAM?
// Perhaps, we should create a more general WASI filetype
// such as __WASI_FILETYPE_SOCKET, and then it would be
// up to the client to check whether it's actually
// STREAM or DGRAM?
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ pub(crate) fn path_open(
fs_flags: wasi::__wasi_fdflags_t,
) -> Result<File> {
use yanix::{
file::{fstatat, openat, AtFlag, Mode, OFlag, SFlag},
file::{fstatat, openat, AtFlag, FileType, Mode, OFlag},
Errno,
};

Expand Down Expand Up @@ -138,7 +138,7 @@ pub(crate) fn path_open(
AtFlag::SYMLINK_NOFOLLOW,
)
} {
if SFlag::from_bits_truncate(stat.st_mode).contains(SFlag::IFSOCK) {
if FileType::from_stat_st_mode(stat.st_mode) == FileType::Socket {
return Err(Error::ENOTSUP);
} else {
return Err(Error::ENXIO);
Expand All @@ -159,7 +159,7 @@ pub(crate) fn path_open(
AtFlag::SYMLINK_NOFOLLOW,
)
} {
if SFlag::from_bits_truncate(stat.st_mode).contains(SFlag::IFLNK) {
if FileType::from_stat_st_mode(stat.st_mode) == FileType::Symlink {
return Err(Error::ELOOP);
}
}
Expand Down
4 changes: 2 additions & 2 deletions crates/wasi-common/src/sys/unix/bsd/hostcalls_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> {
// 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, SFlag};
use yanix::file::{fstatat, FileType};

if errno == Errno::EPERM {
if let Ok(stat) = unsafe {
Expand All @@ -33,7 +33,7 @@ pub(crate) fn path_unlink_file(resolved: PathGet) -> Result<()> {
AtFlag::SYMLINK_NOFOLLOW,
)
} {
if SFlag::from_bits_truncate(stat.st_mode).contains(SFlag::IFDIR) {
if FileType::from_stat_st_mode(stat.st_mode) == FileType::Directory {
errno = Errno::EISDIR;
}
} else {
Expand Down
64 changes: 13 additions & 51 deletions crates/wasi-common/src/sys/unix/host_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@ use crate::host::FileType;
use crate::{error::FromRawOsError, helpers, sys::unix::sys_impl, wasi, Error, Result};
use std::ffi::OsStr;
use std::os::unix::prelude::OsStrExt;
use yanix::{
file::{OFlag, SFlag},
Errno,
};
use yanix::{file::OFlag, Errno};

pub(crate) use sys_impl::host_impl::*;

Expand Down Expand Up @@ -157,24 +154,6 @@ pub(crate) fn nix_from_oflags(oflags: wasi::__wasi_oflags_t) -> OFlag {
nix_flags
}

pub(crate) fn filetype_from_nix(sflags: SFlag) -> FileType {
if sflags.contains(SFlag::IFCHR) {
FileType::CharacterDevice
} else if sflags.contains(SFlag::IFBLK) {
FileType::BlockDevice
} else if sflags.contains(SFlag::IFSOCK) {
FileType::SocketStream
} else if sflags.contains(SFlag::IFDIR) {
FileType::Directory
} else if sflags.contains(SFlag::IFREG) {
FileType::RegularFile
} else if sflags.contains(SFlag::IFLNK) {
FileType::Symlink
} else {
FileType::Unknown
}
}

pub(crate) fn filestat_from_nix(filestat: libc::stat) -> Result<wasi::__wasi_filestat_t> {
use std::convert::TryInto;

Expand All @@ -184,7 +163,7 @@ pub(crate) fn filestat_from_nix(filestat: libc::stat) -> Result<wasi::__wasi_fil
.ok_or(Error::EOVERFLOW)
}

let filetype = SFlag::from_bits_truncate(filestat.st_mode);
let filetype = yanix::file::FileType::from_stat_st_mode(filestat.st_mode);
let dev = stdev_from_nix(filestat.st_dev)?;
let ino = stino_from_nix(filestat.st_ino)?;
let atim = filestat_to_timestamp(
Expand All @@ -208,33 +187,10 @@ pub(crate) fn filestat_from_nix(filestat: libc::stat) -> Result<wasi::__wasi_fil
atim,
ctim,
mtim,
filetype: filetype_from_nix(filetype).to_wasi(),
filetype: FileType::from(filetype).to_wasi(),
})
}

pub(crate) fn dirent_filetype_from_host(
host_entry: &libc::dirent,
) -> Result<wasi::__wasi_filetype_t> {
match host_entry.d_type {
libc::DT_FIFO => Ok(wasi::__WASI_FILETYPE_UNKNOWN),
libc::DT_CHR => Ok(wasi::__WASI_FILETYPE_CHARACTER_DEVICE),
libc::DT_DIR => Ok(wasi::__WASI_FILETYPE_DIRECTORY),
libc::DT_BLK => Ok(wasi::__WASI_FILETYPE_BLOCK_DEVICE),
libc::DT_REG => Ok(wasi::__WASI_FILETYPE_REGULAR_FILE),
libc::DT_LNK => Ok(wasi::__WASI_FILETYPE_SYMBOLIC_LINK),
libc::DT_SOCK => {
// TODO how to discriminate between STREAM and DGRAM?
// Perhaps, we should create a more general WASI filetype
// such as __WASI_FILETYPE_SOCKET, and then it would be
// up to the client to check whether it's actually
// STREAM or DGRAM?
Ok(wasi::__WASI_FILETYPE_UNKNOWN)
}
libc::DT_UNKNOWN => Ok(wasi::__WASI_FILETYPE_UNKNOWN),
_ => Err(Error::EINVAL),
}
}

/// Creates owned WASI path from OS string.
///
/// NB WASI spec requires OS string to be valid UTF-8. Otherwise,
Expand All @@ -243,16 +199,22 @@ pub(crate) fn path_from_host<S: AsRef<OsStr>>(s: S) -> Result<String> {
helpers::path_from_slice(s.as_ref().as_bytes()).map(String::from)
}

impl From<yanix::dir::FileType> for FileType {
fn from(ft: yanix::dir::FileType) -> Self {
use yanix::dir::FileType::*;
impl From<yanix::file::FileType> for FileType {
fn from(ft: yanix::file::FileType) -> Self {
use yanix::file::FileType::*;
match ft {
RegularFile => Self::RegularFile,
Symlink => Self::Symlink,
Directory => Self::Directory,
BlockDevice => Self::BlockDevice,
CharacterDevice => Self::CharacterDevice,
/* Unknown | Socket | Fifo */ _ => Self::Unknown,
/* Unknown | Socket | Fifo */
_ => Self::Unknown,
// TODO how to discriminate between STREAM and DGRAM?
// Perhaps, we should create a more general WASI filetype
// such as __WASI_FILETYPE_SOCKET, and then it would be
// up to the client to check whether it's actually
// STREAM or DGRAM?
}
}
}
6 changes: 3 additions & 3 deletions crates/wasi-common/src/sys/unix/hostcalls_impl/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ pub(crate) fn path_open(
fs_flags: wasi::__wasi_fdflags_t,
) -> Result<File> {
use yanix::{
file::{fstatat, openat, AtFlag, Mode, OFlag, SFlag},
file::{fstatat, openat, AtFlag, FileType, Mode, OFlag},
Errno,
};

Expand Down Expand Up @@ -143,7 +143,7 @@ pub(crate) fn path_open(
AtFlag::SYMLINK_NOFOLLOW,
)
} {
if SFlag::from_bits_truncate(stat.st_mode).contains(SFlag::IFSOCK) {
if FileType::from_stat_st_mode(stat.st_mode) == FileType::Socket {
return Err(Error::ENOTSUP);
} else {
return Err(Error::ENXIO);
Expand All @@ -164,7 +164,7 @@ pub(crate) fn path_open(
AtFlag::SYMLINK_NOFOLLOW,
)
} {
if SFlag::from_bits_truncate(stat.st_mode).contains(SFlag::IFLNK) {
if FileType::from_stat_st_mode(stat.st_mode) == FileType::Symlink {
return Err(Error::ELOOP);
}
}
Expand Down
44 changes: 2 additions & 42 deletions crates/wasi-common/yanix/src/dir.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::{
file::FileType,
sys::dir::{iter_impl, EntryImpl},
Errno, Result,
};
Expand Down Expand Up @@ -84,7 +85,7 @@ impl Entry {

/// Returns the type of this directory entry.
pub fn file_type(&self) -> FileType {
FileType::from_raw(self.0.d_type)
FileType::from_dirent_d_type(self.0.d_type)
}
}

Expand All @@ -99,47 +100,6 @@ impl SeekLoc {
}
}

#[derive(Clone, Copy, Debug)]
#[repr(u8)]
pub enum FileType {
CharacterDevice = libc::DT_CHR,
Directory = libc::DT_DIR,
BlockDevice = libc::DT_BLK,
RegularFile = libc::DT_REG,
Symlink = libc::DT_LNK,
Fifo = libc::DT_FIFO,
Socket = libc::DT_SOCK,
Unknown = libc::DT_UNKNOWN,
}

impl FileType {
pub fn from_raw(file_type: u8) -> Self {
match file_type {
libc::DT_CHR => Self::CharacterDevice,
libc::DT_DIR => Self::Directory,
libc::DT_BLK => Self::BlockDevice,
libc::DT_REG => Self::RegularFile,
libc::DT_LNK => Self::Symlink,
libc::DT_SOCK => Self::Socket,
libc::DT_FIFO => Self::Fifo,
/* libc::DT_UNKNOWN */ _ => Self::Unknown,
}
}

pub fn to_raw(&self) -> u8 {
match self {
Self::CharacterDevice => libc::DT_CHR,
Self::Directory => libc::DT_DIR,
Self::BlockDevice => libc::DT_BLK,
Self::RegularFile => libc::DT_REG,
Self::Symlink => libc::DT_LNK,
Self::Socket => libc::DT_SOCK,
Self::Fifo => libc::DT_FIFO,
Self::Unknown => libc::DT_UNKNOWN,
}
}
}

#[derive(Debug)]
pub struct DirIter<T: Deref<Target = Dir>>(T);

Expand Down
Loading

0 comments on commit 4fe397e

Please sign in to comment.