From efb9f757ce4121f6389555a009e41dbff2b852ae Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 19 Aug 2022 12:35:21 +0200 Subject: [PATCH 1/4] move around some components --- fsrc-core/src/pool.rs | 76 ++++++++++++++++++++++++---- fsrc-core/tests/pool_test.rs | 97 ++++++++---------------------------- 2 files changed, 86 insertions(+), 87 deletions(-) diff --git a/fsrc-core/src/pool.rs b/fsrc-core/src/pool.rs index 0efa7d8..a946f28 100644 --- a/fsrc-core/src/pool.rs +++ b/fsrc-core/src/pool.rs @@ -172,12 +172,12 @@ impl LocalPool { /// size and then copy the given data to the block. Yields a [StoreAddr] which can be used /// to access the data stored in the pool pub fn add(&mut self, data: &[u8]) -> Result { - let data_len = data.as_ref().len(); + let data_len = data.len(); if data_len > Self::MAX_SIZE { return Err(StoreError::DataTooLarge(data_len)); } let addr = self.reserve(data_len)?; - self.write(&addr, data.as_ref())?; + self.write(&addr, data)?; Ok(addr) } @@ -206,7 +206,7 @@ impl LocalPool { pub fn read(&self, addr: &StoreAddr) -> Result<&[u8], StoreError> { let curr_size = self.addr_check(addr)?; let raw_pos = self.raw_pos(addr).unwrap(); - let block = &self.pool.get(addr.pool_idx as usize).unwrap()[raw_pos..curr_size]; + let block = &self.pool.get(addr.pool_idx as usize).unwrap()[raw_pos..raw_pos + curr_size]; Ok(block) } @@ -215,14 +215,36 @@ impl LocalPool { self.addr_check(&addr)?; let block_size = self.pool_cfg.cfg.get(addr.pool_idx as usize).unwrap().1; let raw_pos = self.raw_pos(&addr).unwrap(); - let block = &mut self.pool.get_mut(addr.pool_idx as usize).unwrap()[raw_pos..block_size]; + let block = &mut self.pool.get_mut(addr.pool_idx as usize).unwrap()[raw_pos..raw_pos + block_size]; let size_list = self.sizes_lists.get_mut(addr.pool_idx as usize).unwrap(); size_list[addr.packet_idx as usize] = Self::STORE_FREE; block.fill(0); Ok(()) } + pub fn has_element_at(&self, addr: &StoreAddr) -> Result { + self.validate_addr(addr)?; + let pool_idx = addr.pool_idx as usize; + let size_list = self.sizes_lists.get(pool_idx).unwrap(); + let curr_size = size_list[addr.packet_idx as usize]; + if curr_size == Self::STORE_FREE { + return Ok(false) + } + Ok(true) + } + fn addr_check(&self, addr: &StoreAddr) -> Result { + self.validate_addr(addr)?; + let pool_idx = addr.pool_idx as usize; + let size_list = self.sizes_lists.get(pool_idx).unwrap(); + let curr_size = size_list[addr.packet_idx as usize]; + if curr_size == Self::STORE_FREE { + return Err(StoreError::DataDoesNotExist(*addr)); + } + Ok(curr_size) + } + + fn validate_addr(&self, addr: &StoreAddr) -> Result<(), StoreError> { let pool_idx = addr.pool_idx as usize; if pool_idx as usize >= self.pool_cfg.cfg.len() { return Err(StoreError::InvalidStoreId( @@ -236,12 +258,7 @@ impl LocalPool { Some(*addr), )); } - let size_list = self.sizes_lists.get(pool_idx).unwrap(); - let curr_size = size_list[addr.packet_idx as usize]; - if curr_size == Self::STORE_FREE { - return Err(StoreError::DataDoesNotExist(*addr)); - } - Ok(curr_size) + Ok(()) } fn reserve(&mut self, data_len: usize) -> Result { @@ -279,7 +296,7 @@ impl LocalPool { addr )) })?; - let pool_slice = &mut subpool[packet_pos..self.pool_cfg.cfg[addr.pool_idx as usize].1]; + let pool_slice = &mut subpool[packet_pos..packet_pos + data.len()]; pool_slice.copy_from_slice(data); Ok(()) } @@ -306,6 +323,43 @@ impl LocalPool { } } +pub struct PoolGuard<'a> { + pool: &'a mut LocalPool, + pub addr: StoreAddr, + no_deletion: bool, + deletion_failed_error: Option +} + +impl<'a> PoolGuard<'a> { + pub fn new(pool: &'a mut LocalPool, addr: StoreAddr) -> Self { + Self { + pool, + addr, + no_deletion: false, + deletion_failed_error: None + } + } + + pub fn read(&self) -> Result<&[u8], StoreError> { + self.pool.read(&self.addr) + } + + pub fn release(&mut self) { + self.no_deletion = true; + } +} + +impl Drop for PoolGuard<'_> { + fn drop(&mut self) { + if !self.no_deletion { + let res = self.pool.delete(self.addr); + if res.is_err() { + self.deletion_failed_error = Some(res.unwrap_err()); + } + } + } +} + #[cfg(test)] mod tests { use crate::pool::{LocalPool, PoolCfg, StoreAddr, StoreError, StoreIdError}; diff --git a/fsrc-core/tests/pool_test.rs b/fsrc-core/tests/pool_test.rs index cc6a57e..9f6cf08 100644 --- a/fsrc-core/tests/pool_test.rs +++ b/fsrc-core/tests/pool_test.rs @@ -1,91 +1,36 @@ +use std::ops::DerefMut; use std::sync::{Arc, RwLock}; use std::thread; +use fsrc_core::pool::{LocalPool, PoolCfg, StoreAddr, PoolGuard}; +use std::sync::mpsc::{Sender, Receiver}; +use std::sync::mpsc; -struct PoolDummy { - test_buf: [u8; 128], -} +const DUMMY_DATA: [u8; 4] = [0, 1, 2, 3]; -struct PoolAccessDummy<'a> { - pool_dummy: &'a mut PoolDummy, - no_deletion: bool, -} - -impl PoolAccessDummy<'_> { - fn modify(&mut self) -> &mut [u8] { - self.pool_dummy.modify() - } - - fn release(&mut self) { - self.no_deletion = true; - } -} - -impl Drop for PoolAccessDummy<'_> { - fn drop(&mut self) { - if self.no_deletion { - println!("Pool access: Drop with no deletion") - } else { - self.pool_dummy.delete(); - println!("Pool access: Drop with deletion"); - } - } -} - -impl Default for PoolDummy { - fn default() -> Self { - PoolDummy { test_buf: [0; 128] } - } -} - -impl PoolDummy { - fn modify(&mut self) -> &mut [u8] { - self.test_buf.as_mut_slice() - } - - fn modify_with_accessor(&mut self) -> PoolAccessDummy { - PoolAccessDummy { - pool_dummy: self, - no_deletion: false, - } - } - - fn read(&self) -> &[u8] { - self.test_buf.as_slice() - } - - fn delete(&mut self) { - println!("Store content was deleted"); - } -} - -fn pool_test() { - println!("Hello World"); - let shared_dummy = Arc::new(RwLock::new(PoolDummy::default())); +#[test] +fn threaded_usage() { + let pool_cfg = PoolCfg::new(vec![(16, 6), (32, 3), (8, 12)]); + let shared_dummy = Arc::new(RwLock::new(LocalPool::new(pool_cfg))); let shared_clone = shared_dummy.clone(); - let jh0 = thread::spawn(move || loop { + let (tx, rx): (Sender, Receiver) = mpsc::channel(); + let jh0 = thread::spawn(move || { { let mut dummy = shared_dummy.write().unwrap(); - let buf = dummy.modify(); - buf[0] = 1; - - let mut accessor = dummy.modify_with_accessor(); - let buf = accessor.modify(); - buf[0] = 2; + let addr = dummy.add(&DUMMY_DATA).expect("Writing data failed"); + tx.send(addr).expect("Sending store address failed"); } }); - let jh1 = thread::spawn(move || loop { + let jh1 = thread::spawn(move || { + let mut pool_access = shared_clone.write().unwrap(); + let addr; { - let dummy = shared_clone.read().unwrap(); - let buf = dummy.read(); - println!("Buffer 0: {:?}", buf[0]); + addr = rx.recv().expect("Receiving store address failed"); + let pg = PoolGuard::new(pool_access.deref_mut(), addr); + let read_res = pg.read().expect("Reading failed"); + assert_eq!(read_res, DUMMY_DATA); } - - let mut dummy = shared_clone.write().unwrap(); - let mut accessor = dummy.modify_with_accessor(); - let buf = accessor.modify(); - buf[0] = 3; - accessor.release(); + assert!(!pool_access.has_element_at(&addr).expect("Invalid address")); }); jh0.join().unwrap(); jh1.join().unwrap(); From 1001700411b33e79374079e8801a39951b997b54 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 20 Aug 2022 23:21:36 +0200 Subject: [PATCH 2/4] fsrc-core no_std now --- Cargo.lock | 103 ++++++++++++---------------- fsrc-core/Cargo.toml | 19 +++-- fsrc-core/src/event_man.rs | 19 +++-- fsrc-core/src/events.rs | 3 +- fsrc-core/src/executable.rs | 8 ++- fsrc-core/src/lib.rs | 9 +++ fsrc-core/src/objects.rs | 46 ++++++++----- fsrc-core/src/pool.rs | 19 +++-- fsrc-core/src/tmtc/ccsds_distrib.rs | 2 + fsrc-core/src/tmtc/mod.rs | 2 + fsrc-core/src/tmtc/pus_distrib.rs | 5 ++ fsrc-core/tests/pool_test.rs | 14 ++-- spacepackets | 2 +- 13 files changed, 145 insertions(+), 106 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 455bb58..c5ccd13 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2,6 +2,17 @@ # It is not intended for manual editing. version = 3 +[[package]] +name = "ahash" +version = "0.7.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fcb51a0695d8f838b1ee009b3fbf66bda078cd64590202a864a8f3e8c4315c47" +dependencies = [ + "getrandom", + "once_cell", + "version_check", +] + [[package]] name = "aho-corasick" version = "0.7.18" @@ -222,7 +233,8 @@ version = "0.1.0" dependencies = [ "bus", "downcast-rs", - "num", + "hashbrown", + "num-traits", "once_cell", "postcard", "serde", @@ -238,6 +250,17 @@ dependencies = [ "spacepackets", ] +[[package]] +name = "getrandom" +version = "0.2.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4eb1a864a501629691edf6c15a593b7a51eebaa1e8468e9ddc623de7c9b58ec6" +dependencies = [ + "cfg-if 1.0.0", + "libc", + "wasi", +] + [[package]] name = "hash32" version = "0.2.1" @@ -247,6 +270,15 @@ dependencies = [ "byteorder", ] +[[package]] +name = "hashbrown" +version = "0.12.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8a9ee70c43aaf417c914396645a0fa852624801b24ebb7ae78fe8272889ac888" +dependencies = [ + "ahash", +] + [[package]] name = "heapless" version = "0.7.14" @@ -337,40 +369,6 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "546c37ac5d9e56f55e73b677106873d9d9f5190605e41a856503623648488cae" -[[package]] -name = "num" -version = "0.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "43db66d1170d347f9a065114077f7dccb00c1b9478c89384490a3425279a4606" -dependencies = [ - "num-bigint", - "num-complex", - "num-integer", - "num-iter", - "num-rational", - "num-traits", -] - -[[package]] -name = "num-bigint" -version = "0.4.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f93ab6289c7b344a8a9f60f88d80aa20032336fe78da341afc91c8a2341fc75f" -dependencies = [ - "autocfg", - "num-integer", - "num-traits", -] - -[[package]] -name = "num-complex" -version = "0.4.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "97fbc387afefefd5e9e39493299f3069e14a140dd34dc19b4c1c1a8fddb6a790" -dependencies = [ - "num-traits", -] - [[package]] name = "num-integer" version = "0.1.45" @@ -381,29 +379,6 @@ dependencies = [ "num-traits", ] -[[package]] -name = "num-iter" -version = "0.1.43" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7d03e6c028c5dc5cac6e2dec0efda81fc887605bb3d884578bb6d6bf7514e252" -dependencies = [ - "autocfg", - "num-integer", - "num-traits", -] - -[[package]] -name = "num-rational" -version = "0.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d41702bd167c2df5520b384281bc111a4b5efcf7fbc4c9c222c815b07e0a6a6a" -dependencies = [ - "autocfg", - "num-bigint", - "num-integer", - "num-traits", -] - [[package]] name = "num-traits" version = "0.2.15" @@ -676,6 +651,12 @@ version = "0.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "77439c1b53d2303b20d9459b1ade71a83c716e3f9c34f3228c00e6f185d6c002" +[[package]] +name = "version_check" +version = "0.9.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "49874b5167b65d7193b8aba1567f5c7d93d001cafc34600cee003eda787e483f" + [[package]] name = "void" version = "1.0.2" @@ -691,6 +672,12 @@ dependencies = [ "vcell", ] +[[package]] +name = "wasi" +version = "0.11.0+wasi-snapshot-preview1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9c8d87e72b64a3b4db28d11ce29237c246188f4f51057d65a7eab63b7987e423" + [[package]] name = "wasm-bindgen" version = "0.2.82" diff --git a/fsrc-core/Cargo.toml b/fsrc-core/Cargo.toml index b93b956..81c5928 100644 --- a/fsrc-core/Cargo.toml +++ b/fsrc-core/Cargo.toml @@ -7,16 +7,23 @@ edition = "2021" [dependencies] thiserror = "1.0" -bus = "2.2.3" -num = "0.4" +hashbrown = "0.12.3" -[dependencies.spacepackets] -path = "../spacepackets" +[dependencies.num-traits] +version = "0.2" +default-features = false [dependencies.downcast-rs] version = "1.2.0" default-features = false +[dependencies.bus] +version = "2.2.3" +optional = true + +[dependencies.spacepackets] +path = "../spacepackets" + [dev-dependencies] postcard = { version = "1.0.1", features = ["use-std"] } serde = "1.0.143" @@ -25,4 +32,6 @@ once_cell = "1.13.1" [features] default = ["std"] -std = ["downcast-rs/std"] +std = ["downcast-rs/std", "alloc", "bus"] +alloc = [] + diff --git a/fsrc-core/src/event_man.rs b/fsrc-core/src/event_man.rs index dd219c8..83e4543 100644 --- a/fsrc-core/src/event_man.rs +++ b/fsrc-core/src/event_man.rs @@ -1,6 +1,9 @@ //! [Event][crate::events::Event] management and forwarding use crate::events::{Event, EventRaw, GroupId}; -use std::collections::HashMap; +use alloc::boxed::Box; +use alloc::vec; +use alloc::vec::Vec; +use hashbrown::HashMap; #[derive(PartialEq, Eq, Hash, Copy, Clone)] enum ListenerType { @@ -62,11 +65,14 @@ impl EventManager { key: ListenerType, dest: impl EventListener + 'static, ) { - if let std::collections::hash_map::Entry::Vacant(e) = self.listeners.entry(key) { - e.insert(vec![Listener { - ltype: key, - dest: Box::new(dest), - }]); + if !self.listeners.contains_key(&key) { + self.listeners.insert( + key, + vec![Listener { + ltype: key, + dest: Box::new(dest), + }], + ); } else { let vec = self.listeners.get_mut(&key).unwrap(); // To prevent double insertions @@ -117,6 +123,7 @@ mod tests { use super::{EventListener, HandlerResult, ReceivesAllEvent}; use crate::event_man::EventManager; use crate::events::{Event, Severity}; + use alloc::boxed::Box; use std::sync::mpsc::{channel, Receiver, SendError, Sender}; use std::thread; use std::time::Duration; diff --git a/fsrc-core/src/events.rs b/fsrc-core/src/events.rs index dec000c..be351b9 100644 --- a/fsrc-core/src/events.rs +++ b/fsrc-core/src/events.rs @@ -1,5 +1,4 @@ //! Event support module -use num::pow; pub type GroupId = u16; pub type UniqueId = u16; @@ -47,7 +46,7 @@ impl Event { /// * `unique_id`: Each event has a unique 16 bit ID occupying the last 16 bits of the /// raw event ID pub fn new(severity: Severity, group_id: GroupId, unique_id: UniqueId) -> Option { - if group_id > (pow::pow(2u8 as u16, 13) - 1) { + if group_id > (2u16.pow(13) - 1) { return None; } Some(Event { diff --git a/fsrc-core/src/executable.rs b/fsrc-core/src/executable.rs index 4ca4db0..440f8ee 100644 --- a/fsrc-core/src/executable.rs +++ b/fsrc-core/src/executable.rs @@ -1,10 +1,13 @@ //! Task scheduling module use bus::BusReader; +use std::boxed::Box; use std::error::Error; use std::sync::mpsc::TryRecvError; use std::thread; use std::thread::JoinHandle; use std::time::Duration; +use std::vec; +use std::vec::Vec; #[derive(Debug, PartialEq, Eq)] pub enum OpResult { @@ -138,10 +141,13 @@ pub fn exec_sched_multi< mod tests { use super::{exec_sched_multi, exec_sched_single, Executable, ExecutionType, OpResult}; use bus::Bus; + use std::boxed::Box; use std::error::Error; + use std::string::{String, ToString}; use std::sync::{Arc, Mutex}; use std::time::Duration; - use std::{fmt, thread}; + use std::vec::Vec; + use std::{fmt, thread, vec}; struct TestInfo { exec_num: u32, diff --git a/fsrc-core/src/lib.rs b/fsrc-core/src/lib.rs index f9bed14..70d4b93 100644 --- a/fsrc-core/src/lib.rs +++ b/fsrc-core/src/lib.rs @@ -7,12 +7,21 @@ //! The crates can generally be used in a `no_std` environment, but some crates expect that the //! [alloc](https://doc.rust-lang.org/alloc) crate is available to allow boxed trait objects. //! These are used to supply user code to the crates. +#![no_std] +#[cfg(feature = "alloc")] +extern crate alloc; +#[cfg(any(feature = "std", test))] +extern crate std; + pub mod error; +#[cfg(feature = "alloc")] pub mod event_man; pub mod events; +#[cfg(feature = "std")] pub mod executable; pub mod hal; pub mod objects; +#[cfg(feature = "alloc")] pub mod pool; pub mod tmtc; diff --git a/fsrc-core/src/objects.rs b/fsrc-core/src/objects.rs index 34fdb90..0654f98 100644 --- a/fsrc-core/src/objects.rs +++ b/fsrc-core/src/objects.rs @@ -29,12 +29,12 @@ //! } //! //! impl SystemObject for ExampleSysObj { -//! +//! type Error = (); //! fn get_object_id(&self) -> &ObjectId { //! &self.id //! } //! -//! fn initialize(&mut self) -> Result<(), Box> { +//! fn initialize(&mut self) -> Result<(), Self::Error> { //! self.was_initialized = true; //! Ok(()) //! } @@ -51,9 +51,12 @@ //! assert_eq!(example_obj.id, obj_id); //! assert_eq!(example_obj.dummy, 42); //! ``` - +use alloc::boxed::Box; +#[cfg(not(feature = "std"))] +use alloc::vec::Vec; use downcast_rs::Downcast; -use std::collections::HashMap; +use hashbrown::HashMap; +#[cfg(feature = "std")] use std::error::Error; #[derive(PartialEq, Eq, Hash, Copy, Clone, Debug)] @@ -65,33 +68,37 @@ pub struct ObjectId { /// Each object which is stored inside the [object manager][ObjectManager] needs to implemented /// this trait pub trait SystemObject: Downcast { + type Error; fn get_object_id(&self) -> &ObjectId; - fn initialize(&mut self) -> Result<(), Box>; + fn initialize(&mut self) -> Result<(), Self::Error>; } -downcast_rs::impl_downcast!(SystemObject); +downcast_rs::impl_downcast!(SystemObject assoc Error); pub trait ManagedSystemObject: SystemObject + Send {} -downcast_rs::impl_downcast!(ManagedSystemObject); +downcast_rs::impl_downcast!(ManagedSystemObject assoc Error); /// Helper module to manage multiple [ManagedSystemObjects][ManagedSystemObject] by mapping them /// using an [object ID][ObjectId] -pub struct ObjectManager { - obj_map: HashMap>, +#[cfg(feature = "alloc")] +pub struct ObjectManager { + obj_map: HashMap>>, } -impl Default for ObjectManager { +#[cfg(feature = "alloc")] +impl Default for ObjectManager { fn default() -> Self { Self::new() } } -impl ObjectManager { - pub fn new() -> ObjectManager { +#[cfg(feature = "alloc")] +impl ObjectManager { + pub fn new() -> Self { ObjectManager { obj_map: HashMap::new(), } } - pub fn insert(&mut self, sys_obj: Box) -> bool { + pub fn insert(&mut self, sys_obj: Box>) -> bool { let obj_id = sys_obj.get_object_id(); if self.obj_map.contains_key(obj_id) { return false; @@ -114,14 +121,14 @@ impl ObjectManager { /// Retrieve a reference to an object stored inside the manager. The type to retrieve needs to /// be explicitly passed as a generic parameter or specified on the left hand side of the /// expression. - pub fn get_ref(&self, key: &ObjectId) -> Option<&T> { + pub fn get_ref>(&self, key: &ObjectId) -> Option<&T> { self.obj_map.get(key).and_then(|o| o.downcast_ref::()) } /// Retrieve a mutable reference to an object stored inside the manager. The type to retrieve /// needs to be explicitly passed as a generic parameter or specified on the left hand side /// of the expression. - pub fn get_mut(&mut self, key: &ObjectId) -> Option<&mut T> { + pub fn get_mut>(&mut self, key: &ObjectId) -> Option<&mut T> { self.obj_map .get_mut(key) .and_then(|o| o.downcast_mut::()) @@ -131,7 +138,8 @@ impl ObjectManager { #[cfg(test)] mod tests { use crate::objects::{ManagedSystemObject, ObjectId, ObjectManager, SystemObject}; - use std::error::Error; + use std::boxed::Box; + use std::string::String; use std::sync::{Arc, Mutex}; use std::thread; @@ -152,11 +160,12 @@ mod tests { } impl SystemObject for ExampleSysObj { + type Error = (); fn get_object_id(&self) -> &ObjectId { &self.id } - fn initialize(&mut self) -> Result<(), Box> { + fn initialize(&mut self) -> Result<(), Self::Error> { self.was_initialized = true; Ok(()) } @@ -171,11 +180,12 @@ mod tests { } impl SystemObject for OtherExampleObject { + type Error = (); fn get_object_id(&self) -> &ObjectId { &self.id } - fn initialize(&mut self) -> Result<(), Box> { + fn initialize(&mut self) -> Result<(), Self::Error> { self.was_initialized = true; Ok(()) } diff --git a/fsrc-core/src/pool.rs b/fsrc-core/src/pool.rs index a946f28..e25a82f 100644 --- a/fsrc-core/src/pool.rs +++ b/fsrc-core/src/pool.rs @@ -73,6 +73,10 @@ //! assert_eq!(buf_read_back[0], 7); //! } //! ``` +use alloc::format; +use alloc::string::String; +use alloc::vec; +use alloc::vec::Vec; type NumBlocks = u16; /// Configuration structure of the [local pool][LocalPool] @@ -215,7 +219,8 @@ impl LocalPool { self.addr_check(&addr)?; let block_size = self.pool_cfg.cfg.get(addr.pool_idx as usize).unwrap().1; let raw_pos = self.raw_pos(&addr).unwrap(); - let block = &mut self.pool.get_mut(addr.pool_idx as usize).unwrap()[raw_pos..raw_pos + block_size]; + let block = + &mut self.pool.get_mut(addr.pool_idx as usize).unwrap()[raw_pos..raw_pos + block_size]; let size_list = self.sizes_lists.get_mut(addr.pool_idx as usize).unwrap(); size_list[addr.packet_idx as usize] = Self::STORE_FREE; block.fill(0); @@ -228,7 +233,7 @@ impl LocalPool { let size_list = self.sizes_lists.get(pool_idx).unwrap(); let curr_size = size_list[addr.packet_idx as usize]; if curr_size == Self::STORE_FREE { - return Ok(false) + return Ok(false); } Ok(true) } @@ -327,7 +332,7 @@ pub struct PoolGuard<'a> { pool: &'a mut LocalPool, pub addr: StoreAddr, no_deletion: bool, - deletion_failed_error: Option + deletion_failed_error: Option, } impl<'a> PoolGuard<'a> { @@ -336,7 +341,7 @@ impl<'a> PoolGuard<'a> { pool, addr, no_deletion: false, - deletion_failed_error: None + deletion_failed_error: None, } } @@ -352,9 +357,8 @@ impl<'a> PoolGuard<'a> { impl Drop for PoolGuard<'_> { fn drop(&mut self) { if !self.no_deletion { - let res = self.pool.delete(self.addr); - if res.is_err() { - self.deletion_failed_error = Some(res.unwrap_err()); + if let Err(e) = self.pool.delete(self.addr) { + self.deletion_failed_error = Some(e); } } } @@ -363,6 +367,7 @@ impl Drop for PoolGuard<'_> { #[cfg(test)] mod tests { use crate::pool::{LocalPool, PoolCfg, StoreAddr, StoreError, StoreIdError}; + use alloc::vec; #[test] fn test_cfg() { diff --git a/fsrc-core/src/tmtc/ccsds_distrib.rs b/fsrc-core/src/tmtc/ccsds_distrib.rs index dfe1238..30feb3d 100644 --- a/fsrc-core/src/tmtc/ccsds_distrib.rs +++ b/fsrc-core/src/tmtc/ccsds_distrib.rs @@ -85,6 +85,7 @@ //! mutable_ref.mutable_foo(); //! ``` use crate::tmtc::{ReceivesCcsdsTc, ReceivesTc}; +use alloc::boxed::Box; use downcast_rs::Downcast; use spacepackets::{CcsdsPacket, PacketError, SpHeader}; @@ -187,6 +188,7 @@ pub(crate) mod tests { use spacepackets::CcsdsPacket; use std::collections::VecDeque; use std::sync::{Arc, Mutex}; + use std::vec::Vec; pub fn generate_ping_tc(buf: &mut [u8]) -> &[u8] { let mut sph = SpHeader::tc(0x002, 0x34, 0).unwrap(); diff --git a/fsrc-core/src/tmtc/mod.rs b/fsrc-core/src/tmtc/mod.rs index 0c9c02c..9436047 100644 --- a/fsrc-core/src/tmtc/mod.rs +++ b/fsrc-core/src/tmtc/mod.rs @@ -9,7 +9,9 @@ use crate::error::{FsrcErrorRaw, FsrcGroupIds}; use spacepackets::tc::PusTc; use spacepackets::SpHeader; +#[cfg(feature = "alloc")] pub mod ccsds_distrib; +#[cfg(feature = "alloc")] pub mod pus_distrib; const _RAW_PACKET_ERROR: &str = "raw-tmtc"; diff --git a/fsrc-core/src/tmtc/pus_distrib.rs b/fsrc-core/src/tmtc/pus_distrib.rs index 2928110..906ad20 100644 --- a/fsrc-core/src/tmtc/pus_distrib.rs +++ b/fsrc-core/src/tmtc/pus_distrib.rs @@ -61,6 +61,7 @@ //! assert_eq!(concrete_handler_ref.handler_call_count, 1); //! ``` use crate::tmtc::{ReceivesCcsdsTc, ReceivesEcssPusTc, ReceivesTc}; +use alloc::boxed::Box; use downcast_rs::Downcast; use spacepackets::ecss::{PusError, PusPacket}; use spacepackets::tc::PusTc; @@ -138,10 +139,13 @@ mod tests { generate_ping_tc, BasicApidHandlerOwnedQueue, BasicApidHandlerSharedQueue, }; use crate::tmtc::ccsds_distrib::{ApidPacketHandler, CcsdsDistributor}; + use alloc::vec::Vec; use spacepackets::ecss::PusError; use spacepackets::tc::PusTc; use spacepackets::CcsdsPacket; + #[cfg(feature = "std")] use std::collections::VecDeque; + #[cfg(feature = "std")] use std::sync::{Arc, Mutex}; struct PusHandlerSharedQueue { @@ -244,6 +248,7 @@ mod tests { } #[test] + #[cfg(feature = "std")] fn test_pus_distribution() { let known_packet_queue = Arc::new(Mutex::default()); let unknown_packet_queue = Arc::new(Mutex::default()); diff --git a/fsrc-core/tests/pool_test.rs b/fsrc-core/tests/pool_test.rs index 9f6cf08..f06f302 100644 --- a/fsrc-core/tests/pool_test.rs +++ b/fsrc-core/tests/pool_test.rs @@ -1,9 +1,9 @@ +use fsrc_core::pool::{LocalPool, PoolCfg, PoolGuard, StoreAddr}; use std::ops::DerefMut; +use std::sync::mpsc; +use std::sync::mpsc::{Receiver, Sender}; use std::sync::{Arc, RwLock}; use std::thread; -use fsrc_core::pool::{LocalPool, PoolCfg, StoreAddr, PoolGuard}; -use std::sync::mpsc::{Sender, Receiver}; -use std::sync::mpsc; const DUMMY_DATA: [u8; 4] = [0, 1, 2, 3]; @@ -14,11 +14,9 @@ fn threaded_usage() { let shared_clone = shared_dummy.clone(); let (tx, rx): (Sender, Receiver) = mpsc::channel(); let jh0 = thread::spawn(move || { - { - let mut dummy = shared_dummy.write().unwrap(); - let addr = dummy.add(&DUMMY_DATA).expect("Writing data failed"); - tx.send(addr).expect("Sending store address failed"); - } + let mut dummy = shared_dummy.write().unwrap(); + let addr = dummy.add(&DUMMY_DATA).expect("Writing data failed"); + tx.send(addr).expect("Sending store address failed"); }); let jh1 = thread::spawn(move || { diff --git a/spacepackets b/spacepackets index 35073a4..3970061 160000 --- a/spacepackets +++ b/spacepackets @@ -1 +1 @@ -Subproject commit 35073a45a536051e3852696c501d7afa1b36a808 +Subproject commit 3970061ca1490658c3694384e57657a1dbe0cadd From 7a67dad986492cbc0e4166073b72beab93e004e2 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 20 Aug 2022 23:47:11 +0200 Subject: [PATCH 3/4] cleaned up pool tests --- fsrc-core/src/pool.rs | 285 ++++++++++++++++++++++++------------------ 1 file changed, 164 insertions(+), 121 deletions(-) diff --git a/fsrc-core/src/pool.rs b/fsrc-core/src/pool.rs index e25a82f..2a304ca 100644 --- a/fsrc-core/src/pool.rs +++ b/fsrc-core/src/pool.rs @@ -367,7 +367,13 @@ impl Drop for PoolGuard<'_> { #[cfg(test)] mod tests { use crate::pool::{LocalPool, PoolCfg, StoreAddr, StoreError, StoreIdError}; - use alloc::vec; + use std::vec; + + fn basic_small_pool() -> LocalPool { + // 4 buckets of 4 bytes, 2 of 8 bytes and 1 of 16 bytes + let pool_cfg = PoolCfg::new(vec![(4, 4), (2, 8), (1, 16)]); + LocalPool::new(pool_cfg) + } #[test] fn test_cfg() { @@ -389,10 +395,100 @@ mod tests { } #[test] - fn test_basic() { - // 4 buckets of 4 bytes, 2 of 8 bytes and 1 of 16 bytes - let pool_cfg = PoolCfg::new(vec![(4, 4), (2, 8), (1, 16)]); - let mut local_pool = LocalPool::new(pool_cfg); + fn test_add_and_read() { + let mut local_pool = basic_small_pool(); + let mut test_buf: [u8; 16] = [0; 16]; + for (i, val) in test_buf.iter_mut().enumerate() { + *val = i as u8; + } + let addr = local_pool.add(&test_buf).expect("Adding data failed"); + // Read back data and verify correctness + let res = local_pool.read(&addr); + assert!(res.is_ok()); + let buf_read_back = res.unwrap(); + assert_eq!(buf_read_back.len(), 16); + for (i, &val) in buf_read_back.iter().enumerate() { + assert_eq!(val, i as u8); + } + } + + #[test] + fn test_add_smaller_than_full_slot() { + let mut local_pool = basic_small_pool(); + let test_buf: [u8; 12] = [0; 12]; + let addr = local_pool.add(&test_buf).expect("Adding data failed"); + let res = local_pool.read(&addr).expect("Read back failed"); + assert_eq!(res.len(), 12); + } + + #[test] + fn test_delete() { + let mut local_pool = basic_small_pool(); + let test_buf: [u8; 16] = [0; 16]; + let addr = local_pool.add(&test_buf).expect("Adding data failed"); + // Delete the data + let res = local_pool.delete(addr); + assert!(res.is_ok()); + // Verify that the slot is free by trying to get a reference to it + let res = local_pool.free_element(12); + assert!(res.is_ok()); + let (addr, buf_ref) = res.unwrap(); + assert_eq!( + addr, + StoreAddr { + pool_idx: 2, + packet_idx: 0 + } + ); + assert_eq!(buf_ref.len(), 12); + } + + #[test] + fn test_modify() { + let mut local_pool = basic_small_pool(); + let mut test_buf: [u8; 16] = [0; 16]; + for (i, val) in test_buf.iter_mut().enumerate() { + *val = i as u8; + } + let addr = local_pool.add(&test_buf).expect("Adding data failed"); + + { + // Verify that the slot is free by trying to get a reference to it + let res = local_pool.modify(&addr).expect("Modifying data failed"); + res[0] = 0; + res[1] = 0x42; + } + + let res = local_pool.read(&addr).expect("Reading back data failed"); + assert_eq!(res[0], 0); + assert_eq!(res[1], 0x42); + assert_eq!(res[2], 2); + assert_eq!(res[3], 3); + } + + #[test] + fn test_consecutive_reservation() { + let mut local_pool = basic_small_pool(); + // Reserve two smaller blocks consecutively and verify that the third reservation fails + let res = local_pool.free_element(8); + assert!(res.is_ok()); + let (addr0, _) = res.unwrap(); + let res = local_pool.free_element(8); + assert!(res.is_ok()); + let (addr1, _) = res.unwrap(); + let res = local_pool.free_element(8); + assert!(res.is_err()); + let err = res.unwrap_err(); + assert_eq!(err, StoreError::StoreFull(1)); + + // Verify that the two deletions are successful + assert!(local_pool.delete(addr0).is_ok()); + assert!(local_pool.delete(addr1).is_ok()); + } + + #[test] + fn test_read_does_not_exist() { + let local_pool = basic_small_pool(); // Try to access data which does not exist let res = local_pool.read(&StoreAddr { packet_idx: 0, @@ -403,22 +499,13 @@ mod tests { res.unwrap_err(), StoreError::DataDoesNotExist { .. } )); - let mut test_buf: [u8; 16] = [0; 16]; - for (i, val) in test_buf.iter_mut().enumerate() { - *val = i as u8; - } - let res = local_pool.add(&test_buf); - assert!(res.is_ok()); - let addr = res.unwrap(); - // Only the second subpool has enough storage and only one bucket - assert_eq!( - addr, - StoreAddr { - pool_idx: 2, - packet_idx: 0 - } - ); + } + #[test] + fn test_store_full() { + let mut local_pool = basic_small_pool(); + let test_buf: [u8; 16] = [0; 16]; + assert!(local_pool.add(&test_buf).is_ok()); // The subpool is now full and the call should fail accordingly let res = local_pool.add(&test_buf); assert!(res.is_err()); @@ -427,112 +514,68 @@ mod tests { if let StoreError::StoreFull(subpool) = err { assert_eq!(subpool, 2); } + } - // Read back data and verify correctness + #[test] + fn test_invalid_pool_idx() { + let local_pool = basic_small_pool(); + let addr = StoreAddr { + pool_idx: 3, + packet_idx: 0, + }; let res = local_pool.read(&addr); - assert!(res.is_ok()); - let buf_read_back = res.unwrap(); - assert_eq!(buf_read_back.len(), 16); - for (i, &val) in buf_read_back.iter().enumerate() { - assert_eq!(val, i as u8); - } + assert!(res.is_err()); + let err = res.unwrap_err(); + assert!(matches!( + err, + StoreError::InvalidStoreId(StoreIdError::InvalidSubpool(3), Some(_)) + )); + } - // Delete the data - let res = local_pool.delete(addr); - assert!(res.is_ok()); + #[test] + fn test_invalid_packet_idx() { + let local_pool = basic_small_pool(); + let addr = StoreAddr { + pool_idx: 2, + packet_idx: 1, + }; + assert_eq!(addr.raw(), 0x00020001); + let res = local_pool.read(&addr); + assert!(res.is_err()); + let err = res.unwrap_err(); + assert!(matches!( + err, + StoreError::InvalidStoreId(StoreIdError::InvalidPacketIdx(1), Some(_)) + )); + } - { - // Verify that the slot is free by trying to get a reference to it - let res = local_pool.free_element(12); - assert!(res.is_ok()); - let (addr, buf_ref) = res.unwrap(); - assert_eq!( - addr, - StoreAddr { - pool_idx: 2, - packet_idx: 0 - } - ); - assert_eq!(buf_ref.len(), 12); - assert_eq!(buf_ref, [0; 12]); - buf_ref[0] = 5; - buf_ref[11] = 12; - } + #[test] + fn test_add_too_large() { + let mut local_pool = basic_small_pool(); + let data_too_large = [0; 20]; + let res = local_pool.add(&data_too_large); + assert!(res.is_err()); + let err = res.unwrap_err(); + assert_eq!(err, StoreError::DataTooLarge(20)); + } - { - // Try to request a slot which is too large - let res = local_pool.free_element(20); - assert!(res.is_err()); - assert_eq!(res.unwrap_err(), StoreError::DataTooLarge(20)); + #[test] + fn test_data_too_large_1() { + let mut local_pool = basic_small_pool(); + let res = local_pool.free_element(LocalPool::MAX_SIZE + 1); + assert!(res.is_err()); + assert_eq!( + res.unwrap_err(), + StoreError::DataTooLarge(LocalPool::MAX_SIZE + 1) + ); + } - // Try to modify the 12 bytes requested previously - let res = local_pool.modify(&addr); - assert!(res.is_ok()); - let buf_ref = res.unwrap(); - assert_eq!(buf_ref[0], 5); - assert_eq!(buf_ref[11], 12); - buf_ref[0] = 0; - buf_ref[11] = 0; - } - - { - let addr = StoreAddr { - pool_idx: 3, - packet_idx: 0, - }; - let res = local_pool.read(&addr); - assert!(res.is_err()); - let err = res.unwrap_err(); - assert!(matches!( - err, - StoreError::InvalidStoreId(StoreIdError::InvalidSubpool(3), Some(_)) - )); - } - - { - let addr = StoreAddr { - pool_idx: 2, - packet_idx: 1, - }; - assert_eq!(addr.raw(), 0x00020001); - let res = local_pool.read(&addr); - assert!(res.is_err()); - let err = res.unwrap_err(); - assert!(matches!( - err, - StoreError::InvalidStoreId(StoreIdError::InvalidPacketIdx(1), Some(_)) - )); - - let data_too_large = [0; 20]; - let res = local_pool.add(&data_too_large); - assert!(res.is_err()); - let err = res.unwrap_err(); - assert_eq!(err, StoreError::DataTooLarge(20)); - - let res = local_pool.free_element(LocalPool::MAX_SIZE + 1); - assert!(res.is_err()); - assert_eq!( - res.unwrap_err(), - StoreError::DataTooLarge(LocalPool::MAX_SIZE + 1) - ); - } - - { - // Reserve two smaller blocks consecutively and verify that the third reservation fails - let res = local_pool.free_element(8); - assert!(res.is_ok()); - let (addr0, _) = res.unwrap(); - let res = local_pool.free_element(8); - assert!(res.is_ok()); - let (addr1, _) = res.unwrap(); - let res = local_pool.free_element(8); - assert!(res.is_err()); - let err = res.unwrap_err(); - assert_eq!(err, StoreError::StoreFull(1)); - - // Verify that the two deletions are successful - assert!(local_pool.delete(addr0).is_ok()); - assert!(local_pool.delete(addr1).is_ok()); - } + #[test] + fn test_free_element_too_large() { + let mut local_pool = basic_small_pool(); + // Try to request a slot which is too large + let res = local_pool.free_element(20); + assert!(res.is_err()); + assert_eq!(res.unwrap_err(), StoreError::DataTooLarge(20)); } } From 103984ed6146e8ecec327f40a48378b28bf069ea Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 21 Aug 2022 00:17:46 +0200 Subject: [PATCH 4/4] add PoolRwGuard --- Cargo.lock | 1 + fsrc-core/Cargo.toml | 1 + fsrc-core/src/pool.rs | 107 +++++++++++++++++++++++++++++++++++++++++- 3 files changed, 108 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index c5ccd13..0cefc8b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -232,6 +232,7 @@ name = "fsrc-core" version = "0.1.0" dependencies = [ "bus", + "delegate", "downcast-rs", "hashbrown", "num-traits", diff --git a/fsrc-core/Cargo.toml b/fsrc-core/Cargo.toml index 81c5928..392e4f4 100644 --- a/fsrc-core/Cargo.toml +++ b/fsrc-core/Cargo.toml @@ -7,6 +7,7 @@ edition = "2021" [dependencies] thiserror = "1.0" +delegate = "0.7.0" hashbrown = "0.12.3" [dependencies.num-traits] diff --git a/fsrc-core/src/pool.rs b/fsrc-core/src/pool.rs index 2a304ca..af68d90 100644 --- a/fsrc-core/src/pool.rs +++ b/fsrc-core/src/pool.rs @@ -77,6 +77,8 @@ use alloc::format; use alloc::string::String; use alloc::vec; use alloc::vec::Vec; +use delegate::delegate; + type NumBlocks = u16; /// Configuration structure of the [local pool][LocalPool] @@ -206,6 +208,18 @@ impl LocalPool { Ok(block) } + /// This function behaves like [Self::modify], but consumes the provided address and returns a + /// RAII conformant guard object. + /// + /// Unless the guard [PoolRwGuard::release] method is called, the data for the + /// given address will be deleted automatically when the guard is dropped. + /// This can prevent memory leaks. Users can read (and modify) the data and release the guard + /// if the data in the store is valid for further processing. If the data is faulty, no + /// manual deletion is necessary when returning from a processing function prematurely. + pub fn modify_with_guard(&mut self, addr: StoreAddr) -> PoolRwGuard { + PoolRwGuard::new(self, addr) + } + /// Read data by yielding a read-only reference given a [StoreAddr] pub fn read(&self, addr: &StoreAddr) -> Result<&[u8], StoreError> { let curr_size = self.addr_check(addr)?; @@ -214,6 +228,18 @@ impl LocalPool { Ok(block) } + /// This function behaves like [Self::read], but consumes the provided address and returns a + /// RAII conformant guard object. + /// + /// Unless the guard [PoolRwGuard::release] method is called, the data for the + /// given address will be deleted automatically when the guard is dropped. + /// This can prevent memory leaks. Users can read the data and release the guard + /// if the data in the store is valid for further processing. If the data is faulty, no + /// manual deletion is necessary when returning from a processing function prematurely. + pub fn read_with_guard(&mut self, addr: StoreAddr) -> PoolGuard { + PoolGuard::new(self, addr) + } + /// Delete data inside the pool given a [StoreAddr] pub fn delete(&mut self, addr: StoreAddr) -> Result<(), StoreError> { self.addr_check(&addr)?; @@ -335,6 +361,7 @@ pub struct PoolGuard<'a> { deletion_failed_error: Option, } +/// This helper object impl<'a> PoolGuard<'a> { pub fn new(pool: &'a mut LocalPool, addr: StoreAddr) -> Self { Self { @@ -364,9 +391,34 @@ impl Drop for PoolGuard<'_> { } } +pub struct PoolRwGuard<'a> { + guard: PoolGuard<'a>, +} + +impl<'a> PoolRwGuard<'a> { + pub fn new(pool: &'a mut LocalPool, addr: StoreAddr) -> Self { + Self { + guard: PoolGuard::new(pool, addr), + } + } + + pub fn modify(&mut self) -> Result<&mut [u8], StoreError> { + self.guard.pool.modify(&self.guard.addr) + } + + delegate!( + to self.guard { + pub fn read(&self) -> Result<&[u8], StoreError>; + pub fn release(&mut self); + } + ); +} + #[cfg(test)] mod tests { - use crate::pool::{LocalPool, PoolCfg, StoreAddr, StoreError, StoreIdError}; + use crate::pool::{ + LocalPool, PoolCfg, PoolGuard, PoolRwGuard, StoreAddr, StoreError, StoreIdError, + }; use std::vec; fn basic_small_pool() -> LocalPool { @@ -578,4 +630,57 @@ mod tests { assert!(res.is_err()); assert_eq!(res.unwrap_err(), StoreError::DataTooLarge(20)); } + + #[test] + fn test_pool_guard_deletion_man_creation() { + let mut local_pool = basic_small_pool(); + let test_buf: [u8; 16] = [0; 16]; + let addr = local_pool.add(&test_buf).expect("Adding data failed"); + let read_guard = PoolGuard::new(&mut local_pool, addr); + drop(read_guard); + assert!(!local_pool.has_element_at(&addr).expect("Invalid address")); + } + + #[test] + fn test_pool_guard_deletion() { + let mut local_pool = basic_small_pool(); + let test_buf: [u8; 16] = [0; 16]; + let addr = local_pool.add(&test_buf).expect("Adding data failed"); + let read_guard = local_pool.read_with_guard(addr); + drop(read_guard); + assert!(!local_pool.has_element_at(&addr).expect("Invalid address")); + } + + #[test] + fn test_pool_guard_with_release() { + let mut local_pool = basic_small_pool(); + let test_buf: [u8; 16] = [0; 16]; + let addr = local_pool.add(&test_buf).expect("Adding data failed"); + let mut read_guard = PoolGuard::new(&mut local_pool, addr); + read_guard.release(); + drop(read_guard); + assert!(local_pool.has_element_at(&addr).expect("Invalid address")); + } + + #[test] + fn test_pool_modify_guard_man_creation() { + let mut local_pool = basic_small_pool(); + let test_buf: [u8; 16] = [0; 16]; + let addr = local_pool.add(&test_buf).expect("Adding data failed"); + let mut rw_guard = PoolRwGuard::new(&mut local_pool, addr); + let _ = rw_guard.modify().expect("modify failed"); + drop(rw_guard); + assert!(!local_pool.has_element_at(&addr).expect("Invalid address")); + } + + #[test] + fn test_pool_modify_guard() { + let mut local_pool = basic_small_pool(); + let test_buf: [u8; 16] = [0; 16]; + let addr = local_pool.add(&test_buf).expect("Adding data failed"); + let mut rw_guard = local_pool.modify_with_guard(addr); + let _ = rw_guard.modify().expect("modify failed"); + drop(rw_guard); + assert!(!local_pool.has_element_at(&addr).expect("Invalid address")); + } }