Skip to content

Commit

Permalink
Redesign path crate to follow std::path (#1061)
Browse files Browse the repository at this point in the history
* The `path` crate now exports a `Path` and `PathBuf` struct,
  akin to `std::path`. This makes it easier to support `std`,
  but also removes unnecessary allocations throughout Theseus.

* Notably, `Path::file_stem()` now returns an `Option<&str>`.
  * This causes `crate_name_from_path()` to return a `Result`,
    which is responsible for most of the logic changes required.

Signed-off-by: Klimenty Tsoutsman <klim@tsoutsman.com>
  • Loading branch information
tsoutsman authored Oct 24, 2023
1 parent a859ff5 commit 3fd2e4c
Show file tree
Hide file tree
Showing 33 changed files with 1,021 additions and 353 deletions.
4 changes: 0 additions & 4 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 7 additions & 7 deletions applications/bm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use alloc::string::{String, ToString};
use alloc::sync::Arc;
use hpet::get_hpet;
use heapfile::HeapFile;
use path::Path;
use path::{Path, PathBuf};
use fs_node::{DirRef, FileOrDir, FileRef};
use libtest::*;
use memory::{create_mapping, PteFlags};
Expand Down Expand Up @@ -324,9 +324,9 @@ fn do_spawn_inner(overhead_ct: u64, th: usize, nr: usize, on_cpu: CpuId) -> Resu
.map_err(|_| "could not find the application namespace")?;
let namespace_dir = namespace.dir();
let app_path = namespace_dir.get_file_starting_with("hello-")
.map(|f| Path::new(f.lock().get_absolute_path()))
.map(|f| PathBuf::from(f.lock().get_absolute_path()))
.ok_or("Could not find the application 'hello'")?;
let crate_name = crate_name_from_path(&app_path).to_string();
let crate_name = crate_name_from_path(&app_path).ok_or("invalid app path")?.to_string();

// here we are taking the time at every iteration.
// otherwise the crate is not fully unloaded from the namespace before the next iteration starts
Expand All @@ -336,7 +336,7 @@ fn do_spawn_inner(overhead_ct: u64, th: usize, nr: usize, on_cpu: CpuId) -> Resu
while namespace.get_crate(&crate_name).is_some() { }

start_hpet = hpet.get_counter();
let child = spawn::new_application_task_builder(app_path.clone(), None)?
let child = spawn::new_application_task_builder(&app_path, None)?
.pin_on_cpu(on_cpu)
.spawn()?;

Expand Down Expand Up @@ -1237,7 +1237,7 @@ fn do_fs_read_with_open_inner(filename: &str, overhead_ct: u64, th: usize, nr: u
let hpet = get_hpet().ok_or("Could not retrieve hpet counter")?;


let path = Path::new(filename.to_string());
let path = PathBuf::from(filename.to_string());
let mut _dummy_sum: u64 = 0;
let mut buf = vec![0; READ_BUF_SIZE];
let size = match get_file(filename) {
Expand Down Expand Up @@ -1302,7 +1302,7 @@ fn do_fs_read_only_inner(filename: &str, overhead_ct: u64, th: usize, nr: usize)
let hpet = get_hpet().ok_or("Could not retrieve hpet counter")?;


let path = Path::new(filename.to_string());
let path = PathBuf::from(filename.to_string());
let _dummy_sum: u64 = 0;
let mut buf = vec![0; READ_BUF_SIZE];
let size = match get_file(filename) {
Expand Down Expand Up @@ -1640,7 +1640,7 @@ fn test_file_inner(fileref: FileRef) {
/// Wrapper function to get a file provided a string.
/// Not used in measurements
fn get_file(filename: &str) -> Option<FileRef> {
let path = Path::new(filename.to_string());
let path: &Path = filename.as_ref();
match path.get(&get_cwd().unwrap()) {
Some(file_dir_enum) => {
match file_dir_enum {
Expand Down
4 changes: 2 additions & 2 deletions applications/cat/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ extern crate core2;

use core::str;
use alloc::{
string::{String, ToString},
string::String,
vec::Vec,
};
use getopts::Options;
Expand Down Expand Up @@ -47,7 +47,7 @@ pub fn main(args: Vec<String>) -> isize {
println!("failed to get current task");
return -1;
};
let path = Path::new(matches.free[0].to_string());
let path: &Path = matches.free[0].as_ref();

// navigate to the filepath specified by first argument
match path.get(&cwd) {
Expand Down
6 changes: 2 additions & 4 deletions applications/cd/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,9 @@ extern crate root;
extern crate task;

use alloc::string::String;
use alloc::string::ToString;
use alloc::sync::Arc;
use alloc::vec::Vec;
use getopts::Options;
use path::Path;

pub fn main(args: Vec<String>) -> isize {
let mut opts = Options::new();
Expand All @@ -38,8 +36,8 @@ pub fn main(args: Vec<String>) -> isize {
if matches.free.is_empty() {
curr_env.lock().working_dir = Arc::clone(root::get_root());
} else {
let path = Path::new(matches.free[0].to_string());
match curr_env.lock().chdir(&path) {
let path = matches.free[0].as_ref();
match curr_env.lock().chdir(path) {
Err(environment::Error::NotADirectory) => {
println!("not a directory: {}", path);
return -1;
Expand Down
14 changes: 7 additions & 7 deletions applications/hull/src/builtin.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
//! Builtin shell commands.

use crate::{Error, Result, Shell};
use alloc::{borrow::ToOwned, string::ToString};
use alloc::string::ToString;
use app_io::println;
use path::Path;

// TODO: Decide which builtins we don't need.

Expand Down Expand Up @@ -64,15 +63,16 @@ impl Shell {
return Err(Error::Command(1));
}

let path = Path::new(if let Some(arg) = args.first() {
(*arg).to_owned()
let path = if let Some(arg) = args.first() {
*arg
} else {
"/".to_owned()
});
"/"
}
.as_ref();

let task = task::get_my_current_task().ok_or(Error::CurrentTaskUnavailable)?;

match task.get_env().lock().chdir(&path) {
match task.get_env().lock().chdir(path) {
Ok(()) => Ok(()),
Err(_) => {
println!("no such file or directory: {path}");
Expand Down
6 changes: 3 additions & 3 deletions applications/hull/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ use hashbrown::HashMap;
use job::Job;
use log::{error, warn};
use noline::{builder::EditorBuilder, sync::embedded::IO as Io};
use path::Path;
use path::PathBuf;
use stdio::Stdio;
use sync_block::Mutex;
use task::{ExitValue, KillReason};
Expand Down Expand Up @@ -306,15 +306,15 @@ impl Shell {
.into_iter();

let app_path = match matching_files.next() {
Some(f) => Path::new(f.lock().get_absolute_path()),
Some(f) => PathBuf::from(f.lock().get_absolute_path()),
None => return Err(Error::CommandNotFound(cmd.to_owned())),
};

if matching_files.next().is_some() {
println!("multiple matching files found, running: {app_path}");
}

let task = spawn::new_application_task_builder(app_path, None)
let task = spawn::new_application_task_builder(&app_path, None)
.map_err(Error::SpawnFailed)?
.argument(args.into_iter().map(ToOwned::to_owned).collect::<Vec<_>>())
.block()
Expand Down
5 changes: 2 additions & 3 deletions applications/loadc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ use alloc::{collections::BTreeSet, string::{String, ToString}, sync::Arc, vec::V
use getopts::{Matches, Options};
use memory::{Page, MappedPages, VirtualAddress, PteFlagsArch, PteFlags};
use mod_mgmt::{CrateNamespace, StrongDependency, find_symbol_table, RelocationEntry, write_relocation};
use rustc_demangle::demangle;
use path::Path;
use rustc_demangle::demangle;
use xmas_elf::{
ElfFile,
program::SegmentData,
Expand Down Expand Up @@ -72,8 +72,7 @@ fn rmain(matches: Matches) -> Result<c_int, String> {
).map_err(|_| String::from("failed to get current task"))?;

let path = matches.free.get(0).ok_or_else(|| "Missing path to ELF executable".to_string())?;
let path = Path::new(path.clone());
let file_ref = path.get_file(&curr_wd)
let file_ref = Path::new(path).get_file(&curr_wd)
.ok_or_else(|| format!("Failed to access file at {path:?}"))?;
let file = file_ref.lock();

Expand Down
3 changes: 1 addition & 2 deletions applications/ls/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ extern crate path;

use alloc::{
string::String,
string::ToString,
vec::Vec,
};
use core::fmt::Write;
Expand Down Expand Up @@ -45,7 +44,7 @@ pub fn main(args: Vec<String>) -> isize {
return 0;
}

let path = Path::new(matches.free[0].to_string());
let path: &Path = matches.free[0].as_ref();

// Navigate to the path specified by first argument
match path.get(&curr_wd) {
Expand Down
4 changes: 2 additions & 2 deletions applications/ns/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use alloc::{
use getopts::{Options, Matches};
use mod_mgmt::CrateNamespace;
use fs_node::FileRef;
use path::Path;
use path::PathBuf;


pub fn main(args: Vec<String>) -> isize {
Expand Down Expand Up @@ -68,7 +68,7 @@ fn rmain(matches: Matches) -> Result<(), String> {
let mut output = String::new();

if let Some(crate_obj_file_path) = matches.opt_str("load") {
let path = Path::new(crate_obj_file_path);
let path = PathBuf::from(crate_obj_file_path);
let file = path.get_file(&curr_wd).ok_or_else(||
format!("Couldn't resolve path to crate object file at {path:?}")
)?;
Expand Down
8 changes: 4 additions & 4 deletions applications/qemu_test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@
use alloc::{boxed::Box, string::String, vec::Vec};

use app_io::{print, println};
use path::Path;
use qemu_exit::{QEMUExit, X86};
use task::{ExitValue, KillReason};
use path::{Path, PathBuf};

extern crate alloc;

Expand All @@ -37,7 +37,7 @@ pub fn main(_: Vec<String>) -> isize {
// deadlock.
let file = dir.lock().get_file(file_name.as_ref()).unwrap();
let path = file.lock().get_absolute_path();
Some((file_name, Path::new(path)))
Some((file_name, PathBuf::from(path)))
} else {
None
}
Expand All @@ -56,7 +56,7 @@ pub fn main(_: Vec<String>) -> isize {
num_ignored += 1;
println!("ignored");
} else {
match run_test(path) {
match run_test(&path) {
Ok(_) => println!("ok"),
Err(_) => {
num_failed += 1;
Expand All @@ -81,7 +81,7 @@ pub fn main(_: Vec<String>) -> isize {
}

#[allow(clippy::result_unit_err)]
pub fn run_test(path: Path) -> Result<(), ()> {
pub fn run_test(path: &Path) -> Result<(), ()> {
match spawn::new_application_task_builder(path, None)
.unwrap()
.argument(Vec::new())
Expand Down
4 changes: 2 additions & 2 deletions applications/rm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use alloc::vec::Vec;
use alloc::string::String;
use alloc::string::ToString;
use getopts::Options;
use path::Path;
use path::PathBuf;
use fs_node::{FsNode, FileOrDir};


Expand Down Expand Up @@ -56,7 +56,7 @@ pub fn remove_node(args: Vec<String>) -> Result<(), String> {
}

for path_string in &matches.free {
let path = Path::new(path_string.clone());
let path = PathBuf::from(path_string.clone());
let node_to_delete = match path.get(&working_dir) {
Some(node) => node,
_ => return Err(format!("Couldn't find path {path}")),
Expand Down
6 changes: 3 additions & 3 deletions applications/shell/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -638,10 +638,10 @@ impl Shell {
let app_file = matching_apps.next();
let second_match = matching_apps.next(); // return an error if there are multiple matching apps
let app_path = app_file.xor(second_match)
.map(|f| Path::new(f.lock().get_absolute_path()))
.map(|f| f.lock().get_absolute_path())
.ok_or(AppErr::NotFound(cmd))?;

let taskref = spawn::new_application_task_builder(app_path, None)
let taskref = spawn::new_application_task_builder(app_path.as_ref(), None)
.map_err(|e| AppErr::SpawnErr(e.to_string()))?
.argument(args)
.block()
Expand Down Expand Up @@ -860,7 +860,7 @@ impl Shell {

// Walk through nodes existing in the command.
for node in &nodes {
let path = Path::new(node.to_string());
let path: &Path = node.as_ref();
match path.get(&curr_wd) {
Some(file_dir_enum) => {
match file_dir_enum {
Expand Down
4 changes: 2 additions & 2 deletions applications/swap/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ fn rmain(matches: Matches) -> Result<(), String> {
return Err("failed to get current task".to_string());
};
let override_namespace_crate_dir = if let Some(path) = matches.opt_str("d") {
let path = Path::new(path);
let path: &Path = path.as_ref();
let dir = match path.get(&curr_dir) {
Some(FileOrDir::Dir(dir)) => dir,
_ => return Err(format!("Error: could not find specified namespace crate directory: {path}.")),
Expand Down Expand Up @@ -166,7 +166,7 @@ fn do_swap(
let (into_new_crate_file, new_namespace) = {
if let Some(f) = override_namespace_crate_dir.as_ref().and_then(|ns_dir| ns_dir.get_file_starting_with(new_crate_str)) {
(IntoCrateObjectFile::File(f), None)
} else if let Some(FileOrDir::File(f)) = Path::new(String::from(new_crate_str)).get(curr_dir) {
} else if let Some(FileOrDir::File(f)) = Path::new(new_crate_str).get(curr_dir) {
(IntoCrateObjectFile::File(f), None)
} else {
(IntoCrateObjectFile::Prefix(String::from(new_crate_str)), None)
Expand Down
2 changes: 1 addition & 1 deletion applications/test_wasmtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ pub fn main(args: Vec<String>) -> isize {


fn rmain(args: Vec<String>) -> Result<(), String> {
let path_to_hello_cwasm = Path::new(args.get(0).cloned().unwrap_or("/extra_files/wasm/hello.cwasm".to_string()));
let path_to_hello_cwasm: &Path = args.get(0).map(|arg| &arg[..]).unwrap_or("/extra_files/wasm/hello.cwasm").as_ref();
let Ok(curr_wd) = task::with_current_task(|t| t.get_env().lock().working_dir.clone()) else {
return Err("failed to get current task".to_string());
};
Expand Down
10 changes: 6 additions & 4 deletions applications/upd/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ fn rmain(matches: Matches) -> Result<(), String> {
}
"apply" | "ap" => {
let base_dir_path = matches.free.get(1).ok_or_else(|| String::from("missing BASE_DIR path argument"))?;
apply(&Path::new(base_dir_path.clone()))
apply(base_dir_path.as_ref())
}
other => {
Err(format!("unrecognized command {other:?}"))
Expand Down Expand Up @@ -197,8 +197,8 @@ fn download(remote_endpoint: IpEndpoint, update_build: &str, crate_list: Option<
let size = content.len();
// The name of the crate file that we downloaded is something like: "/keyboard_log/k#keyboard-36be916209949cef.o".
// We need to get just the basename of the file, then remove the crate type prefix ("k#").
let df_path = Path::new(df.name);
let cfile = new_namespace_dir.write_crate_object_file(df_path.basename(), content)?;
let file_name = Path::new(&df.name).file_name().ok_or("crate file path did not have file name")?;
let cfile = new_namespace_dir.write_crate_object_file(file_name, content)?;
println!("Downloaded crate: {:?}, size {}", cfile.lock().get_absolute_path(), size);
}

Expand Down Expand Up @@ -257,7 +257,9 @@ fn apply(base_dir_path: &Path) -> Result<(), String> {
// An empty old_crate_name indicates that there is no old crate or object file to remove, we are just loading a new crate (or inserting its object file)
None
} else {
let old_crate_name = mod_mgmt::crate_name_from_path(&Path::new(old_crate_module_file_name)).to_string();
let old_crate_name = mod_mgmt::crate_name_from_path(old_crate_module_file_name.as_ref())
.ok_or("invalid old crate module file name")?
.to_string();
if curr_namespace.get_crate(&old_crate_name).is_none() {
println!("\t Note: old crate {:?} was not currently loaded into namespace {:?}.", old_crate_name, curr_namespace.name());
}
Expand Down
4 changes: 2 additions & 2 deletions applications/wasm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ pub fn main(args: Vec<String>) -> isize {

// Verify passed preopened directories are real directories.
for dir in preopened_dirs.iter() {
let dir_path = Path::new(dir.clone());
let dir_path: &Path = dir.as_ref();

match dir_path.get(&curr_wd) {
Some(file_dir_enum) => match file_dir_enum {
Expand All @@ -92,7 +92,7 @@ pub fn main(args: Vec<String>) -> isize {
return -1;
}

let wasm_binary_path = Path::new(args[0].clone());
let wasm_binary_path: &Path = args[0].as_ref();

// Parse inputted WebAssembly binary path into byte array.
let wasm_binary: Vec<u8> = match wasm_binary_path.get(&curr_wd) {
Expand Down
4 changes: 2 additions & 2 deletions kernel/console/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,8 @@ fn shell_loop(
mod_mgmt::CrateNamespace::get_crate_object_file_starting_with(&new_app_ns, "hull-")
.expect("Couldn't find hull in default app namespace");

let path = path::Path::new(app_file.lock().get_absolute_path());
let task = spawn::new_application_task_builder(path, Some(new_app_ns))?
let path = app_file.lock().get_absolute_path();
let task = spawn::new_application_task_builder(path.as_ref(), Some(new_app_ns))?
.name(format!("{address:?}_hull"))
.block()
.spawn()?;
Expand Down
Loading

0 comments on commit 3fd2e4c

Please sign in to comment.