Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Remove user attributes
The motivation for this change is to avoid increasing the interchange size
with the larger data for files.

The justification for removing it are the following:
- They are not used anywhere
- They were actually never written, so even if someone used them they did nothing when writing
- Their usage was not documented at all
  • Loading branch information
sosthene-nitrokey committed Mar 1, 2023
commit 14078b7934a5cefc2f4a03587dc5dd08baa0c139
2 changes: 0 additions & 2 deletions src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,6 @@ pub mod request {
ReadDirFilesFirst:
- location: Location
- dir: PathBuf
- user_attribute: Option<UserAttribute>

ReadDirFilesNext:

Expand Down Expand Up @@ -279,7 +278,6 @@ pub mod request {
WriteFile:
- location: Location
- path: PathBuf
- user_attribute: Option<UserAttribute>
- data: LargeMessage

UnsafeInjectKey:
Expand Down
9 changes: 1 addition & 8 deletions src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -577,13 +577,8 @@ pub trait FilesystemClient: PollClient {
&mut self,
location: Location,
dir: PathBuf,
user_attribute: Option<UserAttribute>,
) -> ClientResult<'_, reply::ReadDirFilesFirst, Self> {
self.request(request::ReadDirFilesFirst {
dir,
location,
user_attribute,
})
self.request(request::ReadDirFilesFirst { dir, location })
}

fn read_dir_files_next(&mut self) -> ClientResult<'_, reply::ReadDirFilesNext, Self> {
Expand Down Expand Up @@ -651,13 +646,11 @@ pub trait FilesystemClient: PollClient {
location: Location,
path: PathBuf,
data: LargeMessage,
user_attribute: Option<UserAttribute>,
) -> ClientResult<'_, reply::WriteFile, Self> {
self.request(request::WriteFile {
location,
path,
data,
user_attribute,
})
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ impl<P: Platform> ServiceResources<P> {
}

Request::ReadDirFilesFirst(request) => {
let maybe_data = match filestore.read_dir_files_first(&request.dir, request.location, request.user_attribute.clone())? {
let maybe_data = match filestore.read_dir_files_first(&request.dir, request.location)? {
Some((data, state)) => {
ctx.read_dir_files_state = Some(state);
data
Expand Down
54 changes: 5 additions & 49 deletions src/store/filestore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::{
error::{Error, Result},
// service::ReadDirState,
store::{self, Store},
types::{Location, Message, UserAttribute},
types::{Location, Message},
Bytes,
};

Expand All @@ -17,7 +17,6 @@ pub struct ReadDirFilesState {
real_dir: PathBuf,
last: usize,
location: Location,
user_attribute: Option<UserAttribute>,
}

use littlefs2::{
Expand Down Expand Up @@ -119,7 +118,6 @@ pub trait Filestore {
&mut self,
clients_dir: &PathBuf,
location: Location,
user_attribute: Option<UserAttribute>,
) -> Result<Option<(Option<Message>, ReadDirFilesState)>>;

/// Continuation of `read_dir_files_first`.
Expand Down Expand Up @@ -271,7 +269,6 @@ impl<S: Store> Filestore for ClientFilestore<S> {
&mut self,
clients_dir: &PathBuf,
location: Location,
user_attribute: Option<UserAttribute>,
) -> Result<Option<(Option<Message>, ReadDirFilesState)>> {
if location != Location::Internal {
return Err(Error::RequestNotAvailable);
Expand All @@ -289,35 +286,13 @@ impl<S: Store> Filestore for ClientFilestore<S> {
//
// Option<usize, Result<DirEntry>> -> ??
.map(|(i, entry)| (i, entry.unwrap()))
// skip over directories (including `.` and `..`)
.filter(|(_, entry)| entry.file_type().is_file())
// take first entry that meets requirements
.find(|(_, entry)| {
if let Some(user_attribute) = user_attribute.as_ref() {
let mut path = dir.clone();
path.push(entry.file_name());
let attribute = fs
.attribute(&path, crate::config::USER_ATTRIBUTE_NUMBER)
.unwrap();

if let Some(attribute) = attribute {
user_attribute == attribute.data()
} else {
false
}
} else {
true
}
})
// if there is an entry, construct the state that needs storing out of it,
// and return the file's contents.
// the client, and return both the entry and the state
// skip over directories (including `.` and `..`) and take first file
.find(|(_, entry)| entry.file_type().is_file())
.map(|(i, entry)| {
let read_dir_files_state = ReadDirFilesState {
real_dir: dir.clone(),
last: i,
location,
user_attribute,
};
// The semantics is that for a non-existent file, we return None (not an error)
let data = store::read(self.store, location, entry.path()).ok();
Expand All @@ -340,7 +315,6 @@ impl<S: Store> Filestore for ClientFilestore<S> {
real_dir,
last,
location,
user_attribute,
} = state;
let fs = self.store.ifs();

Expand All @@ -353,31 +327,13 @@ impl<S: Store> Filestore for ClientFilestore<S> {
.skip(last + 1)
// entry is still a Result :/ (see question in `read_dir_first`)
.map(|(i, entry)| (i, entry.unwrap()))
// skip over directories (including `.` and `..`)
.filter(|(_, entry)| entry.file_type().is_file())
// take first entry that meets requirements
.find(|(_, entry)| {
if let Some(user_attribute) = user_attribute.as_ref() {
let mut path = real_dir.clone();
path.push(entry.file_name());
let attribute = fs
.attribute(&path, crate::config::USER_ATTRIBUTE_NUMBER)
.unwrap();
if let Some(attribute) = attribute {
user_attribute == attribute.data()
} else {
false
}
} else {
true
}
})
// skip over directories (including `.` and `..`) and take first file
.find(|(_, entry)| entry.file_type().is_file())
.map(|(i, entry)| {
let read_dir_files_state = ReadDirFilesState {
real_dir: real_dir.clone(),
last: i,
location,
user_attribute,
};
// The semantics is that for a non-existent file, we return None (not an error)
let data = store::read(self.store, location, entry.path()).ok();
Expand Down
7 changes: 1 addition & 6 deletions src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -528,12 +528,7 @@ fn filesystem() {

let data = Bytes::from_slice(&[0; 20]).unwrap();
block!(client
.write_file(
Location::Internal,
PathBuf::from("test_file"),
data.clone(),
None
)
.write_file(Location::Internal, PathBuf::from("test_file"), data.clone())
.expect("no client error"))
.expect("no errors");

Expand Down
2 changes: 0 additions & 2 deletions src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -616,5 +616,3 @@ pub enum SignatureSerialization {
Raw,
// Sec1,
}

pub type UserAttribute = Bytes<MAX_USER_ATTRIBUTE_LENGTH>;
2 changes: 1 addition & 1 deletion tests/virt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ fn run_test(data: u8) {

// ensure that no other client is messing with our filesystem
while syscall!(client.uptime()).uptime < Duration::from_secs(1) {
syscall!(client.write_file(location, path.clone(), write_data.clone(), None));
syscall!(client.write_file(location, path.clone(), write_data.clone()));
let read_data = syscall!(client.read_file(location, path.clone())).data;
assert_eq!(write_data, read_data);
}
Expand Down