fix: copilot suggestions

Author: leongross

Cargo.lock

@@ -5,6 +5,5 @@ authors = ["Caliptra contributors", "OpenPRoT contributors"]
 edition = "2024"
 
 [dependencies]
-zerocopy = {version = "0.8.17", features = ["derive"]}
+zerocopy = { version = "0.8.17", features = ["derive"] }
 bitfield = "0.14.0"
-bytemuck = "1.24.0"

Cargo.toml

@@ -257,8 +257,7 @@ impl PldmCodec for GetFirmwareParametersResponse {
         offset += core::mem::size_of::<u8>();
 
         let bytes = self.parms.encode(&mut buffer[offset..])?;
-        offset += bytes;
-        Ok(offset)
+        Ok(offset + bytes)
     }
 
     fn decode(buffer: &[u8]) -> Result<Self, PldmCodecError> {

src/message/firmware_update/get_fw_params.rs

@@ -17,9 +17,9 @@ pub const GET_PACKAGE_DATA_PORTION_SIZE: usize = 1024;
 #[repr(C, packed)]
 /// The FD sends this command to transfer optional data that shall be received prior to transferring
 /// components during the firmware update process. This command is only used if the firmware update
-/// package contained content within the [FirmwareDevicePackageData] field, the UA provided the length of
+/// package contained content within the FirmwareDevicePackageData field, the UA provided the length of
 /// the package data in the RequestUpdate command, and the FD indicated that it would use this command
-/// in the [FDWillSendGetPackageDataCommand] field.
+/// in the FDWillSendGetPackageDataCommand field.
 pub struct GetPackageDataRequest {
     pub hdr: PldmMsgHeader<[u8; PLDM_MSG_HEADER_LEN]>,
     pub data_transfer_handle: u32,
@@ -120,19 +120,16 @@ impl PldmCodec for GetPackageDataResponse {
             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..offset + size_of_val(&self.hdr.0)].copy_from_slice(&self.hdr.0);
+        offset += size_of_val(&self.hdr.0);
 
         buffer[offset] = self.completion_code;
         offset += 1;
 
-        buffer[offset..offset + std::mem::size_of_val(&self.next_data_transfer_handle)]
+        buffer[offset..offset + 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);
+        offset += size_of_val(&self.next_data_transfer_handle);
 
         buffer[offset] = self.transfer_flag;
         offset += 1;
@@ -141,11 +138,7 @@ impl PldmCodec for GetPackageDataResponse {
             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)
+        Ok(offset + self.portion_of_package_data_len)
     }
 
     fn decode(buffer: &[u8]) -> Result<Self, crate::codec::PldmCodecError> {
@@ -309,7 +302,7 @@ impl<'a> PldmCodecWithLifetime<'a> for GetDeviceMetaDataResponse<'a> {
         buffer[offset..offset + self.portion_of_device_metadata.len()]
             .copy_from_slice(self.portion_of_device_metadata);
 
-        Ok(size)
+        Ok(offset + self.portion_of_device_metadata.len())
     }
 
     fn decode(buffer: &'a [u8]) -> Result<Self, PldmCodecError> {
@@ -350,8 +343,8 @@ impl<'a> PldmCodecWithLifetime<'a> for GetDeviceMetaDataResponse<'a> {
 }
 
 /// The FD sends this command to transfer the data that was originally obtained by the UA through the
-/// [GetDeviceMetaData] command. This command shall only be used if the FD indicated in the
-/// [RequestUpdate] response that it had device metadata that needed to be obtained by the UA. The FD can
+/// [GetDeviceMetaDataRequest] command. This command shall only be used if the FD indicated in the
+/// RequestUpdate response that it had device metadata that needed to be obtained by the UA. The FD can
 /// send this command when it is in any state, except the IDLE and LEARN COMPONENTS state.
 #[derive(Debug, Clone, FromBytes, IntoBytes, Immutable, PartialEq)]
 #[repr(C, packed)]
@@ -403,9 +396,10 @@ pub struct GetMetaDataResponse<'a> {
 
     /// 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 between the values of the firmware update
-    /// baseline transfer size and MaximumTransferSize from the [RequestUpdate] or
-    /// [RequestDownstreamDeviceUpdate] command. When TransferFlag = End or StartAndEnd, the
-    /// variable size of this field can also be less than the firmware update baseline transfer size.
+    /// baseline transfer size and MaximumTransferSize from the RequestUpdate or
+    /// [crate::message::firmware_update::query_downstream::RequestDownstreamDeviceUpdateRequest] command.
+    ///  When TransferFlag = End or StartAndEnd, the variable size of this field can also be less than
+    /// the firmware update baseline transfer size.
     pub portion_of_device_metadata: &'a [u8],
 }
 
