integrate GetSetFdFlags!

change reopen_with_fdflags(&self, fdflags) -> Result<Box<dyn WasiFile>>
to set_fdflags(&mut self, fdflags) -> Result<()>.

this makes way more sense than my prior hare-brained schemes.
This commit is contained in:
Pat Hickey
2021-01-26 12:23:52 -08:00
parent c98d6f6201
commit 1eb8a8a7fe
6 changed files with 83 additions and 63 deletions

View File

@@ -1,8 +1,4 @@
TODOs:
* File::reopen_with_fdflags is unimplemented, File::get_fdflags is lying - these are
fcntl on unix, reopenfile / require reopening on windows.
# Linux # Linux
@@ -12,12 +8,8 @@ TODOs:
* remove_directory_trailing_slashes * remove_directory_trailing_slashes
- cap-std Dir::remove_dir gives EINVAL when trying to remove dir with - cap-std Dir::remove_dir gives EINVAL when trying to remove dir with
trailing slash. otherwise, everything passes. trailing slash. otherwise, everything passes.
* fd_flags_set
- set_fdflags is not implemented. test wanted to clear O_APPEND mode
* path_filestat * path_filestat
- fdstat.fs_flags is not populated correctly - APPEND | SYNC aren't - symlink mtim doesnt match expectations
present because File::get_fdflags isnt implemented correctly
# Windows # Windows
@@ -27,6 +19,7 @@ TODOs:
* file_allocate * file_allocate
- call to fd_allocate(10,10) reduces size from 100 to 20 - call to fd_allocate(10,10) reduces size from 100 to 20
- fix upstream: fd_allocate is impossible on windows, so dont even try - fix upstream: fd_allocate is impossible on windows, so dont even try
- the test should be modified to accept this fd_allocate always failing on windows.
* nofollow_errors * nofollow_errors
- I loosened up some errno acceptance but windows requires rmdir to delete - I loosened up some errno acceptance but windows requires rmdir to delete
a symlink to a directory, rather than unlink_file a symlink to a directory, rather than unlink_file
@@ -34,14 +27,15 @@ TODOs:
- narrowed down the symlink delete issue that only shows up on linux - narrowed down the symlink delete issue that only shows up on linux
* path_rename * path_rename
- permission denied on windows to rename a dir to an existing empty dir - permission denied on windows to rename a dir to an existing empty dir
- This should fail - we're not going to try to emulate this functionality.
- We should disable this test on windows - the test should be modified accept this operation failing completely on windows.
- is there a windows api that makes this possible?
- if not, and we drew the line at "no racy emulation" in fallocate, then
is returning the error windows gives us OK here?
* path_link * path_link
- fails on the first dangling symlink - fails on the first dangling symlink
* fd_flags_set
- same metadata panic as fd_readdir
* path_filestat
- same metadata panic as fd_readdir
## "Trailing slashes are a bonified boondoggle" - Dan ## "Trailing slashes are a bonified boondoggle" - Dan
@@ -51,9 +45,3 @@ TODOs:
* path_symlink_trailing_slashes * path_symlink_trailing_slashes
* remove_directory_trailing_slashes * remove_directory_trailing_slashes
* unlink_file_trailing_slashes * unlink_file_trailing_slashes
## Same missing functionality as over in Linux:
* fd_flags_set
* path_filestat

View File

