Skip to content

Commit

Permalink
Revert "Fix handling of the root dir in path_create_directory and `…
Browse files Browse the repository at this point in the history
…get_inode_at_path_inner`"
  • Loading branch information
Arshia001 authored and theduke committed Nov 8, 2024
1 parent ccfbff4 commit 196af22
Show file tree
Hide file tree
Showing 11 changed files with 29 additions and 108 deletions.
9 changes: 1 addition & 8 deletions lib/virtual-io/src/selector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,14 +138,7 @@ impl Selector {
let mut events = mio::Events::with_capacity(128);
loop {
// Wait for an event to trigger
if let Err(e) = poll.poll(&mut events, None) {
// This can happen when a debugger is attached
#[cfg(debug_assertions)]
if e.kind() == std::io::ErrorKind::Interrupted {
continue;
}
panic!("Unexpected error in selector poll loop: {e:?}");
}
poll.poll(&mut events, None).unwrap();

// Loop through all the events while under a guard lock
let mut guard = engine.inner.lock().unwrap();
Expand Down
2 changes: 1 addition & 1 deletion lib/wasix/src/fs/fd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ impl Fd {
pub struct InodeVal {
pub stat: RwLock<Filestat>,
pub is_preopened: bool,
pub name: RwLock<Cow<'static, str>>,
pub name: Cow<'static, str>,
pub kind: RwLock<Kind>,
}

Expand Down
30 changes: 11 additions & 19 deletions lib/wasix/src/fs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,7 @@ impl WasiFs {
let root_inode = inodes.add_inode_val(InodeVal {
stat: RwLock::new(stat),
is_preopened: true,
name: RwLock::new("/".into()),
name: "/".into(),
kind: RwLock::new(root_kind),
});

Expand Down Expand Up @@ -984,13 +984,6 @@ impl WasiFs {

// TODO: rights checks
'path_iter: for (i, component) in path.components().enumerate() {
// Since we're resolving the path against the given inode, we want to
// assume '/a/b' to be the same as `a/b` relative to the inode, so
// we skip over the RootDir component.
if matches!(component, Component::RootDir) {
continue;
}

// used to terminate symlink resolution properly
let last_component = i + 1 == n_components;
// for each component traverse file structure
Expand Down Expand Up @@ -1344,14 +1337,13 @@ impl WasiFs {
follow_symlinks: bool,
) -> Result<InodeGuard, Errno> {
let base_inode = self.get_fd_inode(base)?;
let start_inode = if !base_inode.deref().name.read().unwrap().starts_with('/')
&& self.is_wasix.load(Ordering::Acquire)
{
let (cur_inode, _) = self.get_current_dir(inodes, base)?;
cur_inode
} else {
self.get_fd_inode(base)?
};
let start_inode =
if !base_inode.deref().name.starts_with('/') && self.is_wasix.load(Ordering::Acquire) {
let (cur_inode, _) = self.get_current_dir(inodes, base)?;
cur_inode
} else {
self.get_fd_inode(base)?
};
self.get_inode_at_path_inner(inodes, start_inode, path, 0, follow_symlinks)
}

Expand Down Expand Up @@ -1506,7 +1498,7 @@ impl WasiFs {
// REVIEW:
// no need for +1, because there is no 0 end-of-string marker
// john: removing the +1 seems cause regression issues
pr_name_len: inode_val.name.read().unwrap().len() as u32 + 1,
pr_name_len: inode_val.name.len() as u32 + 1,
}
.untagged(),
}
Expand Down Expand Up @@ -1617,7 +1609,7 @@ impl WasiFs {
inodes.add_inode_val(InodeVal {
stat: RwLock::new(stat),
is_preopened,
name: RwLock::new(name),
name,
kind: RwLock::new(kind),
})
}
Expand Down Expand Up @@ -2003,7 +1995,7 @@ impl WasiFs {
inodes.add_inode_val(InodeVal {
stat: RwLock::new(stat),
is_preopened: true,
name: RwLock::new(name.to_string().into()),
name: name.to_string().into(),
kind: RwLock::new(kind),
})
};
Expand Down
11 changes: 5 additions & 6 deletions lib/wasix/src/syscalls/wasi/fd_prestat_dir_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ pub fn fd_prestat_dir_name<M: MemorySize>(
let path_chars = wasi_try_mem!(path.slice(&memory, path_len));

let inode = wasi_try!(state.fs.get_fd_inode(fd));
let name = inode.name.read().unwrap();
Span::current().record("path", name.as_ref());
Span::current().record("path", inode.name.as_ref());

// check inode-val.is_preopened?

Expand All @@ -23,11 +22,11 @@ pub fn fd_prestat_dir_name<M: MemorySize>(
Kind::Dir { .. } | Kind::Root { .. } => {
// TODO: verify this: null termination, etc
let path_len: u64 = path_len.into();
if (name.len() as u64) < path_len {
if (inode.name.len() as u64) < path_len {
wasi_try_mem!(path_chars
.subslice(0..name.len() as u64)
.write_slice(name.as_bytes()));
wasi_try_mem!(path_chars.index(name.len() as u64).write(0));
.subslice(0..inode.name.len() as u64)
.write_slice(inode.name.as_bytes()));
wasi_try_mem!(path_chars.index(inode.name.len() as u64).write(0));

//trace!("=> result: \"{}\"", inode_val.name);

Expand Down
12 changes: 2 additions & 10 deletions lib/wasix/src/syscalls/wasi/fd_readdir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,7 @@ pub fn fd_readdir<M: MemorySize>(
entry_vec.extend(entries.iter().filter(|(_, inode)| inode.is_preopened).map(
|(name, inode)| {
let stat = inode.stat.read().unwrap();
(
inode.name.read().unwrap().to_string(),
stat.st_filetype,
stat.st_ino,
)
(inode.name.to_string(), stat.st_filetype, stat.st_ino)
},
));
// adding . and .. special folders
Expand All @@ -92,11 +88,7 @@ pub fn fd_readdir<M: MemorySize>(
.into_iter()
.map(|(name, inode)| {
let stat = inode.stat.read().unwrap();
(
format!("/{}", inode.name.read().unwrap().as_ref()),
stat.st_filetype,
stat.st_ino,
)
(format!("/{}", inode.name), stat.st_filetype, stat.st_ino)
})
.collect()
}
Expand Down
25 changes: 7 additions & 18 deletions lib/wasix/src/syscalls/wasi/path_create_directory.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
use std::{
path::{Component, PathBuf},
str::FromStr,
};
use std::{path::PathBuf, str::FromStr};

use super::*;
use crate::syscalls::*;
Expand Down Expand Up @@ -33,7 +30,7 @@ pub fn path_create_directory<M: MemorySize>(
Span::current().record("path", path_string.as_str());

// Convert relative paths into absolute paths
if !path_string.starts_with("/") {
if path_string.starts_with("./") {
path_string = ctx.data().state.fs.relative_path_to_absolute(path_string);
trace!(
%path_string
Expand Down Expand Up @@ -71,14 +68,11 @@ pub(crate) fn path_create_directory_internal(
let path = std::path::PathBuf::from(path);
let path_vec = path
.components()
.filter_map(|comp| match comp {
Component::RootDir => None,
_ => Some(
comp.as_os_str()
.to_str()
.map(|inner_str| inner_str.to_string())
.ok_or(Errno::Inval),
),
.map(|comp| {
comp.as_os_str()
.to_str()
.map(|inner_str| inner_str.to_string())
.ok_or(Errno::Inval)
})
.collect::<Result<Vec<String>, Errno>>()?;
if path_vec.is_empty() {
Expand All @@ -92,11 +86,6 @@ pub(crate) fn path_create_directory_internal(
let processing_cur_dir_inode = cur_dir_inode.clone();
let mut guard = processing_cur_dir_inode.write();
match guard.deref_mut() {
// TODO: this depends on entries already being filled in. This is the case when you go
// through wasix-libc (which is, realistically, what everybody does) because it stats
// the path before calling path_create_directory, at which point entries is filled in.
// However, an alternate implementation or a manually implemented module may not always
// do this.
Kind::Dir {
ref mut entries,
path,
Expand Down
5 changes: 2 additions & 3 deletions lib/wasix/src/syscalls/wasi/path_rename.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ pub fn path_rename_internal(
}
}

// this is to be sure the source file is fetched from the filesystem if needed
// this is to be sure the source file is fetch from filesystem if needed
wasi_try_ok!(state
.fs
.get_inode_at_path(inodes, source_fd, source_path, true));
Expand Down Expand Up @@ -206,7 +206,7 @@ pub fn path_rename_internal(
if need_create {
let mut guard = target_parent_inode.write();
if let Kind::Dir { entries, .. } = guard.deref_mut() {
let result = entries.insert(target_entry_name.clone(), source_entry);
let result = entries.insert(target_entry_name, source_entry);
assert!(
result.is_none(),
"fatal error: race condition on filesystem detected or internal logic error"
Expand All @@ -219,7 +219,6 @@ pub fn path_rename_internal(
wasi_try_ok!(state
.fs
.get_inode_at_path(inodes, target_fd, target_path, true));
*target_inode.name.write().unwrap() = target_entry_name.into();
target_inode.stat.write().unwrap().st_size = source_size;

Ok(Errno::Success)
Expand Down
9 changes: 0 additions & 9 deletions tests/integration/cli/tests/snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1359,12 +1359,3 @@ fn test_snapshot_worker_panicking() {
));
assert_json_snapshot!(snapshot);
}

#[cfg_attr(any(target_env = "musl", target_os = "windows"), ignore)]
#[test]
fn test_snapshot_mkdir_rename() {
let snapshot = TestBuilder::new()
.with_name(function!())
.run_wasm(include_bytes!("./wasm/mkdir-rename.wasm"));
assert_json_snapshot!(snapshot);
}

This file was deleted.

Binary file removed tests/integration/cli/tests/wasm/mkdir-rename.wasm
Binary file not shown.
12 changes: 0 additions & 12 deletions tests/rust-wasi-tests/src/bin/mkdir-rename.rs

This file was deleted.

0 comments on commit 196af22

Please sign in to comment.