@@ -457,7 +451,7 @@ impl<'a> PldmCodecWithLifetime<'a> for GetMetaDataResponse<'a> {
         buffer[offset..offset + self.portion_of_device_metadata.len()]
             .copy_from_slice(self.portion_of_device_metadata);
 
-        Ok(size)
+        Ok(offset + self.portion_of_device_metadata.len())
     }
 
     fn decode(buffer: &'a [u8]) -> Result<Self, PldmCodecError> {

src/message/firmware_update/get_package_data.rs

@@ -282,12 +282,13 @@ impl<'a> Iterator for QueryDownstreamIdentifiersResponse<'a> {
 
 impl<'a> PldmCodecWithLifetime<'a> for QueryDownstreamIdentifiersResponse<'a> {
     fn encode(&self, buffer: &mut [u8]) -> Result<usize, crate::codec::PldmCodecError> {
-        let max_size = size_of::<PldmMsgHeader<[u8; PLDM_MSG_HEADER_LEN]>>()
+        let size = size_of::<PldmMsgHeader<[u8; PLDM_MSG_HEADER_LEN]>>()
             + size_of::<u8>()
             + size_of::<u32>()
             + size_of::<u8>()
             + self.portion.len();
-        if buffer.len() < max_size {
+
+        if buffer.len() < size {
             return Err(crate::codec::PldmCodecError::BufferTooShort);
         }
 
@@ -307,42 +308,41 @@ impl<'a> PldmCodecWithLifetime<'a> for QueryDownstreamIdentifiersResponse<'a> {
 
         offset += size_of::<u8>();
         buffer[offset..offset + self.portion.len()].copy_from_slice(self.portion);
-        offset += self.portion.len();
 
-        Ok(offset)
+        Ok(offset + self.portion.len())
     }
 
-    fn decode(_buffer: &'a [u8]) -> Result<Self, crate::codec::PldmCodecError> {
+    fn decode(buffer: &'a [u8]) -> Result<Self, crate::codec::PldmCodecError> {
         let min_size = size_of::<PldmMsgHeader<[u8; PLDM_MSG_HEADER_LEN]>>()
             + size_of::<u8>()
             + size_of::<u32>()
             + size_of::<u8>();
 
-        if _buffer.len() < min_size {
+        if buffer.len() < min_size {
             return Err(crate::codec::PldmCodecError::BufferTooShort);
         }
 
         let mut offset = 0;
         let hdr = PldmMsgHeader::<[u8; PLDM_MSG_HEADER_LEN]>::read_from_bytes(
-            &_buffer[offset..offset + PLDM_MSG_HEADER_LEN],
+            &buffer[offset..offset + PLDM_MSG_HEADER_LEN],
         )
         .map_err(|_| crate::codec::PldmCodecError::BufferTooShort)?;
         offset += PLDM_MSG_HEADER_LEN;
 
-        let completion_code = _buffer[offset];
+        let completion_code = buffer[offset];
         offset += size_of::<u8>();
 
         let next_data_transfer_handle = u32::from_le_bytes(
-            _buffer[offset..offset + size_of::<u32>()]
+            buffer[offset..offset + size_of::<u32>()]
                 .try_into()
                 .map_err(|_| crate::codec::PldmCodecError::BufferTooShort)?,
         );
         offset += size_of::<u32>();
 
-        let transfer_flag = _buffer[offset];
+        let transfer_flag = buffer[offset];
         offset += size_of::<u8>();
 
-        let portion = &_buffer[offset..];
+        let portion = &buffer[offset..];
 
         Ok(QueryDownstreamIdentifiersResponse {
             hdr,
@@ -438,7 +438,6 @@ impl Iterator for DownstreamDevice<'_> {
         self._iter_offset += descriptor_size;
         self._iter_dev_count += 1;
 
-        // println!("{descriptor:?}");
         Some(descriptor)
     }
 }
@@ -885,18 +884,22 @@ impl PldmCodec for DownstreamDeviceParameterTable {
         let capabilities_during_update = CapabilitiesDuringUpdate::decode(&buffer[offset..])?;
         offset += size_of::<CapabilitiesDuringUpdate>();
 
-        let mut active_component_version_string = PldmFirmwareString::default();
-        active_component_version_string.str_type = active_component_version_string_type;
-        active_component_version_string.str_len = active_component_version_string_length;
+        let mut active_component_version_string = PldmFirmwareString {
+            str_type: active_component_version_string_type,
+            str_len: active_component_version_string_length,
+            str_data: [0u8; 32],
+        };
         active_component_version_string.str_data[..active_component_version_string_length as usize]
             .copy_from_slice(
                 &buffer[offset..offset + active_component_version_string_length as usize],
             );
         offset += active_component_version_string_length as usize;
 
-        let mut pending_component_version_string = PldmFirmwareString::default();
-        pending_component_version_string.str_type = pending_component_version_string_type;
-        pending_component_version_string.str_len = pending_component_version_string_length;
+        let mut pending_component_version_string = PldmFirmwareString {
+            str_type: pending_component_version_string_type,
+            str_len: pending_component_version_string_length,
+            str_data: [0u8; 32],
+        };
         pending_component_version_string.str_data
             [..pending_component_version_string_length as usize]
             .copy_from_slice(
@@ -948,9 +951,6 @@ impl PldmCodec for GetDownstreamFirmwareParametersResponse {
             + size_of::<u8>()
             + self.portion.size();
 
-        println!("DownstreamDeviceParameterTable encode size: {}", size);
-        println!("Buffer length: {}", buffer.len());
-
         if buffer.len() < size {
             return Err(crate::codec::PldmCodecError::BufferTooShort);
         }
@@ -972,9 +972,8 @@ impl PldmCodec for GetDownstreamFirmwareParametersResponse {
         offset += size_of::<u8>();
 
         let bytes_written = self.portion.encode(&mut buffer[offset..])?;
-        offset += bytes_written;
 
-        Ok(offset)
+        Ok(offset + bytes_written)
     }
 
     fn decode(buffer: &[u8]) -> Result<Self, crate::codec::PldmCodecError> {
@@ -1257,8 +1256,6 @@ mod tests {
                 + size_of::<ComponentActivationMethods>()
                 + size_of::<CapabilitiesDuringUpdate>();
 
-        dbg!(STR_DATA_OFFSET);
-
         let mut buffer = [0u8; STR_DATA_OFFSET + 4 + 4];
         let bytes_written = table.encode(&mut buffer).unwrap();
 
@@ -1292,8 +1289,6 @@ mod tests {
         )
         .unwrap();
 
-        dbg!(size_of_val(&downstream_device_parameter_table));
-
         let portion = GetDownstreamFirmwareParametersPortion {
             get_downstream_firmware_parameters_capability: FirmwareDeviceCapability(0u32),
             downstream_device_count: 1,
@@ -1310,7 +1305,6 @@ mod tests {
 
     #[test]
     fn test_get_downstream_firmware_parameters_response_codec() {
-        // todo!();
         let instance_id: InstanceId = 0x01;
         let downstream_device_index = DownstreamDeviceIndex::try_from(1).unwrap();
         let cap: FirmwareDeviceCapability = FirmwareDeviceCapability(0u32);

src/message/firmware_update/query_downstream.rs

@@ -1,6 +1,5 @@
 // Licensed under the Apache-2.0 license
-
-use crate::codec::{PldmCodec, PldmCodecError, PldmCodecWithLifetime};
+use crate::codec::{PldmCodecError, PldmCodecWithLifetime};
 use crate::protocol::base::{
     InstanceId, PldmMsgHeader, PldmMsgType, PldmSupportedType, PLDM_MSG_HEADER_LEN,
 };
@@ -106,15 +105,19 @@ impl<'a> PldmCodecWithLifetime<'a> for RequestFirmwareDataResponse<'a> {
             return Err(PldmCodecError::BufferTooShort);
         }
 
-        let (fixed, data) =
+        let (fixed, _) =
             RequestFirmwareDataResponseFixed::read_from_prefix(&buffer[..size]).unwrap();
-        Ok(RequestFirmwareDataResponse { fixed, data })
+        Ok(RequestFirmwareDataResponse {
+            fixed,
+            data: &buffer[size..],
+        })
     }
 }
 
 #[cfg(test)]
 mod test {
     use super::*;
+    use crate::codec::PldmCodec;
 
     #[test]
     fn test_request_firmware_data_request_codec() {

src/message/firmware_update/request_fw_data.rs

@@ -259,7 +259,7 @@ impl PldmFailureResponse {
 /// Macro to define PLDM completion code enum with base codes and custom codes.
 /// By default, the entire [PldmBaseCompletionCode] is included as a
 /// variant named `BaseCodes`. The additional custom completion codes can be
-/// specified as variants, e.g. [FwUpdateCompletionCode].
+/// specified as variants of [FwUpdateCompletionCode].
 ///
 /// This macro is useful for the completion codes for the command responses, to ensure
 /// type safety while still allowing the use of base completion codes and an

src/protocol/base.rs

@@ -333,65 +333,6 @@ impl Descriptor {
     }
 }
 
-// impl PldmCodec for Descriptor {
-//     fn encode(&self, buffer: &mut [u8]) -> Result<usize, PldmCodecError> {
-//         if buffer.len() < self.codec_size_in_bytes() {
-//             return Err(PldmCodecError::BufferTooShort);
-//         }
-//         let mut offset = 0;
-
-//         self.descriptor_type
-//             .write_to(&mut buffer[offset..offset + core::mem::size_of::<u16>()])
-//             .unwrap();
-//         offset += core::mem::size_of::<u16>();
-
-//         self.descriptor_length
-//             .write_to(&mut buffer[offset..offset + core::mem::size_of::<u16>()])
-//             .unwrap();
-//         offset += core::mem::size_of::<u16>();
-
-//         self.descriptor_data[..self.descriptor_length as usize]
-//             .write_to(&mut buffer[offset..offset + self.descriptor_length as usize])
-//             .unwrap();
-//         offset += self.descriptor_length as usize;
-
-//         Ok(offset)
-//     }
-
-//     fn decode(buffer: &[u8]) -> Result<Self, PldmCodecError> {
-//         let mut offset = 0;
-
-//         let descriptor_type = u16::read_from_bytes(
-//             buffer
-//                 .get(offset..offset + core::mem::size_of::<u16>())
-//                 .ok_or(PldmCodecError::BufferTooShort)?,
-//         )
-//         .unwrap();
-//         offset += core::mem::size_of::<u16>();
-
-//         let descriptor_length = u16::read_from_bytes(
-//             buffer
-//                 .get(offset..offset + core::mem::size_of::<u16>())
-//                 .ok_or(PldmCodecError::BufferTooShort)?,
-//         )
-//         .unwrap();
-//         offset += core::mem::size_of::<u16>();
-
-//         let mut descriptor_data = [0u8; DESCRIPTOR_DATA_MAX_LEN];
-//         descriptor_data[..descriptor_length as usize].copy_from_slice(
-//             buffer
-//                 .get(offset..offset + descriptor_length as usize)
-//                 .ok_or(PldmCodecError::BufferTooShort)?,
-//         );
-
-//         Ok(Descriptor {
-//             descriptor_type,
-//             descriptor_length,
-//             descriptor_data,
-//         })
-//     }
-// }
-
 bitfield! {
     /// FDPCapabilitiesDuringUpdate
     ///
@@ -541,9 +482,8 @@ impl PldmCodec for PldmFirmwareString {
         self.str_data[..self.str_len as usize]
             .write_to(&mut buffer[offset..offset + self.str_len as usize])
             .unwrap();
-        offset += self.str_len as usize;
 
-        Ok(offset)
+        Ok(offset + self.str_len as usize)
     }
 
     fn decode(buffer: &[u8]) -> Result<Self, PldmCodecError> {