@@ -3,8 +3,10 @@ use fs_set_times::{SetTimes, SystemTimeSpec};
use std::any::Any; use std::any::Any;
use std::convert::TryInto; use std::convert::TryInto;
use std::io; use std::io;
use system_interface::fs::{Advice, FileIoExt}; use system_interface::{
use system_interface::io::ReadReady; fs::{Advice, FileIoExt, GetSetFdFlags},
io::ReadReady,
};
use wasi_c2::{ use wasi_c2::{
file::{FdFlags, FileType, Filestat, WasiFile}, file::{FdFlags, FileType, Filestat, WasiFile},
Error, Error,
@@ -35,11 +37,11 @@ impl WasiFile for File {
Ok(filetype_from(&meta.file_type())) Ok(filetype_from(&meta.file_type()))
} }
fn get_fdflags(&self) -> Result<FdFlags, Error> { fn get_fdflags(&self) -> Result<FdFlags, Error> {
// XXX get_fdflags is not implemented but lets lie rather than panic: let fdflags = self.0.get_fd_flags()?;
Ok(FdFlags::empty()) Ok(from_sysif_fdflags(fdflags))
} }
unsafe fn reopen_with_fdflags(&self, _fdflags: FdFlags) -> Result<Box<dyn WasiFile>, Error> { fn set_fdflags(&mut self, fdflags: FdFlags) -> Result<(), Error> {
todo!("reopen_with_fdflags is not implemented") Ok(self.0.set_fd_flags(to_sysif_fdflags(fdflags))?)
} }
fn get_filestat(&self) -> Result<Filestat, Error> { fn get_filestat(&self) -> Result<Filestat, Error> {
let meta = self.0.metadata()?; let meta = self.0.metadata()?;
@@ -150,3 +152,42 @@ pub fn convert_systimespec(t: Option<wasi_c2::SystemTimeSpec>) -> Option<SystemT
None => None, None => None,
} }
} }
pub fn to_sysif_fdflags(f: wasi_c2::file::FdFlags) -> system_interface::fs::FdFlags {
let mut out = system_interface::fs::FdFlags::empty();
if f.contains(wasi_c2::file::FdFlags::APPEND) {
out |= system_interface::fs::FdFlags::APPEND;
}
if f.contains(wasi_c2::file::FdFlags::DSYNC) {
out |= system_interface::fs::FdFlags::DSYNC;
}
if f.contains(wasi_c2::file::FdFlags::NONBLOCK) {
out |= system_interface::fs::FdFlags::NONBLOCK;
}
if f.contains(wasi_c2::file::FdFlags::RSYNC) {
out |= system_interface::fs::FdFlags::RSYNC;
}
if f.contains(wasi_c2::file::FdFlags::SYNC) {
out |= system_interface::fs::FdFlags::SYNC;
}
out
}
pub fn from_sysif_fdflags(f: system_interface::fs::FdFlags) -> wasi_c2::file::FdFlags {
let mut out = wasi_c2::file::FdFlags::empty();
if f.contains(system_interface::fs::FdFlags::APPEND) {
out |= wasi_c2::file::FdFlags::APPEND;
}
if f.contains(system_interface::fs::FdFlags::DSYNC) {
out |= wasi_c2::file::FdFlags::DSYNC;
}
if f.contains(system_interface::fs::FdFlags::NONBLOCK) {
out |= wasi_c2::file::FdFlags::NONBLOCK;
}
if f.contains(system_interface::fs::FdFlags::RSYNC) {
out |= wasi_c2::file::FdFlags::RSYNC;
}
if f.contains(system_interface::fs::FdFlags::SYNC) {
out |= wasi_c2::file::FdFlags::SYNC;
}
out
}

View File

