Skip to content

Commit

Permalink
Remove user attributes
Browse files Browse the repository at this point in the history
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
1 parent ebc798f commit 6d6335d
Show file tree
Hide file tree
Showing 7 changed files with 9 additions and 65 deletions.
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
50 changes: 5 additions & 45 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 @@ -291,33 +288,13 @@ impl<S: Store> Filestore for ClientFilestore<S> {
.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
// take the first entry
.next()
.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 +317,6 @@ impl<S: Store> Filestore for ClientFilestore<S> {
real_dir,
last,
location,
user_attribute,
} = state;
let fs = self.store.ifs();

Expand All @@ -355,29 +331,13 @@ impl<S: Store> Filestore for ClientFilestore<S> {
.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
}
})
// take first entry
.next()
.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

0 comments on commit 6d6335d

Please sign in to comment.