remove one more path dependency

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<UserFaultHook>,
         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;

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<u64, RemoteEntityConfig>);
@@ -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<RemoteEntityConfig>);
@@ -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<UserFaultHook: UserFaultHookProvider> {
     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: <https://sanaregistry.org/r/checksum_identifiers/records/4>,
+/// Entry in CRC catalogue: <https://reveng.sourceforge.io/crc-catalogue/all.htm#crc.cat.crc-32>
 pub const CRC_32: Crc<u32> = Crc::<u32>::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: <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>
 pub const CRC_32C: Crc<u32> = Crc::<u32>::new(&CRC_32_ISCSI);
 
 #[derive(Debug, PartialEq, Eq, Copy, Clone)]

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<PduSender, UserFaultHook, Vfs, RemoteCfgTable, SeqCountProvider>
 {
+    /// 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<UserFaultHook>,
         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<super::TransmissionMode> {
         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
     }