@@ -36,10 +36,9 @@ impl WasiFile for Stdin {
Ok(FileType::Unknown) Ok(FileType::Unknown)
} }
fn get_fdflags(&self) -> Result<FdFlags, Error> { fn get_fdflags(&self) -> Result<FdFlags, Error> {
// XXX get_fdflags is not implemented but lets lie rather than panic:
Ok(FdFlags::empty()) Ok(FdFlags::empty())
} }
unsafe fn reopen_with_fdflags(&self, _fdflags: FdFlags) -> Result<Box<dyn WasiFile>, Error> { fn set_fdflags(&mut self, _fdflags: FdFlags) -> Result<(), Error> {
Err(Error::badf()) Err(Error::badf())
} }
fn get_filestat(&self) -> Result<Filestat, Error> { fn get_filestat(&self) -> Result<Filestat, Error> {
@@ -125,13 +124,9 @@ macro_rules! wasi_file_write_impl {
Ok(FileType::Unknown) Ok(FileType::Unknown)
} }
fn get_fdflags(&self) -> Result<FdFlags, Error> { fn get_fdflags(&self) -> Result<FdFlags, Error> {
// XXX get_fdflags is not implemented but lets lie rather than panic: Ok(FdFlags::APPEND)
Ok(FdFlags::empty())
} }
unsafe fn reopen_with_fdflags( fn set_fdflags(&mut self, _fdflags: FdFlags) -> Result<(), Error> {
&self,
_fdflags: FdFlags,
) -> Result<Box<dyn WasiFile>, Error> {
Err(Error::badf()) Err(Error::badf())
} }
fn get_filestat(&self) -> Result<Filestat, Error> { fn get_filestat(&self) -> Result<Filestat, Error> {

View File

@@ -1,8 +1,8 @@
use crate::{Error, ErrorExt, SystemTimeSpec}; use crate::{Error, ErrorExt, SystemTimeSpec};
use bitflags::bitflags; use bitflags::bitflags;
use std::any::Any; use std::any::Any;
use std::cell::Ref; use std::cell::{Ref, RefMut};
use std::ops::Deref; use std::ops::{Deref, DerefMut};
pub trait WasiFile { pub trait WasiFile {
fn as_any(&self) -> &dyn Any; fn as_any(&self) -> &dyn Any;
@@ -10,9 +10,7 @@ pub trait WasiFile {
fn sync(&self) -> Result<(), Error>; // file op fn sync(&self) -> Result<(), Error>; // file op
fn get_filetype(&self) -> Result<FileType, Error>; // file op fn get_filetype(&self) -> Result<FileType, Error>; // file op
fn get_fdflags(&self) -> Result<FdFlags, Error>; // file op fn get_fdflags(&self) -> Result<FdFlags, Error>; // file op
/// This method takes a `&self` so that it can be called on a `&dyn WasiFile`. However, fn set_fdflags(&mut self, flags: FdFlags) -> Result<(), Error>; // file op
/// the caller makes the additional guarantee to drop `self` after the call is successful.
unsafe fn reopen_with_fdflags(&self, flags: FdFlags) -> Result<Box<dyn WasiFile>, Error>; // file op
fn get_filestat(&self) -> Result<Filestat, Error>; // split out get_length as a read & write op, rest is a file op fn get_filestat(&self) -> Result<Filestat, Error>; // split out get_length as a read & write op, rest is a file op
fn set_filestat_size(&self, _size: u64) -> Result<(), Error>; // write op fn set_filestat_size(&self, _size: u64) -> Result<(), Error>; // write op
fn advise( fn advise(
@@ -83,22 +81,14 @@ pub struct Filestat {
pub(crate) trait TableFileExt { pub(crate) trait TableFileExt {
fn get_file(&self, fd: u32) -> Result<Ref<FileEntry>, Error>; fn get_file(&self, fd: u32) -> Result<Ref<FileEntry>, Error>;
fn update_file_in_place<F>(&mut self, fd: u32, f: F) -> Result<(), Error> fn get_file_mut(&self, fd: u32) -> Result<RefMut<FileEntry>, Error>;
where
F: FnOnce(&dyn WasiFile) -> Result<Box<dyn WasiFile>, Error>;
} }
impl TableFileExt for crate::table::Table { impl TableFileExt for crate::table::Table {
fn get_file(&self, fd: u32) -> Result<Ref<FileEntry>, Error> { fn get_file(&self, fd: u32) -> Result<Ref<FileEntry>, Error> {
self.get(fd) self.get(fd)
} }
fn update_file_in_place<F>(&mut self, fd: u32, f: F) -> Result<(), Error> fn get_file_mut(&self, fd: u32) -> Result<RefMut<FileEntry>, Error> {
where self.get_mut(fd)
F: FnOnce(&dyn WasiFile) -> Result<Box<dyn WasiFile>, Error>,
{
self.update_in_place(fd, |FileEntry { caps, file }| {
let file = f(file.deref())?;
Ok(FileEntry { caps, file })
})
} }
} }
@@ -145,6 +135,16 @@ impl<'a> FileEntryExt<'a> for Ref<'a, FileEntry> {
Ok(Ref::map(self, |r| r.file.deref())) Ok(Ref::map(self, |r| r.file.deref()))
} }
} }
pub trait FileEntryMutExt<'a> {
fn get_cap(self, caps: FileCaps) -> Result<RefMut<'a, dyn WasiFile>, Error>;
}
impl<'a> FileEntryMutExt<'a> for RefMut<'a, FileEntry> {
fn get_cap(self, caps: FileCaps) -> Result<RefMut<'a, dyn WasiFile>, Error> {
self.capable_of(caps)?;
Ok(RefMut::map(self, |r| r.file.deref_mut()))
}
}
bitflags! { bitflags! {
pub struct FileCaps : u32 { pub struct FileCaps : u32 {

View File

@@ -114,7 +114,7 @@ impl<R: Read + Any> WasiFile for ReadPipe<R> {
fn get_fdflags(&self) -> Result<FdFlags, Error> { fn get_fdflags(&self) -> Result<FdFlags, Error> {
Ok(FdFlags::empty()) Ok(FdFlags::empty())
} }
unsafe fn reopen_with_fdflags(&self, _fdflags: FdFlags) -> Result<Box<dyn WasiFile>, Error> { fn set_fdflags(&mut self, _fdflags: FdFlags) -> Result<(), Error> {
Err(Error::badf()) Err(Error::badf())
} }
fn get_filestat(&self) -> Result<Filestat, Error> { fn get_filestat(&self) -> Result<Filestat, Error> {
@@ -250,7 +250,7 @@ impl<W: Write + Any> WasiFile for WritePipe<W> {
fn get_fdflags(&self) -> Result<FdFlags, Error> { fn get_fdflags(&self) -> Result<FdFlags, Error> {
Ok(FdFlags::APPEND) Ok(FdFlags::APPEND)
} }
unsafe fn reopen_with_fdflags(&self, _fdflags: FdFlags) -> Result<Box<dyn WasiFile>, Error> { fn set_fdflags(&mut self, _fdflags: FdFlags) -> Result<(), Error> {
Err(Error::badf()) Err(Error::badf())
} }
fn get_filestat(&self) -> Result<Filestat, Error> { fn get_filestat(&self) -> Result<Filestat, Error> {

View File

@@ -2,8 +2,8 @@
use crate::{ use crate::{
dir::{DirCaps, DirEntry, DirEntryExt, DirFdStat, ReaddirCursor, ReaddirEntity, TableDirExt}, dir::{DirCaps, DirEntry, DirEntryExt, DirFdStat, ReaddirCursor, ReaddirEntity, TableDirExt},
file::{ file::{
FdFlags, FdStat, FileCaps, FileEntry, FileEntryExt, FileType, Filestat, OFlags, FdFlags, FdStat, FileCaps, FileEntry, FileEntryExt, FileEntryMutExt, FileType, Filestat,
TableFileExt, OFlags, TableFileExt,
}, },
sched::{ sched::{
subscription::{RwEventFlags, SubscriptionResult}, subscription::{RwEventFlags, SubscriptionResult},
@@ -332,14 +332,10 @@ impl<'a> wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
} }
fn fd_fdstat_set_flags(&self, fd: types::Fd, flags: types::Fdflags) -> Result<(), Error> { fn fd_fdstat_set_flags(&self, fd: types::Fd, flags: types::Fdflags) -> Result<(), Error> {
let mut table = self.table(); self.table()
let fd = u32::from(fd); .get_file_mut(u32::from(fd))?
let table_check = table.get_file(fd)?.get_cap(FileCaps::FDSTAT_SET_FLAGS)?; .get_cap(FileCaps::FDSTAT_SET_FLAGS)?
drop(table_check); .set_fdflags(FdFlags::from(&flags))
table.update_file_in_place(fd, |f| unsafe {
// Safety: update_file_in_place will drop `f` after this call.
f.reopen_with_fdflags(FdFlags::from(&flags))
})
} }
fn fd_fdstat_set_rights( fn fd_fdstat_set_rights(