Skip to content

Commit

Permalink
Merge pull request #562 from peterfang/deprecate-fixedstring
Browse files Browse the repository at this point in the history 
string: Deprecate FixedString
  • Loading branch information
@joergroedel
joergroedel authored Dec 16, 2024
2 parents b50c35d + d488de9 commit ec92d83
Show file tree
Hide file tree
Showing 10 changed files with 88 additions and 205 deletions.
28 changes: 15 additions & 13 deletions kernel/src/acpi/tables.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ extern crate alloc;

use crate::error::SvsmError;
use crate::fw_cfg::FwCfg;
use crate::string::FixedString;
use alloc::string::{FromUtf8Error, String};
use alloc::vec::Vec;
use core::mem;
use core::str;
use zerocopy::{FromBytes, FromZeros, Immutable, IntoBytes, KnownLayout};

/// ACPI Root System Description Pointer (RSDP)
Expand Down Expand Up @@ -122,11 +123,11 @@ impl ACPITableHeader {

/// Print a human-readable summary of the ACPI table header's fields
#[expect(dead_code)]
fn print_summary(&self) {
let sig = FixedString::from(self.sig);
let oem_id = FixedString::from(self.oem_id);
let oem_table_id = FixedString::from(self.oem_table_id);
let compiler_id = FixedString::from(self.compiler_id);
fn print_summary(&self) -> Result<(), str::Utf8Error> {
let sig = str::from_utf8(&self.sig)?;
let oem_id = str::from_utf8(&self.oem_id)?;
let oem_table_id = str::from_utf8(&self.oem_table_id)?;
let compiler_id = str::from_utf8(&self.compiler_id)?;
log::trace!(
"ACPI: [{} {} {} {} {} {} {} {} {}]",
sig,
Expand All @@ -139,6 +140,7 @@ impl ACPITableHeader {
compiler_id,
self.compiler_rev
);
Ok(())
}
}

Expand Down Expand Up @@ -182,8 +184,8 @@ impl ACPITable {
///
/// This method returns the 4-character signature of the ACPI table, such as "APIC."
#[expect(dead_code)]
fn signature(&self) -> FixedString<4> {
FixedString::from(self.header.sig)
fn signature(&self) -> Result<String, FromUtf8Error> {
String::from_utf8(Vec::from(&self.header.sig))
}

/// Get the content of the ACPI table.
Expand Down Expand Up @@ -227,7 +229,7 @@ impl ACPITable {
#[derive(Debug)]
struct ACPITableMeta {
/// 4-character signature of the table
sig: FixedString<4>,
sig: String,
/// The offset of the table within the table buffer
offset: usize,
}
Expand All @@ -245,9 +247,9 @@ impl ACPITableMeta {
/// # Returns
///
/// A new [`ACPITableMeta`] instance.
fn new(header: &RawACPITableHeader, offset: usize) -> Self {
let sig = FixedString::from(header.sig);
Self { sig, offset }
fn new(header: &RawACPITableHeader, offset: usize) -> Result<Self, SvsmError> {
let sig = String::from_utf8(Vec::from(&header.sig)).map_err(|_| SvsmError::Acpi)?;
Ok(Self { sig, offset })
}
}

Expand Down Expand Up @@ -322,7 +324,7 @@ impl ACPITableBuffer {
let raw_header = self.buf.get(offset..).ok_or(SvsmError::Acpi)?;
let (raw_header, _) =
RawACPITableHeader::ref_from_prefix(raw_header).map_err(|_| SvsmError::Acpi)?;
let meta = ACPITableMeta::new(raw_header, offset);
let meta = ACPITableMeta::new(raw_header, offset)?;
self.tables.push(meta);
}

Expand Down
11 changes: 4 additions & 7 deletions kernel/src/fs/api.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
// Author: Joerg Roedel <[email protected]>

extern crate alloc;
use alloc::string::String;
use alloc::sync::Arc;
use alloc::vec::Vec;

Expand All @@ -13,13 +14,9 @@ use core::fmt::Debug;
use crate::error::SvsmError;
use crate::fs::Buffer;
use crate::mm::PageRef;
use crate::string::FixedString;
use packit::PackItError;

/// Maximum supported length for a single filename
const MAX_FILENAME_LENGTH: usize = 64;
pub type FileName = FixedString<MAX_FILENAME_LENGTH>;
pub type FileNameArray = [u8; MAX_FILENAME_LENGTH];
pub type FileName = String;

/// Represents the type of error occured
/// while doing SVSM filesystem operations.
Expand Down Expand Up @@ -207,7 +204,7 @@ pub trait Directory: Debug + Send + Sync {
/// [`Result<DirEntry, SvsmError>`]: A [`Result`] containing the [`DirEntry`]
/// corresponding to the entry being looked up in the directory if present, or
/// an [`SvsmError`] if not present.
fn lookup_entry(&self, name: FileName) -> Result<DirEntry, SvsmError>;
fn lookup_entry(&self, name: &FileName) -> Result<DirEntry, SvsmError>;

/// Used to create a new file in the directory.
///
Expand Down Expand Up @@ -243,7 +240,7 @@ pub trait Directory: Debug + Send + Sync {
///
/// [`Result<(), SvsmError>`]: A [`Result`] containing the empty
/// value on success, or an [`SvsmError`] on failure
fn unlink(&self, name: FileName) -> Result<(), SvsmError>;
fn unlink(&self, name: &FileName) -> Result<(), SvsmError>;
}

/// Represents a directory entry which could
Expand Down
16 changes: 8 additions & 8 deletions kernel/src/fs/filesystem.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ where

for item in path_items {
let dir_name = FileName::from(item);
let dir_entry = current_dir.lookup_entry(dir_name)?;
let dir_entry = current_dir.lookup_entry(&dir_name)?;
current_dir = match dir_entry {
DirEntry::File(_) => return Err(SvsmError::FileSystem(FsError::file_not_found())),
DirEntry::Directory(dir) => dir,
Expand Down Expand Up @@ -472,7 +472,7 @@ where

for item in path_items {
let dir_name = FileName::from(item);
let lookup = current_dir.lookup_entry(dir_name);
let lookup = current_dir.lookup_entry(&dir_name);
let dir_entry = match lookup {
Ok(entry) => entry,
Err(_) => DirEntry::Directory(current_dir.create_directory(dir_name)?),
Expand Down Expand Up @@ -510,7 +510,7 @@ pub fn open_root(
let file_name = FileName::from(path_items.next_back().unwrap());
let current_dir = walk_path(root_dir, path_items)?;

let dir_entry = current_dir.lookup_entry(file_name)?;
let dir_entry = current_dir.lookup_entry(&file_name)?;

match dir_entry {
DirEntry::Directory(_) => Err(SvsmError::FileSystem(FsError::file_not_found())),
Expand Down Expand Up @@ -652,7 +652,7 @@ pub fn create_all(path: &str) -> Result<FileHandle, SvsmError> {
let file_name = FileName::from(path_items.next_back().unwrap());
let current_dir = walk_path_create(path_items)?;

if file_name.length() == 0 {
if file_name.is_empty() {
return Err(SvsmError::FileSystem(FsError::inval()));
}

Expand Down Expand Up @@ -716,8 +716,8 @@ pub fn unlink_root(root_dir: Arc<dyn Directory>, path: &str) -> Result<(), SvsmE
let entry_name = FileName::from(path_items.next_back().unwrap());
let dir = walk_path(root_dir, path_items)?;

match dir.lookup_entry(entry_name)? {
DirEntry::File(_) => dir.unlink(entry_name),
match dir.lookup_entry(&entry_name)? {
DirEntry::File(_) => dir.unlink(&entry_name),
DirEntry::Directory(_) => Err(SvsmError::FileSystem(FsError::is_dir())),
}
}
Expand Down Expand Up @@ -757,11 +757,11 @@ pub fn rmdir_root(root_dir: Arc<dyn Directory>, path: &str) -> Result<(), SvsmEr
let entry_name = FileName::from(path_items.next_back().unwrap());
let dir = walk_path(root_dir, path_items)?;

match dir.lookup_entry(entry_name)? {
match dir.lookup_entry(&entry_name)? {
DirEntry::File(_) => Err(SvsmError::FileSystem(FsError::is_file())),
DirEntry::Directory(target) => {
target.prepare_remove()?;
dir.unlink(entry_name)
dir.unlink(&entry_name)
}
}
}
Expand Down
22 changes: 11 additions & 11 deletions kernel/src/fs/obj.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,23 +55,23 @@ impl FsObj {
}

pub fn read_buffer(&self, buffer: &mut dyn Buffer) -> Result<usize, SvsmError> {
let FsObjEntry::File(ref fh) = self.entry else {
let FsObjEntry::File(fh) = &self.entry else {
return Err(SvsmError::NotSupported);
};

fh.read_buffer(buffer)
}

pub fn write_buffer(&self, buffer: &dyn Buffer) -> Result<usize, SvsmError> {
let FsObjEntry::File(ref fh) = self.entry else {
let FsObjEntry::File(fh) = &self.entry else {
return Err(SvsmError::NotSupported);
};

fh.write_buffer(buffer)
}

pub fn seek_abs(&self, offset: usize) -> Result<usize, SvsmError> {
let FsObjEntry::File(ref fh) = self.entry else {
let FsObjEntry::File(fh) = &self.entry else {
return Err(SvsmError::NotSupported);
};

Expand All @@ -80,7 +80,7 @@ impl FsObj {
}

pub fn seek_rel(&self, offset: isize) -> Result<usize, SvsmError> {
let FsObjEntry::File(ref fh) = self.entry else {
let FsObjEntry::File(fh) = &self.entry else {
return Err(SvsmError::NotSupported);
};

Expand All @@ -89,7 +89,7 @@ impl FsObj {
}

pub fn seek_end(&self, offset: usize) -> Result<usize, SvsmError> {
let FsObjEntry::File(ref fh) = self.entry else {
let FsObjEntry::File(fh) = &self.entry else {
return Err(SvsmError::NotSupported);
};

Expand All @@ -98,38 +98,38 @@ impl FsObj {
}

pub fn position(&self) -> Result<usize, SvsmError> {
let FsObjEntry::File(ref fh) = self.entry else {
let FsObjEntry::File(fh) = &self.entry else {
return Err(SvsmError::NotSupported);
};

Ok(fh.position())
}

pub fn file_size(&self) -> Result<usize, SvsmError> {
let FsObjEntry::File(ref fh) = self.entry else {
let FsObjEntry::File(fh) = &self.entry else {
return Err(SvsmError::NotSupported);
};

Ok(fh.size())
}

pub fn truncate(&self, length: usize) -> Result<usize, SvsmError> {
let FsObjEntry::File(ref fh) = self.entry else {
let FsObjEntry::File(fh) = &self.entry else {
return Err(SvsmError::NotSupported);
};

fh.truncate(length)
}

pub fn readdir(&self) -> Result<Option<(FileName, DirEntry)>, SvsmError> {
let FsObjEntry::Directory(ref dh) = self.entry else {
let FsObjEntry::Directory(dh) = &self.entry else {
return Err(SvsmError::NotSupported);
};

let next = dh.next.fetch_add(1, Ordering::Relaxed);
if let Some(&name) = dh.list.get(next) {
if let Some(name) = dh.list.get(next) {
let dirent = dh.dir.lookup_entry(name)?;
Ok(Some((name, dirent)))
Ok(Some((name.clone(), dirent)))
} else {
dh.next.fetch_sub(1, Ordering::Relaxed);
Ok(None)
Expand Down
33 changes: 20 additions & 13 deletions kernel/src/fs/ramfs.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,10 @@ impl RawRamDirectory {
}

fn list(&self) -> Vec<FileName> {
self.entries.iter().map(|e| e.name).collect::<Vec<_>>()
self.entries
.iter()
.map(|e| e.name.clone())
.collect::<Vec<_>>()
}

fn prepare_remove(&mut self) -> Result<(), SvsmError> {
Expand All @@ -359,9 +362,9 @@ impl RawRamDirectory {
}
}

fn lookup_entry(&self, name: FileName) -> Result<DirEntry, SvsmError> {
fn lookup_entry(&self, name: &FileName) -> Result<DirEntry, SvsmError> {
for e in self.entries.iter() {
if e.name == name {
if &e.name == name {
return Ok(e.entry.clone());
}
}
Expand Down Expand Up @@ -399,8 +402,8 @@ impl RawRamDirectory {
Ok(new_dir)
}

fn unlink(&mut self, name: FileName) -> Result<(), SvsmError> {
let pos = self.entries.iter().position(|e| e.name == name);
fn unlink(&mut self, name: &FileName) -> Result<(), SvsmError> {
let pos = self.entries.iter().position(|e| &e.name == name);

match pos {
Some(idx) => {
Expand Down Expand Up @@ -436,7 +439,7 @@ impl Directory for RamDirectory {
self.directory.lock_write().prepare_remove()
}

fn lookup_entry(&self, name: FileName) -> Result<DirEntry, SvsmError> {
fn lookup_entry(&self, name: &FileName) -> Result<DirEntry, SvsmError> {
self.directory.lock_read().lookup_entry(name)
}

Expand All @@ -448,7 +451,7 @@ impl Directory for RamDirectory {
self.directory.lock_write().create_directory(name)
}

fn unlink(&self, name: FileName) -> Result<(), SvsmError> {
fn unlink(&self, name: &FileName) -> Result<(), SvsmError> {
self.directory.lock_write().unlink(name)
}
}
Expand Down Expand Up @@ -542,23 +545,27 @@ mod tests {

let ram_dir = RamDirectory::new();

ram_dir.create_file(f_name).expect("Failed to create file");
ram_dir
.create_directory(d_name)
.create_file(f_name.clone())
.expect("Failed to create file");
ram_dir
.create_directory(d_name.clone())
.expect("Failed to create directory");

let list = ram_dir.list();
assert_eq!(list, [f_name, d_name]);
assert_eq!(list, [f_name.clone(), d_name.clone()]);

let entry = ram_dir.lookup_entry(f_name).expect("Failed to lookup file");
let entry = ram_dir
.lookup_entry(&f_name)
.expect("Failed to lookup file");
assert!(entry.is_file());

let entry = ram_dir
.lookup_entry(d_name)
.lookup_entry(&d_name)
.expect("Failed to lookup directory");
assert!(entry.is_dir());

ram_dir.unlink(d_name).expect("Failed to unlink directory");
ram_dir.unlink(&d_name).expect("Failed to unlink directory");

let list = ram_dir.list();
assert_eq!(list, [f_name]);
Expand Down
Loading

0 comments on commit ec92d83

Please sign in to comment.