fd-commands: undo dynamic buffer sizes and add tests

Signed-off-by: leongross <[email protected]>

Author: Your Name

src/codec.rs

@@ -37,6 +37,35 @@ pub trait PldmCodec: core::fmt::Debug + Sized {
     fn decode(buffer: &[u8]) -> Result<Self, PldmCodecError>;
 }
 
+/// A trait for encoding and decoding PLDM messages with explicit lifetime requirements.
+///
+/// This trait is similar to `PldmCodec` but supports types that borrow data from the buffer
+/// during decoding, requiring an explicit lifetime parameter.
+pub trait PldmCodecWithLifetime<'a>: core::fmt::Debug + Sized {
+    /// Encodes the PLDM message into the provided byte buffer.
+    ///
+    /// # Arguments
+    ///
+    /// * `buffer` - A mutable reference to a byte slice where the encoded message will be stored.
+    ///
+    /// # Returns
+    ///
+    /// A `Result` containing the size of the encoded message on success, or a `PldmCodecError` on failure.
+    fn encode(&self, buffer: &mut [u8]) -> Result<usize, PldmCodecError>;
+
+    /// Decodes a PLDM message from the provided byte buffer.
+    ///
+    /// # Arguments
+    ///
+    /// * `buffer` - A reference to a byte slice containing the encoded message. The decoded
+    ///   type may hold references to this buffer.
+    ///
+    /// # Returns
+    ///
+    /// A `Result` containing the decoded message on success, or a `PldmCodecError` on failure.
+    fn decode(buffer: &'a [u8]) -> Result<Self, PldmCodecError>;
+}
+
 // Default implementation of PldmCodec for types that can leverage zerocopy.
 impl<T> PldmCodec for T
 where

src/message/firmware_update/get_package_data.rs

@@ -7,6 +7,7 @@ use crate::protocol::base::{
 
 use crate::pldm_completion_code;
 
+use crate::codec::{PldmCodec, PldmCodecError, PldmCodecWithLifetime};
 use crate::protocol::firmware_update::{FwUpdateCmd, FwUpdateCompletionCode};
 use zerocopy::{FromBytes, Immutable, IntoBytes};
 
@@ -53,9 +54,11 @@ pldm_completion_code! {
     }
 }
 
-#[derive(Debug, Clone, FromBytes, IntoBytes, Immutable, PartialEq)]
-#[repr(C, packed)]
-pub struct GetPackageDataResponse<'a> {
+const MAX_PORTION_DATA_SIZE: usize = 0xff;
+#[derive(Debug, Clone, PartialEq, FromBytes)]
+#[repr(C)]
+/// GetPackageDataResponse is parameterized over the portion package data size.
+pub struct GetPackageDataResponse {
     pub hdr: PldmMsgHeader<[u8; PLDM_MSG_HEADER_LEN]>,
 
     /// PLDM_BASE_CODES, COMMAND_NOT_EXPECTED, NO_PACKAGE_DATA,
@@ -69,17 +72,26 @@ pub struct GetPackageDataResponse<'a> {
     /// If the FD provided a value in the GetPackageDataMaximumTransferSize field, then the UA should
     /// select the amount of data to return such that the byte length for this field, except when TransferFlag
     /// = End or StartAndEnd, is equal to or less than that value.
-    pub portion_of_package_data: &'a [u8],
+    pub portion_of_package_data: [u8; MAX_PORTION_DATA_SIZE],
+
+    // Non-spec field. Since the spec tells us that this can vary in unknown size,
+    // we save the max and keep track of the actual size in this var.
+    // When encoding this into byte form, we have to omit this field and only
+    // encode as many bytes of [portion_of_package] as [portion_of_package_data_len]
+    // tells us to.
+    portion_of_package_data_len: usize,
 }
 
