Skip to content

Commit 259bdb4

Browse files
Sl1mb0Sl1mb0
committed
refactor: return Result<Option<SharedVfsNode>> from get_VfsCtxView::get_node_from_start
1 parent ce5918c commit 259bdb4

1 file changed

Lines changed: 74 additions & 48 deletions

File tree

‎host/src/vfs/mod.rs‎

Lines changed: 74 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -428,21 +428,36 @@ impl<'a> VfsCtxView<'a> {
428428
}
429429

430430
/// Get node at given path.
431-
fn node_at(&self, res: Resource<Descriptor>, path: &str) -> FsResult<SharedVfsNode> {
431+
fn node_at(&self, res: Resource<Descriptor>, path: &str) -> FsResult<Option<SharedVfsNode>> {
432432
let node = self.node(res)?;
433433
self.get_node_from_start(path, node)
434434
}
435435

436436
/// Get node at given path from given starting node.
437-
fn get_node_from_start(&self, path: &str, node: SharedVfsNode) -> FsResult<SharedVfsNode> {
437+
fn get_node_from_start(
438+
&self,
439+
path: &str,
440+
node: SharedVfsNode,
441+
) -> FsResult<Option<SharedVfsNode>> {
442+
if path.is_empty() {
443+
return Err(FsError::trap(ErrorCode::Invalid));
444+
}
445+
438446
let (is_root, directions) = PathTraversal::parse(path, &self.vfs_state.limits)?;
439447

440448
let start = if is_root {
441449
Arc::clone(&self.vfs_state.root)
442450
} else {
443451
node
444452
};
445-
VfsNode::traverse(start, directions)
453+
454+
match VfsNode::traverse(start, directions) {
455+
Ok(node) => Ok(Some(node)),
456+
Err(e) => match e.downcast_ref() {
457+
Some(ErrorCode::NoEntry) => Ok(None),
458+
_ => Err(e),
459+
},
460+
}
446461
}
447462

448463
/// Get the parent node and base name for a given path.
@@ -698,7 +713,7 @@ impl<'a> filesystem::types::HostDescriptor for VfsCtxView<'a> {
698713
_path_flags: PathFlags,
699714
path: String,
700715
) -> FsResult<DescriptorStat> {
701-
Ok(self.node_at(self_, &path)?.read().unwrap().stat())
716+
Ok(self.node_at(self_, &path)?.unwrap().read().unwrap().stat())
702717
}
703718

704719
async fn set_times_at(
@@ -731,10 +746,6 @@ impl<'a> filesystem::types::HostDescriptor for VfsCtxView<'a> {
731746
open_flags: OpenFlags,
732747
flags: DescriptorFlags,
733748
) -> FsResult<Resource<Descriptor>> {
734-
if path.is_empty() {
735-
return Err(FsError::trap(ErrorCode::Invalid));
736-
}
737-
738749
let base_desc = self.get_descriptor(self_)?;
739750
let base_node = Arc::clone(&base_desc.node);
740751
let base_flags = base_desc.flags;
@@ -744,30 +755,36 @@ impl<'a> filesystem::types::HostDescriptor for VfsCtxView<'a> {
744755
let exclusive = open_flags.contains(OpenFlags::EXCLUSIVE);
745756
let truncate = open_flags.contains(OpenFlags::TRUNCATE);
746757

747-
// Per POSIX: O_CREAT only creates regular files, not directories.
748-
// https://github.com/WebAssembly/WASI/blob/184b0c0e9fd437e5e5601d6e327a28feddbbd7f7/proposals/filesystem/wit/types.wit#L145-L146
749-
// "If O_CREAT and O_DIRECTORY are set and the requested access mode is neither
750-
// O_WRONLY nor O_RDWR, the result is unspecified."
751-
// We choose to disallow this combination entirely.
752-
if create && directory {
753-
return Err(FsError::trap(ErrorCode::Invalid));
754-
}
755-
756-
if directory && flags.contains(DescriptorFlags::WRITE) {
757-
// Per POSIX: "O_DIRECTORY: "If path resolves to a non-directory file, fail and set errno to [ENOTDIR]."
758-
return Err(FsError::trap(ErrorCode::IsDirectory));
759-
}
760-
761758
// Try to resolve the path to an existing node
762759
let existing = self.get_node_from_start(&path, Arc::clone(&base_node));
763760

764761
let node = match (existing, create, directory, exclusive, truncate) {
765-
(Ok(node), true, _, false, false) => node, // Per POSIX: "If the file exists, O_CREAT has no effect except as noted under O_EXCL below.
766-
(Ok(_), true, _, true, _) => {
762+
(_, true, true, _, _) => {
763+
// Per POSIX: O_CREAT only creates regular files, not directories.
764+
// https://github.com/WebAssembly/WASI/blob/184b0c0e9fd437e5e5601d6e327a28feddbbd7f7/proposals/filesystem/wit/types.wit#L145-L146
765+
// "If O_CREAT and O_DIRECTORY are set and the requested access mode is
766+
// neither O_WRONLY nor O_RDWR, the result is unspecified." We choose to
767+
// disallow this combination entirely.
768+
return Err(FsError::trap(ErrorCode::Invalid));
769+
}
770+
(Ok(Some(node)), _, true, _, _) if flags.contains(DescriptorFlags::WRITE) => {
771+
if matches!(node.read().unwrap().kind, VfsNodeKind::Directory { .. }) {
772+
// Disallow opening directories with write permissions.
773+
// POSIX isn't clear here, so we choose to disallow this
774+
// combination entirely.
775+
return Err(FsError::trap(ErrorCode::IsDirectory));
776+
} else {
777+
// Per POSIX: "O_DIRECTORY: "If path resolves to a
778+
// non-directory file, fail and set errno to [ENOTDIR]."
779+
return Err(FsError::trap(ErrorCode::NotDirectory));
780+
}
781+
}
782+
(Ok(Some(node)), true, _, false, false) => node, // Per POSIX: "If the file exists, O_CREAT has no effect except as noted under O_EXCL below.
783+
(Ok(Some(_)), true, _, true, _) => {
767784
// Per POSIX: "O_CREAT and O_EXCL are set, open() shall fail if the file exists"
768785
return Err(FsError::trap(ErrorCode::Exist));
769786
}
770-
(Ok(node), false, true, false, false) => {
787+
(Ok(Some(node)), false, true, false, false) => {
771788
// Per POSIX: "O_DIRECTORY: "If path resolves to a non-directory file, fail and set errno to [ENOTDIR]."
772789
let guard = node.read().unwrap();
773790
if !matches!(guard.kind, VfsNodeKind::Directory { .. }) {
@@ -776,15 +793,19 @@ impl<'a> filesystem::types::HostDescriptor for VfsCtxView<'a> {
776793
drop(guard);
777794
node
778795
}
779-
(Ok(node), _, _, _, true) => {
796+
(Ok(Some(node)), _, _, _, true) => {
780797
let mut guard = node.write().unwrap();
781798
match &mut guard.kind {
782799
VfsNodeKind::File { content } => {
783800
// Per POSIX: "The result of using O_TRUNC without either O_RDWR
784801
// or O_WRONLY is undefined." We allow it but it's a no-op
785802
// unless write permission is granted.
786803
if flags.contains(DescriptorFlags::WRITE) {
787-
content.clear();
804+
self.vfs_state
805+
.limiter
806+
.shrink(content.capacity())
807+
.map_err(|_| FsError::trap(ErrorCode::InsufficientMemory))?;
808+
*content = Vec::new();
788809
}
789810
}
790811
VfsNodeKind::Directory { .. } => {
@@ -798,13 +819,13 @@ impl<'a> filesystem::types::HostDescriptor for VfsCtxView<'a> {
798819
drop(guard);
799820
node
800821
}
801-
(Ok(node), _, _, _, _) => node,
802-
(Err(_), false, _, _, _) => {
822+
(Ok(Some(node)), _, _, _, _) => node,
823+
(Ok(None), false, _, _, _) => {
803824
// "If O_CREAT is not set and the file does not exist, open()
804825
// shall fail and set errno to [ENOENT]."
805826
return Err(FsError::trap(ErrorCode::NoEntry));
806827
}
807-
(Err(_), true, _, _, _) => {
828+
(Ok(None), true, _, _, _) => {
808829
// "If O_CREAT is set and the file does not exist, it shall be
809830
// created as a regular file with permissions"
810831
if !base_flags.contains(DescriptorFlags::MUTATE_DIRECTORY) {
@@ -821,19 +842,6 @@ impl<'a> filesystem::types::HostDescriptor for VfsCtxView<'a> {
821842
parent: Some(Arc::downgrade(&parent_node)),
822843
}));
823844

824-
// Account for resource usage
825-
self.vfs_state
826-
.inodes_allocation
827-
.inc(1)
828-
.map_err(FsError::trap)?;
829-
let growth = name.len() + std::mem::size_of_val(&new_file);
830-
self.vfs_state.limiter.grow(growth).map_err(|_| {
831-
// Rollback inode allocation since we failed to account for
832-
// the new file's name and node size
833-
self.vfs_state.inodes_allocation.dec(1);
834-
FsError::trap(ErrorCode::InsufficientMemory)
835-
})?;
836-
837845
// Insert the new file into the parent directory
838846
match &mut parent_node.write().unwrap().kind {
839847
VfsNodeKind::File { .. } => {
@@ -843,8 +851,20 @@ impl<'a> filesystem::types::HostDescriptor for VfsCtxView<'a> {
843851
return Err(FsError::trap(ErrorCode::NotDirectory));
844852
}
845853
VfsNodeKind::Directory { children } => {
854+
let growth = name.len() + std::mem::size_of_val(&new_file);
846855
match children.entry(name) {
847856
Entry::Vacant(entry) => {
857+
// Account for resource usage
858+
self.vfs_state
859+
.inodes_allocation
860+
.inc(1)
861+
.map_err(FsError::trap)?;
862+
self.vfs_state.limiter.grow(growth).map_err(|_| {
863+
// Rollback inode allocation since we failed to account for
864+
// the new file's name and node size
865+
self.vfs_state.inodes_allocation.dec(1);
866+
FsError::trap(ErrorCode::InsufficientMemory)
867+
})?;
848868
entry.insert(Arc::clone(&new_file));
849869
}
850870
Entry::Occupied(_) => {
@@ -860,6 +880,10 @@ impl<'a> filesystem::types::HostDescriptor for VfsCtxView<'a> {
860880

861881
new_file
862882
}
883+
(Err(e), _, _, _, _) => {
884+
// Path parsing error
885+
return Err(FsError::trap(e));
886+
}
863887
};
864888

865889
let res = self
@@ -939,6 +963,7 @@ impl<'a> filesystem::types::HostDescriptor for VfsCtxView<'a> {
939963
) -> FsResult<MetadataHashValue> {
940964
Ok(self
941965
.node_at(self_, &path)?
966+
.unwrap()
942967
.read()
943968
.unwrap()
944969
.metadata_hash(&self.vfs_state.metadata_hash_key))
@@ -1231,13 +1256,14 @@ mod tests {
12311256
let node = ctx.node_at(desc, name).unwrap();
12321257

12331258
{
1259+
let node = node.unwrap();
12341260
let mut guard = node.write().unwrap();
12351261
if let VfsNodeKind::File { content: c } = &mut guard.kind {
12361262
*c = content;
12371263
}
1264+
drop(guard);
1265+
node
12381266
}
1239-
1240-
node
12411267
}
12421268

12431269
// ==================== create_directory_at tests ====================
@@ -1290,7 +1316,7 @@ mod tests {
12901316

12911317
let desc = create_test_descriptor(&mut ctx, DescriptorFlags::READ);
12921318
let node = ctx.node_at(desc, "testdir").unwrap();
1293-
assert_is_directory(&node);
1319+
assert_is_directory(&node.unwrap());
12941320
}
12951321

12961322
#[tokio::test]
@@ -1567,7 +1593,7 @@ mod tests {
15671593

15681594
let desc = create_test_descriptor(&mut ctx, DescriptorFlags::READ);
15691595
let node = ctx.node_at(desc, "newfile").unwrap();
1570-
assert_empty_file(&node);
1596+
assert_empty_file(&node.unwrap());
15711597
}
15721598

15731599
#[tokio::test]
@@ -1845,7 +1871,7 @@ mod tests {
18451871

18461872
let desc = create_test_descriptor(&mut ctx, DescriptorFlags::READ);
18471873
let node = ctx.node_at(desc, "newfile").unwrap();
1848-
assert_empty_file(&node);
1874+
assert_empty_file(&node.unwrap());
18491875
}
18501876

18511877
#[tokio::test]

0 commit comments

Comments
 (0)