From bd2aa84a79504dca7ea10088aa1e5f902300a02c Mon Sep 17 00:00:00 2001 From: Klim Tsoutsman Date: Sun, 18 Sep 2022 20:28:44 +1000 Subject: [PATCH 1/3] Add `vfs` API and `memory_vfs` implementation Signed-off-by: Klim Tsoutsman --- Cargo.lock | 15 ++ kernel/memory_fs/Cargo.toml | 9 + kernel/memory_fs/src/lib.rs | 272 +++++++++++++++++++++++++++++++ kernel/vfs/Cargo.toml | 8 + kernel/vfs/src/lib.rs | 74 +++++++++ kernel/vfs/src/node/directory.rs | 147 +++++++++++++++++ kernel/vfs/src/node/file.rs | 45 +++++ kernel/vfs/src/node/mod.rs | 50 ++++++ kernel/vfs/src/path.rs | 109 +++++++++++++ 9 files changed, 729 insertions(+) create mode 100644 kernel/memory_fs/Cargo.toml create mode 100644 kernel/memory_fs/src/lib.rs create mode 100644 kernel/vfs/Cargo.toml create mode 100644 kernel/vfs/src/lib.rs create mode 100644 kernel/vfs/src/node/directory.rs create mode 100644 kernel/vfs/src/node/file.rs create mode 100644 kernel/vfs/src/node/mod.rs create mode 100644 kernel/vfs/src/path.rs diff --git a/Cargo.lock b/Cargo.lock index f7ff5577da..3bd06d841e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1710,6 +1710,14 @@ dependencies = [ "zerocopy", ] +[[package]] +name = "memory_fs" +version = "0.1.0" +dependencies = [ + "spin 0.9.0", + "vfs", +] + [[package]] name = "memory_initialization" version = "0.1.0" @@ -3722,6 +3730,13 @@ version = "0.9.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "49874b5167b65d7193b8aba1567f5c7d93d001cafc34600cee003eda787e483f" +[[package]] +name = "vfs" +version = "0.1.0" +dependencies = [ + "spin 0.9.0", +] + [[package]] name = "vfs_node" version = "0.1.0" diff --git a/kernel/memory_fs/Cargo.toml b/kernel/memory_fs/Cargo.toml new file mode 100644 index 0000000000..a76eb3c0bf --- /dev/null +++ b/kernel/memory_fs/Cargo.toml @@ -0,0 +1,9 @@ +[package] +name = "memory_fs" +version = "0.1.0" +description = "In-memory file system" +edition = "2021" + +[dependencies] +spin = "0.9.0" +vfs = { path = "../vfs" } diff --git a/kernel/memory_fs/src/lib.rs b/kernel/memory_fs/src/lib.rs new file mode 100644 index 0000000000..07302735b1 --- /dev/null +++ b/kernel/memory_fs/src/lib.rs @@ -0,0 +1,272 @@ +//! In-memory file system. +//! +//! This is expected to eventually replace `memfs`. + +#![cfg_attr(not(test), no_std)] + +extern crate alloc; + +use alloc::{ + borrow::ToOwned, + string::String, + sync::{Arc, Weak}, + vec::Vec, +}; +use vfs::{Directory, File, FileSystem, Node, NodeKind, SeekFrom}; + +#[derive(Default)] +pub struct MemoryFileSystem { + root: Arc, +} + +impl MemoryFileSystem { + pub fn new() -> Self { + Self::default() + } +} + +impl FileSystem for MemoryFileSystem { + fn root(&self) -> Arc { + Arc::clone(&self.root) as Arc + } +} + +#[derive(Debug, Default)] +pub struct MemoryDirectory { + name: spin::Mutex, + parent: Option>, + nodes: spin::Mutex>, +} + +impl Node for MemoryDirectory { + fn name(&self) -> String { + self.name.lock().clone() + } + + fn set_name(&self, name: String) { + *self.name.lock() = name; + } + + fn parent(&self) -> Option> { + match self.parent { + Some(ref parent) => Some(Weak::clone(parent).upgrade()? as Arc), + None => None, + } + } + + fn as_specific(self: Arc) -> NodeKind { + NodeKind::Directory(self) + } +} + +impl Directory for MemoryDirectory { + fn get_entry(&self, name: &str) -> Option> { + self.nodes + .lock() + .iter() + .find(|node| node.name() == name) + .cloned() + .map(|node| node.into_node()) + } + + fn insert_file_entry(self: Arc, name: &str) -> Result, &'static str> { + let file = Arc::new(MemoryFile { + name: spin::Mutex::new(name.to_owned()), + parent: Arc::downgrade(&self), + ..Default::default() + }); + + let mut nodes = self.nodes.lock(); + if nodes.iter().any(|node| node.name() == name) { + return Err("node with same name already exists"); + } + nodes.push(DirectoryEntry::File(Arc::clone(&file))); + + Ok(file) + } + + fn insert_directory_entry( + self: Arc, + name: &str, + ) -> Result, &'static str> { + let dir = Arc::new(MemoryDirectory { + name: spin::Mutex::new(name.to_owned()), + parent: Some(Arc::downgrade(&self)), + ..Default::default() + }); + + let mut nodes = self.nodes.lock(); + if nodes.iter().any(|node| node.name() == name) { + return Err("node with same name already exists"); + } + nodes.push(DirectoryEntry::Directory(Arc::clone(&dir))); + + Ok(dir) + } + + fn remove_entry(self: Arc, name: &str) -> Option> { + let mut nodes = self.nodes.lock(); + let index = nodes.iter().position(|node| node.name() == name)?; + Some(nodes.remove(index).into_node()) + } + + fn list(&self) -> Vec> { + self.nodes.lock().clone().into_iter().map(|entry| entry.into_node()).collect() + } +} + +#[derive(Clone, Debug)] +enum DirectoryEntry { + Directory(Arc), + File(Arc), +} + +impl DirectoryEntry { + fn name(&self) -> String { + match self { + DirectoryEntry::Directory(dir) => dir.name(), + DirectoryEntry::File(file) => file.name(), + } + } + + fn into_node(self) -> Arc { + match self { + DirectoryEntry::Directory(dir) => dir, + DirectoryEntry::File(file) => file, + } + } +} + +#[derive(Debug, Default)] +pub struct MemoryFile { + name: spin::Mutex, + parent: Weak, + state: spin::Mutex, +} + +#[derive(Debug, Default)] +struct FileState { + offset: usize, + data: Vec, +} + +impl Node for MemoryFile { + fn name(&self) -> String { + self.name.lock().clone() + } + + fn set_name(&self, name: String) { + *self.name.lock() = name; + } + + fn parent(&self) -> Option> { + Some(Weak::clone(&self.parent).upgrade()? as Arc) + } + + fn as_specific(self: Arc) -> NodeKind { + NodeKind::File(self) + } +} + +impl File for MemoryFile { + fn read(&self, buffer: &mut [u8]) -> Result { + let mut state = self.state.lock(); + let read_len = core::cmp::min(state.data.len() - state.offset, buffer.len()); + + buffer[..read_len].copy_from_slice(&state.data[state.offset..(state.offset + read_len)]); + + state.offset += read_len; + Ok(read_len) + } + + fn write(&self, buffer: &[u8]) -> Result { + let mut state = self.state.lock(); + let write_len = core::cmp::min(state.data.len() - state.offset, buffer.len()); + + let offset = state.offset; + state.data[offset..(offset + write_len)].clone_from_slice(&buffer[..write_len]); + + if write_len < buffer.len() { + state.data.extend(&buffer[write_len..]); + } + + state.offset += buffer.len(); + Ok(buffer.len()) + } + + fn seek(&self, offset: SeekFrom) -> Result { + let mut state = self.state.lock(); + state.offset = match offset { + SeekFrom::Start(o) => o, + SeekFrom::Current(o) => (o + state.offset as isize) + .try_into() + .map_err(|_| "tried to seek to negative offset")?, + SeekFrom::End(o) => (state.data.len() as isize + o) + .try_into() + .map_err(|_| "tried to seek to negative offset")?, + }; + Ok(state.offset) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_fs_path_resolution() { + let fs = MemoryFileSystem::new(); + + let temp_dir = fs.root().insert_directory("temp".into()).unwrap(); + let foo_file = Arc::clone(&temp_dir).insert_file("foo".into()).unwrap(); + let bar_file = fs.root().insert_file("bar".into()).unwrap(); + + // We only compare the data pointers of the dynamic object. + // https://rust-lang.github.io/rust-clippy/master/index.html#vtable_address_comparisons + macro_rules! dyn_cmp { + ($left:expr, $right:expr) => { + assert_eq!(Arc::as_ptr(&$left) as *const (), Arc::as_ptr(&$right) as *const ()); + }; + } + + dyn_cmp!(fs.root(), fs.root().get_directory(".".into()).unwrap()); + dyn_cmp!(fs.root(), fs.root().get_directory(".////.//./".into()).unwrap()); + + dyn_cmp!(temp_dir, fs.root().get_directory("temp".into()).unwrap()); + dyn_cmp!(temp_dir, temp_dir.clone().get_directory(".".into()).unwrap()); + + dyn_cmp!(foo_file, fs.root().get_file("temp/foo".into()).unwrap()); + dyn_cmp!(foo_file, temp_dir.clone().get_file("foo".into()).unwrap()); + + dyn_cmp!(bar_file, fs.root().get_file("bar".into()).unwrap()); + dyn_cmp!(bar_file, temp_dir.get_file("../bar".into()).unwrap()); + } + + #[test] + fn test_files() { + let fs = MemoryFileSystem::new(); + let foo_file = fs.root().insert_file("foo".into()).unwrap(); + + assert_eq!(foo_file.write(&[0, 1, 2, 3, 4, 5]), Ok(6)); + + let mut buf = [0; 1]; + + assert_eq!(foo_file.seek(SeekFrom::Current(-1)), Ok(5)); + assert_eq!(foo_file.read(&mut buf), Ok(1)); + assert_eq!(buf, [5]); + + assert_eq!(foo_file.seek(SeekFrom::Start(1)), Ok(1)); + assert_eq!(foo_file.read(&mut buf), Ok(1)); + assert_eq!(buf, [1]); + + // Offset is at two now. + + assert_eq!(foo_file.seek(SeekFrom::Current(2)), Ok(4)); + assert_eq!(foo_file.read(&mut buf), Ok(1)); + assert_eq!(buf, [4]); + + assert_eq!(foo_file.seek(SeekFrom::End(-3)), Ok(3)); + assert_eq!(foo_file.read(&mut buf), Ok(1)); + assert_eq!(buf, [3]); + } +} diff --git a/kernel/vfs/Cargo.toml b/kernel/vfs/Cargo.toml new file mode 100644 index 0000000000..a47b91d9b9 --- /dev/null +++ b/kernel/vfs/Cargo.toml @@ -0,0 +1,8 @@ +[package] +name = "vfs" +version = "0.1.0" +description = "File system abstractions" +edition = "2021" + +[dependencies] +spin = "0.9.0" diff --git a/kernel/vfs/src/lib.rs b/kernel/vfs/src/lib.rs new file mode 100644 index 0000000000..26f48220d6 --- /dev/null +++ b/kernel/vfs/src/lib.rs @@ -0,0 +1,74 @@ +//! File system abstractions for Theseus. +//! +//! These are expected to eventually replace `fs_node`. +//! +//! Theseus uses forest mounting to keep track of different file systems. The +//! differences between forest mounting and tree mounting are explained below: +//! +//! - Forest mounting (i.e. Windows flavour): There are multiple directory tree +//! structures, with each one being a contained file system. For example, +//! Windows' drives (e.g. `C:`). Technically Windows does support folder mount +//! points but we're ignoring that for the sake of simplicity. +//! - Tree mounting (i.e. Unix flavour): There is a single directory tree +//! structure originiating in the root directory. Filesystems are mounted to +//! subdirectories of the root. +//! +//! Theseus file systems are similar to Windows drives, except they use an +//! arbitrary string identifier rather than a letter. + +#![cfg_attr(not(test), no_std)] +#![feature(trait_upcasting)] +#![allow(incomplete_features)] + +extern crate alloc; + +use alloc::{string::String, sync::Arc, vec::Vec}; +use spin::RwLock; + +mod node; +mod path; + +pub use path::*; +pub use node::*; + +/// The currently mounted file systems. +/// +/// We use a [`Vec`] rather than a [`HashMap`] because it's more performant when +/// the number of entries is in the tens. It also avoids the indirection of +/// `lazy_static`. +static FILE_SYSTEMS: RwLock)>> = RwLock::new(Vec::new()); + +/// A file system. +pub trait FileSystem: Send + Sync { + /// Returns the root directory. + fn root(&self) -> Arc; +} + +/// Returns the file system with the specified `id`. +pub fn file_system(id: &str) -> Option> { + FILE_SYSTEMS.read().iter().find(|s| s.0 == id).map(|(_, fs)| fs).cloned() +} + +/// Mounts a file system. +/// +/// # Errors +/// +/// Returns an error if a file system with the specified `id` already exists. +#[allow(clippy::result_unit_err)] +pub fn mount_file_system(id: String, fs: Arc) -> Result<(), ()> { + let mut file_systems = FILE_SYSTEMS.write(); + if file_systems.iter().any(|s| s.0 == id) { + return Err(()); + }; + file_systems.push((id, fs)); + Ok(()) +} + +/// Unmounts the file system with the specified `id`. +/// +/// Returns the file system if it exists. +pub fn unmount_file_system(id: &str) -> Option> { + let mut file_systems = FILE_SYSTEMS.write(); + let index = file_systems.iter().position(|s| s.0 == id)?; + Some(file_systems.remove(index).1) +} diff --git a/kernel/vfs/src/node/directory.rs b/kernel/vfs/src/node/directory.rs new file mode 100644 index 0000000000..6e870c2cd1 --- /dev/null +++ b/kernel/vfs/src/node/directory.rs @@ -0,0 +1,147 @@ +// TODO: We would rather these methods take &Arc, but that isn't currently +// possible. +// https://internals.rust-lang.org/t/rc-arc-borrowed-an-object-safe-version-of-rc-t-arc-t/8896/9 + +use crate::{File, Node, NodeKind, Path}; +use alloc::{sync::Arc, vec::Vec}; + +/// A directory. +/// +/// Consumers of this trait should use the methods implemented for the `dyn +/// Directory` type. +pub trait Directory: Node { + /// Retrieves the entry with the specified name. + /// + /// `name` must not contain slashes. + fn get_entry(&self, name: &str) -> Option>; + + /// Retrieves the file entry with the specified name. + /// + /// `name` must not contain slashes. + fn get_file_entry(&self, name: &str) -> Option> { + match self.get_entry(name)?.as_specific() { + NodeKind::File(file) => Some(file), + _ => None, + } + } + + /// Retrieves the directory entry with the specified name. + /// + /// `name` must not contain slashes. + fn get_directory_entry(&self, name: &str) -> Option> { + match self.get_entry(name)?.as_specific() { + NodeKind::Directory(dir) => Some(dir), + _ => None, + } + } + + /// Inserts a file with the specified name, returning it. + /// + /// `name` must not contain slashes. + fn insert_file_entry(self: Arc, name: &str) -> Result, &'static str>; + + /// Inserts a directory with the specified name, returning it. + /// + /// `name` must not contain slashes. + fn insert_directory_entry( + self: Arc, + name: &str, + ) -> Result, &'static str>; + + /// Removes the node with the specified name, returning it. + /// + /// `name` must not contain slashes. + fn remove_entry(self: Arc, name: &str) -> Option>; + + /// Returns a list of directory entries. + fn list(&self) -> Vec>; +} + +// Why we use an impl block rather than including these methods in the trait: +// https://users.rust-lang.org/t/casting-arc-t-to-arc-dyn-trait/81407 + +// FIXME: Invalid file/dir names e.g. ., .. + +impl dyn Directory { + /// Retrieves the file system node at the specified path. + /// + /// The path is relative to `self`. + pub fn get(self: Arc, path: Path) -> Option> { + let (dir_path, entry_name) = path.split_final_component(); + let dir = traverse_relative_path(self, dir_path)?; + handle_component(dir, entry_name) + } + + /// Retrieves the file at the specified path. + /// + /// The path is relative to `self`. + pub fn get_file(self: Arc, path: Path) -> Option> { + match self.get(path)?.as_specific() { + NodeKind::File(file) => Some(file), + _ => None, + } + } + + /// Retrieves the directory at the specified path. + /// + /// The path is relative to `self`. + pub fn get_directory(self: Arc, path: Path) -> Option> { + match self.get(path)?.as_specific() { + NodeKind::Directory(dir) => Some(dir), + _ => None, + } + } + + /// Inserts a file at the specified path, returning it. + /// + /// The path is relative to `self`. + pub fn insert_file(self: Arc, path: Path) -> Result, &'static str> { + let (dir_path, entry_name) = path.split_final_component(); + let dir = traverse_relative_path(self, dir_path).ok_or("directory doesn't exist")?; + dir.insert_file_entry(entry_name) + } + + /// Inserts a directory at the specified path, returning it. + /// + /// The path is relative to `self`. + pub fn insert_directory( + self: Arc, + path: Path, + ) -> Result, &'static str> { + let (dir_path, entry_name) = path.split_final_component(); + // TODO: Add option to recursively insert directories? + let dir = traverse_relative_path(self, dir_path).ok_or("directory doesn't exist")?; + dir.insert_directory_entry(entry_name) + } + + /// Removes the node at the specified path, returning it. + /// + /// The path is relative to `self`. + pub fn remove(self: Arc, path: Path) -> Option> { + let (dir_path, entry_name) = path.split_final_component(); + let dir = traverse_relative_path(self, dir_path)?; + dir.remove_entry(entry_name) + } +} + +fn handle_component(current_dir: Arc, component: &str) -> Option> { + match component { + "" | "." => Some(current_dir), + ".." => Some(current_dir.parent().unwrap_or(current_dir)), + _ => current_dir.get_entry(component), + } +} + +/// Returns the directory at the relative path from the specified directory. +fn traverse_relative_path( + mut current_dir: Arc, + path: Path, +) -> Option> { + for component in path.components() { + current_dir = match handle_component(current_dir, component)?.as_specific() { + NodeKind::Directory(dir) => dir, + _ => return None, + }; + } + Some(current_dir) +} diff --git a/kernel/vfs/src/node/file.rs b/kernel/vfs/src/node/file.rs new file mode 100644 index 0000000000..aeae1078b5 --- /dev/null +++ b/kernel/vfs/src/node/file.rs @@ -0,0 +1,45 @@ +use crate::Node; + +/// A file. +/// +/// The methods don't take a mutable reference to `self` as types implementing +/// `File` should use interior mutability. +pub trait File: Node { + /// Read some bytes from the file into the specified buffer, returning how + /// many bytes were read. + fn read(&self, buffer: &mut [u8]) -> Result; + + /// Write a buffer into this writer, returning how many bytes were written. + fn write(&self, buffer: &[u8]) -> Result; + + /// Seek to an offset, in bytes, in a stream. + /// + /// A seek beyond the end of a stream is allowed, but behaviour is defined + /// by the implementation. + /// + /// If the seek operation completed successfully, this method returns the + /// new position from the start of the stream. + fn seek(&self, pos: SeekFrom) -> Result; +} + +/// Enumeration of possible methods to seek within a file. +/// +/// It is used by the [`File::seek`] method. +pub enum SeekFrom { + /// Sets the offset to the provided number of bytes. + Start(usize), + + /// Sets the offset to the size of this object plus the specified number of + /// bytes. + /// + /// It is possible to seek beyond the end of an object, but it's an error to + /// seek before byte 0. + End(isize), + + /// Sets the offset to the current position plus the specified number of + /// bytes. + /// + /// It is possible to seek beyond the end of an object, but it's an error to + /// seek before byte 0. + Current(isize), +} diff --git a/kernel/vfs/src/node/mod.rs b/kernel/vfs/src/node/mod.rs new file mode 100644 index 0000000000..e2b8292d23 --- /dev/null +++ b/kernel/vfs/src/node/mod.rs @@ -0,0 +1,50 @@ +mod directory; +mod file; + +pub use directory::Directory; +pub use file::{File, SeekFrom}; + +use crate::{file_system, Path}; +use alloc::{string::String, sync::Arc}; + +/// A file system node. +/// +/// Types implementing this trait should also implement either `File` or +/// `Directory`. +pub trait Node { + /// Returns the node's name. + fn name(&self) -> String; + + /// Sets the node's name. + fn set_name(&self, name: String); + + /// Returns the node's parent. + /// + /// A return value of [`None`] either indicates `self` is a root directory, + /// or its parent has been removed. + fn parent(&self) -> Option>; + + /// Returns the specific node kind. + fn as_specific(self: Arc) -> NodeKind; +} + +/// A specific file system node kind. +pub enum NodeKind { + Directory(Arc), + File(Arc), +} + +/// Gets the node at the `path`. +/// +/// The path consists of a file system id, followed by a colon, followed by the +/// path. For example, `tmp:/a/b` or `nvme:/foo/bar`. +pub fn get_node(path: Path) -> Option> { + let (fs_id, mut path) = path.as_ref().split_once(':')?; + let root = file_system(fs_id)?.root(); + // Path must be absolute + path = match path.strip_prefix('/') { + Some(path) => path, + None => return None, + }; + root.get(Path::new(path)) +} diff --git a/kernel/vfs/src/path.rs b/kernel/vfs/src/path.rs new file mode 100644 index 0000000000..6fec508808 --- /dev/null +++ b/kernel/vfs/src/path.rs @@ -0,0 +1,109 @@ +/// A file system path. +#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] +pub struct Path<'a> { + inner: &'a str, +} + +/// An iterator over the components of a [`Path`]. +pub struct Components<'a> { + inner: core::str::Split<'a, char>, +} + +impl<'a> Iterator for Components<'a> { + type Item = &'a str; + + fn next(&mut self) -> Option { + self.inner.next() + } +} + +impl<'a> Path<'a> { + pub fn new(inner: &'a str) -> Self { + Self { inner } + } + + pub fn components(&self) -> Components<'a> { + let inner = match self.inner.strip_prefix('/') { + Some(inner) => inner, + None => self.inner, + }; + Components { inner: inner.split('/') } + } + + pub fn split_final_component(&self) -> (Path, &str) { + // TODO: What do we do about trailing slashes? + let (path, final_component) = self.inner.rsplit_once('/').unwrap_or(("", self.inner)); + (Path::new(path), final_component) + } +} + +impl<'a> AsRef for Path<'a> { + fn as_ref(&self) -> &str { + self.inner + } +} + +impl<'a> From<&'a str> for Path<'a> { + fn from(inner: &'a str) -> Self { + Self::new(inner) + } +} + +impl<'a> From> for &'a str { + fn from(path: Path<'a>) -> Self { + path.inner + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_components() { + let path = Path::new("/if/pirus/and/crips"); + assert_eq!(path.components().collect::>(), vec!["if", "pirus", "and", "crips"]); + + let path = Path::new("all/got/along/theyd/probably"); + assert_eq!( + path.components().collect::>(), + vec!["all", "got", "along", "theyd", "probably"] + ); + } + + #[test] + fn test_split_final_component() { + let path = Path::new("got"); + assert_eq!(path.split_final_component(), (Path::new(""), "got")); + + let path = Path::new("/me"); + assert_eq!(path.split_final_component(), (Path::new(""), "me")); + + let path = Path::new("/down/by/the"); + assert_eq!(path.split_final_component(), (Path::new("/down/by"), "the")); + + let path = Path::new("/end/of/the/song"); + assert_eq!(path.split_final_component(), (Path::new("/end/of/the"), "song")); + + let path = Path::new("seems/like/the/whole"); + assert_eq!(path.split_final_component(), (Path::new("seems/like/the"), "whole")); + + let path = Path::new("city/go/against/me"); + assert_eq!(path.split_final_component(), (Path::new("city/go/against"), "me")); + } + + #[test] + fn test_path_str_conversions() { + let path = Path::new("/every/time/im/in/the/street/i/hear"); + let string: &str = path.into(); + assert_eq!(string, "/every/time/im/in/the/street/i/hear"); + assert_eq!(string, path.as_ref()); + assert_eq!(Path::from(string), path); + + let path = Path::new("yawk/yawk/yawk/yawk"); + let string: &str = path.into(); + assert_eq!(string, "yawk/yawk/yawk/yawk"); + assert_eq!(string, path.as_ref()); + assert_eq!(Path::from(string), path); + } +} From ce2df52a3da499457f5fb4334c1a7e0f5ad732b9 Mon Sep 17 00:00:00 2001 From: Klim Tsoutsman Date: Mon, 19 Sep 2022 21:10:24 +1000 Subject: [PATCH 2/3] Use `swap_remove` when unmounting fs Signed-off-by: Klim Tsoutsman --- kernel/vfs/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/vfs/src/lib.rs b/kernel/vfs/src/lib.rs index 26f48220d6..1dcb877d9e 100644 --- a/kernel/vfs/src/lib.rs +++ b/kernel/vfs/src/lib.rs @@ -70,5 +70,5 @@ pub fn mount_file_system(id: String, fs: Arc) -> Result<(), ()> pub fn unmount_file_system(id: &str) -> Option> { let mut file_systems = FILE_SYSTEMS.write(); let index = file_systems.iter().position(|s| s.0 == id)?; - Some(file_systems.remove(index).1) + Some(file_systems.swap_remove(index).1) } From cfa9452a6e026f48738b9a855fae0a66eb3122ca Mon Sep 17 00:00:00 2001 From: Klim Tsoutsman Date: Tue, 20 Sep 2022 08:53:12 +1000 Subject: [PATCH 3/3] Rename methods to use 'create' over 'insert' Signed-off-by: Klim Tsoutsman --- kernel/memory_fs/src/lib.rs | 12 ++++++------ kernel/vfs/src/node/directory.rs | 22 +++++++++++----------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/kernel/memory_fs/src/lib.rs b/kernel/memory_fs/src/lib.rs index 07302735b1..14d4a6e38d 100644 --- a/kernel/memory_fs/src/lib.rs +++ b/kernel/memory_fs/src/lib.rs @@ -69,7 +69,7 @@ impl Directory for MemoryDirectory { .map(|node| node.into_node()) } - fn insert_file_entry(self: Arc, name: &str) -> Result, &'static str> { + fn create_file_entry(self: Arc, name: &str) -> Result, &'static str> { let file = Arc::new(MemoryFile { name: spin::Mutex::new(name.to_owned()), parent: Arc::downgrade(&self), @@ -85,7 +85,7 @@ impl Directory for MemoryDirectory { Ok(file) } - fn insert_directory_entry( + fn create_directory_entry( self: Arc, name: &str, ) -> Result, &'static str> { @@ -217,9 +217,9 @@ mod tests { fn test_fs_path_resolution() { let fs = MemoryFileSystem::new(); - let temp_dir = fs.root().insert_directory("temp".into()).unwrap(); - let foo_file = Arc::clone(&temp_dir).insert_file("foo".into()).unwrap(); - let bar_file = fs.root().insert_file("bar".into()).unwrap(); + let temp_dir = fs.root().create_directory("temp".into()).unwrap(); + let foo_file = Arc::clone(&temp_dir).create_file("foo".into()).unwrap(); + let bar_file = fs.root().create_file("bar".into()).unwrap(); // We only compare the data pointers of the dynamic object. // https://rust-lang.github.io/rust-clippy/master/index.html#vtable_address_comparisons @@ -245,7 +245,7 @@ mod tests { #[test] fn test_files() { let fs = MemoryFileSystem::new(); - let foo_file = fs.root().insert_file("foo".into()).unwrap(); + let foo_file = fs.root().create_file("foo".into()).unwrap(); assert_eq!(foo_file.write(&[0, 1, 2, 3, 4, 5]), Ok(6)); diff --git a/kernel/vfs/src/node/directory.rs b/kernel/vfs/src/node/directory.rs index 6e870c2cd1..25e8aa0e19 100644 --- a/kernel/vfs/src/node/directory.rs +++ b/kernel/vfs/src/node/directory.rs @@ -35,15 +35,15 @@ pub trait Directory: Node { } } - /// Inserts a file with the specified name, returning it. + /// Creates a file with the specified name. /// /// `name` must not contain slashes. - fn insert_file_entry(self: Arc, name: &str) -> Result, &'static str>; + fn create_file_entry(self: Arc, name: &str) -> Result, &'static str>; - /// Inserts a directory with the specified name, returning it. + /// Creates a directory with the specified name. /// /// `name` must not contain slashes. - fn insert_directory_entry( + fn create_directory_entry( self: Arc, name: &str, ) -> Result, &'static str>; @@ -92,26 +92,26 @@ impl dyn Directory { } } - /// Inserts a file at the specified path, returning it. + /// Creates a file at the specified path. /// /// The path is relative to `self`. - pub fn insert_file(self: Arc, path: Path) -> Result, &'static str> { + pub fn create_file(self: Arc, path: Path) -> Result, &'static str> { let (dir_path, entry_name) = path.split_final_component(); let dir = traverse_relative_path(self, dir_path).ok_or("directory doesn't exist")?; - dir.insert_file_entry(entry_name) + dir.create_file_entry(entry_name) } - /// Inserts a directory at the specified path, returning it. + /// Creates a directory at the specified path. /// /// The path is relative to `self`. - pub fn insert_directory( + pub fn create_directory( self: Arc, path: Path, ) -> Result, &'static str> { let (dir_path, entry_name) = path.split_final_component(); - // TODO: Add option to recursively insert directories? + // TODO: Add option to recursively create directories? let dir = traverse_relative_path(self, dir_path).ok_or("directory doesn't exist")?; - dir.insert_directory_entry(entry_name) + dir.create_directory_entry(entry_name) } /// Removes the node at the specified path, returning it.