From 5a4bd9710a20ce482878ff507de6fac10094ee24 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 5 Sep 2024 00:06:40 +0200 Subject: [PATCH] remove one more path dependency --- src/dest.rs | 117 +++++++++++++++++++++++++++++++++----------------- src/lib.rs | 28 +++++++++--- src/source.rs | 63 ++++++++++++++++++++++++--- 3 files changed, 157 insertions(+), 51 deletions(-) diff --git a/src/dest.rs b/src/dest.rs index da209f7..44d6933 100644 --- a/src/dest.rs +++ b/src/dest.rs @@ -1,6 +1,6 @@ use crate::{user::TransactionFinishedParams, DummyPduProvider, GenericSendError, PduProvider}; -use core::str::{from_utf8, Utf8Error}; -use std::path::{Path, PathBuf}; +use core::str::{from_utf8, from_utf8_unchecked, Utf8Error}; +use std::path::Path; use super::{ filestore::{FilestoreError, NativeFilestore, VirtualFilestore}, @@ -33,7 +33,8 @@ struct FileProperties { src_file_name_len: usize, dest_file_name: [u8; u8::MAX as usize], dest_file_name_len: usize, - dest_path_buf: PathBuf, + dest_path_buf: [u8; u8::MAX as usize * 2], + dest_file_path_len: usize, } #[derive(Debug, PartialEq, Eq, Copy, Clone)] @@ -42,6 +43,7 @@ enum CompletionDisposition { Cancelled = 1, } +/// This enumeration models the different transaction steps of the destination entity handler. #[derive(Debug, Copy, Clone, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub enum TransactionStep { @@ -114,7 +116,8 @@ impl Default for FileProperties { src_file_name_len: Default::default(), dest_file_name: [0; u8::MAX as usize], dest_file_name_len: Default::default(), - dest_path_buf: Default::default(), + dest_path_buf: [0; u8::MAX as usize * 2], + dest_file_path_len: Default::default(), } } } @@ -198,14 +201,19 @@ pub enum DestError { /// /// The [DestinationHandler::state_machine] function is the primary function to drive the /// destination handler. It can be used to insert packets into the destination -/// handler and driving the state machine, which might generate new -/// packets to be sent to the remote entity. Please note that the destination handler can also -/// only process Metadata, EOF and Prompt PDUs in addition to ACK PDUs where the acknowledged -/// PDU is the Finished PDU. +/// handler and driving the state machine, which might generate new packets to be sent to the +/// remote entity. Please note that the destination handler can also only process Metadata, EOF and +/// Prompt PDUs in addition to ACK PDUs where the acknowledged PDU is the Finished PDU. +/// All generated packets are sent using the user provided [PduSendProvider]. /// -/// All generated packets are sent via the [CfdpPacketSender] trait, which is implemented by the -/// user and passed as a constructor parameter. The number of generated packets is returned -/// by the state machine call. +/// The handler requires the [alloc] feature but will allocated all required memory on construction +/// time. This means that the handler is still suitable for embedded systems where run-time +/// allocation is prohibited. Furthermore, it uses the [VirtualFilestore] abstraction to allow +/// usage on systems without a [std] filesystem. +/// +/// This handler does not support concurrency out of the box. Instead, if concurrent handling +/// is required, it is recommended to create a new handler and run all active handlers inside a +/// thread pool, or move the newly created handler to a new thread. pub struct DestinationHandler< PduSender: PduSendProvider, UserFaultHook: UserFaultHookProvider, @@ -256,21 +264,21 @@ impl< /// /// # Arguments /// - /// * `local_cfg` - The local CFDP entity configuration, consisting of the local entity ID, - /// the indication configuration, and the fault handlers. + /// * `local_cfg` - The local CFDP entity configuration. /// * `max_packet_len` - The maximum expected generated packet size in bytes. Each time a /// packet is sent, it will be buffered inside an internal buffer. The length of this buffer /// will be determined by this parameter. This parameter can either be a known upper bound, /// or it can specifically be determined by the largest packet size parameter of all remote /// entity configurations in the passed `remote_cfg_table`. - /// * `packet_sender` - All generated packets are sent via this abstraction. - /// * `vfs` - Virtual filestore implementation to decouple the CFDP implementation from the - /// underlying filestore/filesystem. This allows to use this handler for embedded systems - /// where a standard runtime might not be available. - /// * `remote_cfg_table` - A table of all expected remote entities this entity will communicate - /// with. It contains various configuration parameters required for file transfers. - /// * `check_timer_creator` - This is used by the CFDP handler to generate timers required - /// by various tasks. + /// * `pdu_sender` - [PduSendProvider] used to send generated PDU packets. + /// * `vfs` - [VirtualFilestore] implementation used by the handler, which decouples the CFDP + /// implementation from the underlying filestore/filesystem. This allows to use this handler + /// for embedded systems where a standard runtime might not be available. + /// * `remote_cfg_table` - The [RemoteEntityConfigProvider] used to look up remote + /// entities and target specific configuration for file copy operations. + /// * `check_timer_creator` - [CheckTimerProviderCreator] used by the CFDP handler to generate + /// timers required by various tasks. This allows to use this handler for embedded systems + /// where the standard time APIs might not be available. pub fn new( local_cfg: LocalEntityConfig, max_packet_len: usize, @@ -303,7 +311,7 @@ impl< /// packets into the destination handler. /// /// The state machine should either be called if a packet with the appropriate destination ID - /// is received, or periodically in IDLE periods to perform all CFDP related tasks, for example + /// is received and periodically to perform all CFDP related tasks, for example /// checking for timeouts or missed file segments. /// /// The function returns the number of sent PDU packets on success. @@ -470,7 +478,14 @@ impl< }); } if let Err(e) = self.vfs.write_data( - self.tparams.file_properties.dest_path_buf.to_str().unwrap(), + // Safety: It was already verified that the path is valid during the transaction start. + unsafe { + from_utf8_unchecked( + //from_utf8( + &self.tparams.file_properties.dest_path_buf + [0..self.tparams.file_properties.dest_file_path_len], + ) + }, fd_pdu.offset(), fd_pdu.file_data(), ) { @@ -556,7 +571,13 @@ impl< file_delivery_complete = true; } else { match self.vfs.checksum_verify( - self.tparams.file_properties.dest_path_buf.to_str().unwrap(), + // Safety: It was already verified that the path is valid during the transaction start. + unsafe { + from_utf8_unchecked( + &self.tparams.file_properties.dest_path_buf + [0..self.tparams.file_properties.dest_file_path_len], + ) + }, self.tparams.metadata_params().checksum_type, checksum, &mut self.tparams.cksum_buf, @@ -675,8 +696,10 @@ impl< &self.tparams.file_properties.dest_file_name [..self.tparams.file_properties.dest_file_name_len], )?; - let dest_path = Path::new(dest_name); - self.tparams.file_properties.dest_path_buf = dest_path.to_path_buf(); + //let dest_path = Path::new(dest_name); + self.tparams.file_properties.dest_path_buf[0..dest_name.len()] + .copy_from_slice(dest_name.as_bytes()); + self.tparams.file_properties.dest_file_path_len = dest_name.len(); let source_id = self.tparams.pdu_conf.source_id(); let id = TransactionId::new(source_id, self.tparams.pdu_conf.transaction_seq_num); let src_name = from_utf8( @@ -708,26 +731,37 @@ impl< self.tparams.tstate.transaction_id = Some(id); cfdp_user.metadata_recvd_indication(&metadata_recvd_params); - // TODO: This is the only remaining function which uses std.. the easiest way would - // probably be to use a static pre-allocated dest path buffer to store any concatenated - // paths. - if dest_path.exists() && self.vfs.is_dir(dest_path.to_str().unwrap())? { + if self.vfs.exists(dest_name)? && self.vfs.is_dir(dest_name)? { + // TODO: We require a VFS function to retrieve the file name from a full path to + // avoid the last std runtime dependency. + // Create new destination path by concatenating the last part of the source source // name and the destination folder. For example, for a source file of /tmp/hello.txt // and a destination name of /home/test, the resulting file name should be // /home/test/hello.txt - let source_path = Path::new(from_utf8( - &self.tparams.file_properties.src_file_name - [..self.tparams.file_properties.src_file_name_len], - )?); + // Safety: It was already verified that the path is valid during the transaction start. + let source_path = Path::new(unsafe { + from_utf8_unchecked( + //from_utf8( + &self.tparams.file_properties.src_file_name + [..self.tparams.file_properties.src_file_name_len], + ) + }); let source_name = source_path.file_name(); if source_name.is_none() { return Err(DestError::PathConcat); } let source_name = source_name.unwrap(); - self.tparams.file_properties.dest_path_buf.push(source_name); + self.tparams.file_properties.dest_path_buf[dest_name.len()] = b'/'; + self.tparams.file_properties.dest_path_buf + [dest_name.len() + 1..dest_name.len() + 1 + source_name.len()] + .copy_from_slice(source_name.as_encoded_bytes()); + self.tparams.file_properties.dest_file_path_len += 1 + source_name.len(); } - let dest_path_str = self.tparams.file_properties.dest_path_buf.to_str().unwrap(); + let dest_path_str = from_utf8( + &self.tparams.file_properties.dest_path_buf + [0..self.tparams.file_properties.dest_file_path_len], + )?; if self.vfs.exists(dest_path_str)? { self.vfs.truncate_file(dest_path_str)?; } else { @@ -763,8 +797,13 @@ impl< .disposition_on_cancellation && self.tstate().delivery_code == DeliveryCode::Incomplete { - self.vfs - .remove_file(self.tparams.file_properties.dest_path_buf.to_str().unwrap())?; + // Safety: We already verified that the path is valid during the transaction start. + self.vfs.remove_file(unsafe { + from_utf8_unchecked( + &self.tparams.file_properties.dest_path_buf + [0..self.tparams.file_properties.dest_file_path_len], + ) + })?; self.tstate_mut().file_status = FileStatus::DiscardDeliberately; } let tstate = self.tstate(); @@ -870,7 +909,7 @@ mod tests { use core::{cell::Cell, sync::atomic::AtomicBool}; #[allow(unused_imports)] use std::println; - use std::{fs, string::String}; + use std::{fs, path::PathBuf, string::String}; use alloc::{sync::Arc, vec::Vec}; use rand::Rng; diff --git a/src/lib.rs b/src/lib.rs index 9db8859..30a61f0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -219,6 +219,8 @@ pub trait RemoteEntityConfigProvider { fn remove_config(&mut self, remote_id: u64) -> bool; } +/// This is a thin wrapper around a [HashMap] to store remote entity configurations. +/// It implements the full [RemoteEntityConfigProvider] trait. #[cfg(feature = "std")] #[derive(Default)] pub struct StdRemoteEntityConfigProvider(pub HashMap); @@ -239,6 +241,8 @@ impl RemoteEntityConfigProvider for StdRemoteEntityConfigProvider { } } +/// This is a thin wrapper around a [alloc::vec::Vec] to store remote entity configurations. +/// It implements the full [RemoteEntityConfigProvider] trait. #[cfg(feature = "alloc")] #[derive(Default)] pub struct VecRemoteEntityConfigProvider(pub alloc::vec::Vec); @@ -273,6 +277,9 @@ impl RemoteEntityConfigProvider for VecRemoteEntityConfigProvider { } } +/// A remote entity configurations also implements the [RemoteEntityConfigProvider], but the +/// [RemoteEntityConfigProvider::add_config] and [RemoteEntityConfigProvider::remove_config] +/// are no-ops and always returns [false]. impl RemoteEntityConfigProvider for RemoteEntityConfig { fn get(&self, remote_id: u64) -> Option<&RemoteEntityConfig> { if remote_id == self.entity_id.value() { @@ -300,7 +307,7 @@ impl RemoteEntityConfigProvider for RemoteEntityConfig { /// This trait introduces some callbacks which will be called when a particular CFDP fault /// handler is called. /// -/// It is passed into the CFDP handlers as part of the [DefaultFaultHandler] and the local entity +/// It is passed into the CFDP handlers as part of the [UserFaultHookProvider] and the local entity /// configuration and provides a way to specify custom user error handlers. This allows to /// implement some CFDP features like fault handler logging, which would not be possible /// generically otherwise. @@ -327,6 +334,8 @@ pub trait UserFaultHookProvider { fn ignore_cb(&mut self, transaction_id: TransactionId, cond: ConditionCode, progress: u64); } +/// Dummy fault hook which implements [UserFaultHookProvider] but only provides empty +/// implementations. #[derive(Default, Debug, PartialEq, Eq, Copy, Clone)] pub struct DummyFaultHook {} @@ -364,7 +373,7 @@ impl UserFaultHookProvider for DummyFaultHook { /// It does so by mapping each applicable [spacepackets::cfdp::ConditionCode] to a fault handler /// which is denoted by the four [spacepackets::cfdp::FaultHandlerCode]s. This code is used /// to select the error handling inside the CFDP handler itself in addition to dispatching to a -/// user-provided callback function provided by the [UserFaultHandler]. +/// user-provided callback function provided by the [UserFaultHookProvider]. /// /// Some note on the provided default settings: /// @@ -487,6 +496,7 @@ impl Default for IndicationConfig { } } +/// Each CFDP entity handler has a [LocalEntityConfig]uration. pub struct LocalEntityConfig { pub id: UnsignedByteField, pub indication_cfg: IndicationConfig, @@ -563,7 +573,7 @@ pub mod std_mod { } } - /// Simple implementation of the [CheckTimerCreator] trait assuming a standard runtime. + /// Simple implementation of the [CountdownProvider] trait assuming a standard runtime. /// It also assumes that a second accuracy of the check timer period is sufficient. #[derive(Debug)] pub struct StdCheckTimer { @@ -682,11 +692,15 @@ pub enum State { Suspended = 2, } -/// SANA registry entry: https://sanaregistry.org/r/checksum_identifiers/records/4 -/// Entry in CRC catalogue: https://reveng.sourceforge.io/crc-catalogue/all.htm#crc.cat.crc-32 +/// [crc::Crc] instance using [crc::CRC_32_ISO_HDLC]. +/// +/// SANA registry entry: , +/// Entry in CRC catalogue: pub const CRC_32: Crc = Crc::::new(&CRC_32_ISO_HDLC); -/// SANA registry entry: https://sanaregistry.org/r/checksum_identifiers/records/3 -/// Entry in CRC catalogue: https://reveng.sourceforge.io/crc-catalogue/all.htm#crc.cat.crc-32-iscsi +/// [crc::Crc] instance using [crc::CRC_32_ISCSI]. +/// +/// SANA registry entry: , +/// Entry in CRC catalogue: pub const CRC_32C: Crc = Crc::::new(&CRC_32_ISCSI); #[derive(Debug, PartialEq, Eq, Copy, Clone)] diff --git a/src/source.rs b/src/source.rs index 921fcc4..115f9ea 100644 --- a/src/source.rs +++ b/src/source.rs @@ -32,6 +32,7 @@ use super::{ RemoteEntityConfigProvider, State, TransactionId, UserFaultHookProvider, }; +/// This enumeration models the different transaction steps of the source entity handler. #[derive(Debug, Copy, Clone, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub enum TransactionStep { @@ -137,6 +138,33 @@ pub enum PutRequestError { FilestoreError(#[from] FilestoreError), } +/// This is the primary CFDP source handler. It models the CFDP source entity, which is +/// primarily responsible for handling put requests to send files to another CFDP destination +/// entity. +/// +/// As such, it contains a state machine to perform all operations necessary to perform a +/// source-to-destination file transfer. This class uses the user provides [PduSendProvider] to +/// send the CFDP PDU packets generated by the state machine. +/// +/// The following core functions are the primary interface: +/// +/// 1. [Self::put_request] can be used to start transactions, most notably to start +/// and perform a Copy File procedure to send a file or to send a Proxy Put Request to request +/// a file. +/// 2. [Self::state_machine] is the primary interface to execute an +/// active file transfer. It generates the necessary CFDP PDUs for this process. +/// This method is also used to insert received packets with the appropriate destination ID +/// and target handler type into the state machine. +/// +/// A put request will only be accepted if the handler is in the idle state. +/// +/// The handler requires the [alloc] feature but will allocated all required memory on construction +/// time. This means that the handler is still suitable for embedded systems where run-time +/// allocation is prohibited. Furthermore, it uses the [VirtualFilestore] abstraction to allow +/// usage on systems without a [std] filesystem. +/// This handler does not support concurrency out of the box. Instead, if concurrent handling +/// is required, it is recommended to create a new handler and run all active handlers inside a +/// thread pool, or move the newly created handler to a new thread. pub struct SourceHandler< PduSender: PduSendProvider, UserFaultHook: UserFaultHookProvider, @@ -168,6 +196,25 @@ impl< SeqCountProvider: SequenceCountProvider, > SourceHandler { + /// Creates a new instance of a source handler. + /// + /// # Arguments + /// + /// * `cfg` - The local entity configuration for this source handler. + /// * `pdu_sender` - [PduSendProvider] provider used to send CFDP PDUs generated by the handler. + /// * `vfs` - [VirtualFilestore] implementation used by the handler, which decouples the CFDP + /// implementation from the underlying filestore/filesystem. This allows to use this handler + /// for embedded systems where a standard runtime might not be available. + /// * `put_request_cacher` - The put request cacher is used cache put requests without + /// requiring run-time allocation. + /// * `pdu_and_cksum_buf_size` - The handler requires a buffer to generate PDUs and perform + /// checksum calculations. The user can specify the size of this buffer, so this should be + /// set to the maximum expected PDU size or a conservative upper bound for this size, for + /// example 2048 or 4096 bytes. + /// * `remote_cfg_table` - The [RemoteEntityConfigProvider] used to look up remote + /// entities and target specific configuration for file copy operations. + /// * `seq_count_provider` - The [SequenceCountProvider] used to generate the [TransactionId] + /// which contains an incrementing counter. pub fn new( cfg: LocalEntityConfig, pdu_sender: PduSender, @@ -192,6 +239,7 @@ impl< } } + /// Calls [Self::state_machine], without inserting a packet. pub fn state_machine_no_packet( &mut self, cfdp_user: &mut impl CfdpUser, @@ -271,6 +319,13 @@ impl< Ok(()) } + /// This function is used to pass a put request to the source handler, which is + /// also used to start a file copy operation. As such, this function models the Put.request + /// CFDP primtiive. + + /// Please note that the source handler can also process one put request at a time. + /// The caller is responsible of creating a new source handler, one handler can only handle + /// one file copy request at a time. pub fn put_request( &mut self, put_request: &impl ReadablePutRequest, @@ -348,6 +403,7 @@ impl< derived_max_seg_len as u64 } + /// Returns the [TransmissionMode] for the active file operation. #[inline] pub fn transmission_mode(&self) -> Option { self.tstate.as_ref().map(|v| v.transmission_mode) @@ -366,7 +422,6 @@ impl< self.prepare_and_send_metadata_pdu()?; self.state_helper.step = TransactionStep::SendingFileData; sent_packets += 1; - // return Ok(1); } if self.state_helper.step == TransactionStep::SendingFileData { if let ControlFlow::Break(packets) = self.file_data_fsm()? { @@ -378,7 +433,6 @@ impl< if self.state_helper.step == TransactionStep::SendingEof { self.eof_fsm(cfdp_user)?; sent_packets += 1; - // return Ok(1); } if self.state_helper.step == TransactionStep::WaitingForFinished { /* @@ -768,13 +822,12 @@ impl< fn handle_keep_alive_pdu(&mut self) {} - /// Get the step, which denotes the exact step of a pending CFDP transaction when applicable. + /// Get the [TransactionStep], which denotes the exact step of a pending CFDP transaction when + /// applicable. pub fn step(&self) -> TransactionStep { self.state_helper.step } - /// Get the step, which denotes whether the CFDP handler is active, and which CFDP class - /// is used if it is active. pub fn state(&self) -> State { self.state_helper.state }