-impl<'a> GetPackageDataResponse<'a> {
+impl GetPackageDataResponse {
     pub fn new(
         instance_id: InstanceId,
         completion_code: GetPackageDataCode,
         next_data_transfer_handle: u32,
         transfer_flag: TransferOperationFlag,
-        portion_of_package_data: &'a [u8],
+        portion_of_package_data: &[u8],
     ) -> Self {
+        let mut pdata: [u8; MAX_PORTION_DATA_SIZE] = [0x00; MAX_PORTION_DATA_SIZE];
+        pdata[0..portion_of_package_data.len()].copy_from_slice(portion_of_package_data);
         GetPackageDataResponse {
             hdr: PldmMsgHeader::new(
                 instance_id,
@@ -90,11 +102,101 @@ impl<'a> GetPackageDataResponse<'a> {
             completion_code: completion_code.into(),
             next_data_transfer_handle,
             transfer_flag: transfer_flag as u8,
-            portion_of_package_data,
+            portion_of_package_data: pdata,
+            portion_of_package_data_len: portion_of_package_data.len(),
         }
     }
 }
 
+// See: src/message/firmware_update/get_fw_params.rs for manual decode etc
+impl PldmCodec for GetPackageDataResponse {
+    fn encode(&self, buffer: &mut [u8]) -> Result<usize, crate::codec::PldmCodecError> {
+        if buffer.len()
+            < core::mem::size_of::<GetPackageDataResponse>()
+                - MAX_PORTION_DATA_SIZE
+                - core::mem::size_of_val(&self.portion_of_package_data_len)
+                + self.portion_of_package_data_len
+        {
+            return Err(PldmCodecError::BufferTooShort);
+        }
+
+        dbg!(core::mem::size_of::<GetPackageDataResponse>());
+
+        let mut offset = 0;
+        buffer[offset..offset + std::mem::size_of_val(&self.hdr.0)].copy_from_slice(&self.hdr.0);
+        offset += std::mem::size_of_val(&self.hdr.0);
+        dbg!(offset);
+
+        buffer[offset] = self.completion_code;
+        offset += 1;
+
+        buffer[offset..offset + std::mem::size_of_val(&self.next_data_transfer_handle)]
+            .copy_from_slice(self.next_data_transfer_handle.as_bytes());
+        offset += std::mem::size_of_val(&self.next_data_transfer_handle);
+
+        buffer[offset] = self.transfer_flag;
+        offset += 1;
+
+        buffer[offset..offset + self.portion_of_package_data_len].copy_from_slice(
+            self.portion_of_package_data[0..self.portion_of_package_data_len].as_bytes(),
+        );
+
+        offset += self.portion_of_package_data_len;
+        dbg!(&buffer, &buffer.len(), &self.portion_of_package_data_len);
+        dbg!(self.portion_of_package_data_len);
+
+        Ok(offset)
+    }
+
+    fn decode(buffer: &[u8]) -> Result<Self, crate::codec::PldmCodecError> {
+        const MIN_LEN: usize = core::mem::size_of::<GetPackageDataResponse>()
+            - core::mem::size_of::<usize>()
+            - core::mem::size_of::<u8>() * MAX_PORTION_DATA_SIZE;
+
+        if buffer.len() < MIN_LEN {
+            return Err(PldmCodecError::BufferTooShort);
+        }
+
+        let mut offset = 0;
+
+        let mut hdr_bytes = [0u8; PLDM_MSG_HEADER_LEN];
+        hdr_bytes.copy_from_slice(&buffer[offset..offset + PLDM_MSG_HEADER_LEN]);
+
+        let hdr = PldmMsgHeader(hdr_bytes);
+        offset += PLDM_MSG_HEADER_LEN;
+
+        let completion_code = GetPackageDataCode::try_from(buffer[offset])
+            .map_err(|_| PldmCodecError::Unsupported)?;
+        offset += 1;
+
+        let next_data_transfer_handle = u32::from_le_bytes([
+            buffer[offset],
+            buffer[offset + 1],
+            buffer[offset + 2],
+            buffer[offset + 3],
+        ]);
+        offset += 4;
+
+        let transfer_flag = TransferOperationFlag::try_from(buffer[offset])
+            .map_err(|_| PldmCodecError::Unsupported)?;
+        offset += 1;
+
+        let portion_len = buffer.len() - offset;
+        if portion_len > MAX_PORTION_DATA_SIZE {
+            return Err(PldmCodecError::BufferTooShort);
+        }
+
+        let portion_data = &buffer[offset..];
+        Ok(Self::new(
+            hdr.instance_id(),
+            completion_code,
+            next_data_transfer_handle,
+            transfer_flag,
+            portion_data,
+        ))
+    }
+}
+
 /// The UA sends this command to acquire optional data that the FD shall transfer to the UA prior to
 /// beginning the transfer of component images. This command is only used if the FD has indicated in the
 /// RequestUpdate command response that it has data that shall be retrieved and restored by the UA. The
@@ -264,11 +366,75 @@ impl<'a> GetMetaDataResponse<'a> {
     }
 }
 
+impl<'a> PldmCodecWithLifetime<'a> for GetMetaDataResponse<'a> {
+    fn encode(&self, buffer: &mut [u8]) -> Result<usize, PldmCodecError> {
+        let size = core::mem::size_of::<Self>() - core::mem::size_of::<&'a [u8]>();
+        if buffer.len() < size + self.portion_of_device_metadata.len() {
+            return Err(PldmCodecError::BufferTooShort);
+        }
+
+        let mut offset = 0;
+        self.hdr
+            .write_to_prefix(&mut buffer[offset..])
+            .map_err(|_| PldmCodecError::BufferTooShort)?;
+        offset += PLDM_MSG_HEADER_LEN;
+
+        buffer[offset] = self.completion_code;
+        offset += 1;
+
+        buffer[offset..offset + 4].copy_from_slice(&self.next_data_transfer_handle.to_le_bytes());
+        offset += 4;
+
+        buffer[offset] = self.transfer_flag;
+        offset += 1;
+
+        buffer[offset..offset + self.portion_of_device_metadata.len()]
+            .copy_from_slice(self.portion_of_device_metadata);
+
+        Ok(size)
+    }
+
+    fn decode(buffer: &'a [u8]) -> Result<Self, PldmCodecError> {
+        let size = core::mem::size_of::<Self>() - core::mem::size_of::<&'a [u8]>();
+        if buffer.len() < size {
+            return Err(PldmCodecError::BufferTooShort);
+        }
+
+        let mut offset = 0;
+
+        let hdr = PldmMsgHeader::read_from_prefix(&buffer[offset..])
+            .map_err(|_| PldmCodecError::BufferTooShort)?
+            .0;
+        offset += PLDM_MSG_HEADER_LEN;
+
+        let completion_code = buffer[offset];
+        offset += 1;
+
+        let next_data_transfer_handle = u32::from_le_bytes(
+            buffer[offset..offset + 4]
+                .try_into()
+                .map_err(|_| PldmCodecError::BufferTooShort)?,
+        );
+        offset += 4;
+
+        let transfer_flag = buffer[offset];
+        offset += 1;
+
+        let portion_of_device_metadata = &buffer[offset..];
+
+        Ok(Self {
+            hdr,
+            completion_code,
+            next_data_transfer_handle,
+            transfer_flag,
+            portion_of_device_metadata,
+        })
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::protocol;
-
     use crate::codec::PldmCodec;
 
     #[test]
@@ -288,31 +454,69 @@ mod tests {
     }
 
     #[test]
-    fn test_get_data_response() {
+    fn test_get_package_data_response() {
+        const PORTION_LEN: usize = 10;
+
         let instance_id: InstanceId = 0x01;
         let next_data_transfer_handle: u32 = 0x12345678;
         let transfer_operation_flag = TransferOperationFlag::GetFirstPart;
-        let portion = [0u8; 0xff];
+        let portion = [22u8; PORTION_LEN];
 
-        let _ = GetPackageDataResponse::new(
+        let resp = GetPackageDataResponse::new(
             instance_id,
             GetPackageDataCode::BaseCodes(PldmBaseCompletionCode::Success),
             next_data_transfer_handle,
             transfer_operation_flag,
             &portion,
         );
 
-        //TODO: encoding for response does not work atm due to unknown sizes
-        // let mut buffer = [0u8; core::mem::size_of::<GetPackageDataResponse>()];
-        // request.encode(&mut buffer).unwrap();
-        // let decoded = GetPackageDataResponse::decode(&buffer).unwrap();
+        let mut buffer_fitted = [0u8; core::mem::size_of::<GetPackageDataResponse>()
+            - MAX_PORTION_DATA_SIZE
+            - core::mem::size_of::<usize>()
+            + PORTION_LEN];
+
+        dbg!(&buffer_fitted.len());
 
-        // assert_eq!(request, decoded);
+        resp.encode(&mut buffer_fitted).unwrap();
+        let decoded = GetPackageDataResponse::decode(&buffer_fitted).unwrap();
+        assert_eq!(resp, decoded);
     }
 
     #[test]
-    fn test_get_device_metadata_request() {}
+    fn test_get_device_metadata_request() {
+        let instance_id: InstanceId = 0x01;
+        let data_transfer_handle = 0x12345678;
+        let req = GetMetaDataRequest::new(
+            instance_id,
+            data_transfer_handle,
+            TransferOperationFlag::GetFirstPart,
+        );
+
+        let mut buffer = [0u8; core::mem::size_of::<GetMetaDataRequest>()];
+        req.encode(&mut buffer).unwrap();
+
+        let decoded = GetMetaDataRequest::decode(&buffer).unwrap();
+        assert_eq!(req, decoded);
+    }
 
     #[test]
-    fn test_get_device_metadata_reponse() {}
+    fn test_get_device_metadata_response() {
+        let instance_id: InstanceId = 0x01;
+        let data_transfer_handle = 0x12345678;
+        let payload = [11u8; 20];
+
+        let resp = GetMetaDataResponse::new(
+            instance_id,
+            GetMetaDataCode::BaseCodes(PldmBaseCompletionCode::Success),
+            data_transfer_handle,
+            TransferOperationFlag::GetFirstPart,
+            &payload,
+        );
+
+        let mut buffer = [0u8; 9 + 20];
+        resp.encode(&mut buffer).unwrap();
+
+        let decoded = GetMetaDataResponse::decode(&mut buffer).unwrap();
+        assert_eq!(resp, decoded);
+    }
 }

src/protocol/base.rs

@@ -289,6 +289,26 @@ macro_rules! pldm_completion_code {
                 }
             }
         }
+
+        impl TryFrom<u8> for $enum_name {
+            type Error = crate::error::PldmError;
+
+            fn try_from(code: u8) -> Result<Self, Self::Error> {
+                if let Ok(base_code) = PldmBaseCompletionCode::try_from(code) {
+                    return Ok($enum_name::BaseCodes(base_code));
+                }
+
+                match FwUpdateCompletionCode::try_from(code) {
+                    $(
+                        Ok(FwUpdateCompletionCode::$variant) => {
+                            Ok($enum_name::$variant)
+                        }
+                    )*
+                    Ok(_) => Err(crate::error::PldmError::InvalidCompletionCode),
+                    Err(e) => Err(e),
+                }
+        }
+    }
     };
 }