Scatter-shot improvements.

Things I noticed when reading over the code.

- Conversions to more idiomatic code.
- Spelling fixes.
- Minor comment improvements.
- Some minor formatting changes.

Signed-off-by: Nicholas Nethercote <[email protected]>

Author: nnethercote

mctp-estack/src/fragment.rs

@@ -87,7 +87,8 @@ impl Fragmenter {
     /// Returns fragments for the MCTP payload
     ///
     /// The same input message `payload` should be passed to each `fragment()` call.
-    /// In `SendOutput::Packet(buf)`, `out` is borrowed as the returned fragment, filled with packet contents.
+    /// In `SendOutput::Packet(buf)`, `out` is borrowed as the returned fragment, filled with
+    /// packet contents.
     ///
     /// `out` must be at least as large as the specified `mtu`.
     pub fn fragment<'f>(

mctp-estack/src/lib.rs

@@ -23,7 +23,7 @@
 //! ## Configuration
 //!
 //! `mctp-estack` uses fixed sizes to be suitable on no-alloc platforms.
-//! These can be configured at build time, see [`config`]
+//! These can be configured at build time, see [`config`].
 
 #![cfg_attr(not(feature = "std"), no_std)]
 // Temporarily relaxed for zerocopy_channel vendored code.
@@ -176,9 +176,9 @@ pub struct Stack {
     // Unused reassemblers set Reassembler::is_unused()
     reassemblers: [(Reassembler, Vec<u8, MAX_PAYLOAD>); NUM_RECEIVE],
 
-    /// monotonic time and counter.
+    /// Monotonic time and counter.
     now: EventStamp,
-    /// cached next expiry time from update()
+    /// Cached next expiry time from update().
     next_timeout: u64,
 
     // Arbitrary counter to make tag allocation more variable.
@@ -668,8 +668,8 @@ pub struct MctpMessage<'a> {
     pub ic: MsgIC,
     pub payload: &'a [u8],
 
-    /// Cookie is set for response messages when the request had `cookie` set in the [`Stack::start_send`] call.
-    /// "Response" message refers having `TO` bit unset.
+    /// Cookie is set for response messages when the request had `cookie` set in the
+    /// [`Stack::start_send`] call. "Response" message refers having `TO` bit unset.
     reassembler: &'a mut Reassembler,
 
     // By default when a MctpMessage is dropped the reassembler will be
@@ -773,7 +773,6 @@ pub(crate) mod fmt {
 
 #[cfg(test)]
 mod tests {
-
     // TODO:
     // back to back fragmenter/reassembler
 

mctp-estack/src/serial.rs

@@ -169,12 +169,8 @@ impl MctpSerialHandler {
                         // Complete frame
                         let packet = &self.rxbuf[2..][..self.rxcount];
                         return Some(packet);
-                    } else {
-                        warn!(
-                            "Bad checksum got {:04x} calc {:04x}",
-                            cs, cs_calc
-                        );
                     }
+                    warn!("Bad checksum got {:04x} calc {:04x}", cs, cs_calc);
                 } else {
                     // restart
                     self.rxpos = Pos::SerialRevision;

mctp-usb-embassy/src/mctpusb.rs

@@ -146,9 +146,7 @@ impl<'d, D: Driver<'d>> Receiver<'d, D> {
             // Refill
             let l = match self.ep.read(&mut self.buf).await {
                 Ok(l) => l,
-                Err(_e) => {
-                    return None;
-                }
+                Err(_e) => return None,
             };
             self.remaining = Range { start: 0, end: l };
         }
@@ -158,9 +156,7 @@ impl<'d, D: Driver<'d>> Receiver<'d, D> {
         let rem = &self.buf[self.remaining.clone()];
         let (pkt, rem) = match MctpUsbHandler::decode(rem) {
             Ok(a) => a,
-            Err(e) => {
-                return Some(Err(e));
-            }
+            Err(e) => return Some(Err(e)),
         };
         self.remaining.start = self.remaining.end - rem.len();
         Some(Ok(pkt))

pldm-file/src/client.rs

@@ -191,7 +191,7 @@ where
         let resp_data_len = read_resp.len as usize;
 
         if rest.len() != resp_data_len + 4 {
-            return Err(proto_error!("invalid resonse data length"));
+            return Err(proto_error!("invalid response data length"));
         }
 
         let (resp_data, resp_cs) = rest.split_at(resp_data_len);

pldm-file/src/host.rs

@@ -106,7 +106,7 @@ impl<const N: usize> Responder<N> {
         &mut self,
         mut comm: R,
         req: &PldmRequest<'_>,
-        host: &mut impl Host,
+        _host: &mut impl Host,
     ) -> pldm::Result<()> {
         if req.typ != PLDM_TYPE_FILE_TRANSFER {
             trace!("pldm-fw non-pldm-fw request {req:?}");
@@ -125,9 +125,9 @@ impl<const N: usize> Responder<N> {
         };
 
         let r = match cmd {
-            Cmd::DfProperties => self.cmd_dfproperties(req, host),
-            Cmd::DfOpen => self.cmd_dfopen(req, host),
-            Cmd::DfClose => self.cmd_dfclose(req, host),
+            Cmd::DfProperties => self.cmd_dfproperties(req),
+            Cmd::DfOpen => self.cmd_dfopen(req),
+            Cmd::DfClose => self.cmd_dfclose(req),
             _ => {
                 trace!("unhandled command {cmd:?}");
                 Err(CCode::ERROR_UNSUPPORTED_PLDM_CMD.into())
@@ -207,7 +207,6 @@ impl<const N: usize> Responder<N> {
     fn cmd_dfproperties<'a>(
         &mut self,
         req: &'a PldmRequest<'a>,
-        _host: &mut impl Host,
     ) -> Result<PldmResponse<'a>> {
         let (rest, dfp) = DfPropertiesReq::from_bytes((&req.data, 0))?;
 
@@ -234,7 +233,6 @@ impl<const N: usize> Responder<N> {
     fn cmd_dfopen<'a>(
         &mut self,
         req: &'a PldmRequest<'a>,
-        _host: &mut impl Host,
     ) -> Result<PldmResponse<'a>> {
         let (rest, dfo) = DfOpenReq::from_bytes((&req.data, 0))?;
 
@@ -254,8 +252,7 @@ impl<const N: usize> Responder<N> {
         let id = self
             .files
             .iter()
-            .enumerate()
-            .find_map(|(n, e)| if e.is_none() { Some(n) } else { None })
+            .position(|e| e.is_none())
             .ok_or(file_ccode::MAX_NUM_FDS_EXCEEDED)?;
 
         self.files[id].replace(file_ctx);
@@ -273,7 +270,6 @@ impl<const N: usize> Responder<N> {
     fn cmd_dfclose<'a>(
         &mut self,
         req: &'a PldmRequest<'a>,
-        _host: &mut impl Host,
     ) -> Result<PldmResponse<'a>> {
         let (rest, dfc) = DfCloseReq::from_bytes((&req.data, 0))?;
 

pldm-fw-cli/src/bin/pldm-fw.rs

@@ -9,17 +9,13 @@ use anyhow::{bail, Context};
 use argh::FromArgs;
 use enumset::{EnumSet, EnumSetType};
 use mctp_linux::{MctpAddr, MctpLinuxListener};
-use std::fmt::Write as _;
 use std::io::Write;
 
 fn comma_separated<T: EnumSetType + std::fmt::Debug>(e: EnumSet<T>) -> String {
-    let mut s = String::new();
-    let mut first = true;
-    for i in e.iter() {
-        write!(s, "{}{:?}", if first { "" } else { "," }, i).unwrap();
-        first = false;
-    }
-    s
+    e.iter()
+        .map(|t| format!("{t:?}"))
+        .collect::<Vec<_>>()
+        .join(",")
 }
 
 fn print_device_info(
@@ -112,10 +108,9 @@ fn extract_component(
     pkg: &pldm_fw::pkg::Package,
     idx: usize,
 ) -> anyhow::Result<()> {
-    if idx >= pkg.components.len() {
-        bail!("no component with index {}", idx);
-    }
-    let comp = &pkg.components[idx];
+    let Some(comp) = pkg.components.get(idx) else {
+        bail!("no component with index {}", idx)
+    };
 
     let fname = format!("component-{}.{:04x}.bin", idx, comp.identifier);
     let mut f = std::fs::File::create(&fname)

pldm-fw/src/fd.rs

@@ -167,17 +167,21 @@ impl<R: RespChannel> Responder<R> {
         // Check for consistent EID
         match cmd {
             // informational commands or Cancel always allowed
-            | Cmd::QueryDeviceIdentifiers
+            Cmd::QueryDeviceIdentifiers
             | Cmd::GetFirmwareParameters
             | Cmd::GetStatus
             | Cmd::CancelUpdate
             // RequestUpdate will check itself
-            | Cmd::RequestUpdate
-            => (),
+            | Cmd::RequestUpdate => (),
             _ => {
                 if self.ua_eid != Some(eid) {
-                    debug!("Ignoring {cmd:?} from mismatching EID {eid}, expected {:?}", self.ua_eid);
-                    self.reply_error(req, &mut comm,
+                    debug!(
+                        "Ignoring {cmd:?} from mismatching EID {eid}, expected {:?}",
+                        self.ua_eid
+                    );
+                    self.reply_error(
+                        req,
+                        &mut comm,
                         CCode::ERROR_NOT_READY as u8,
                     );
                 }
@@ -243,7 +247,7 @@ impl<R: RespChannel> Responder<R> {
                 req_comm,
                 req_pending,
                 ..
-            } => req_pending.then(|| req_comm),
+            } => req_pending.then_some(req_comm),
             _ => None,
         };
         trace!("pending reply {}", r.is_some());
@@ -268,14 +272,15 @@ impl<R: RespChannel> Responder<R> {
 
         match Cmd::from_u8(rsp.cmd) {
             Some(Cmd::RequestFirmwareData) => self.download_response(rsp, d),
-            | Some(Cmd::TransferComplete)
-            | Some(Cmd::VerifyComplete)
-            | Some(Cmd::ApplyComplete)
+
             // Ignore replies to these requests.
             // We may have already moved on to a later state
             // and don't have any useful retry for them.
-            => Ok(()),
-            _ => Err(proto_error!("Unsupported PLDM response"))
+            Some(Cmd::TransferComplete)
+            | Some(Cmd::VerifyComplete)
+            | Some(Cmd::ApplyComplete) => Ok(()),
+
+            _ => Err(proto_error!("Unsupported PLDM response")),
         }
     }
 

pldm-fw/src/lib.rs

@@ -1342,7 +1342,6 @@ pub struct UpdateTransferProgress {
 
 #[cfg(test)]
 mod tests {
-
     use crate::*;
 
     #[test]

pldm-fw/src/pkg.rs

@@ -70,25 +70,15 @@ impl<'a> ComponentBitmap {
     }
 
     pub fn as_index_str(&self) -> String {
-        let mut s = String::new();
-        let mut first = true;
-        for i in 0usize..self.n_bits {
-            if self.bit(i) {
-                s.push_str(&format!("{}{}", if first { "" } else { ", " }, i));
-                first = false;
-            }
-        }
-        s
+        (0usize..self.n_bits)
+            .filter(|&i| self.bit(i))
+            .map(|i| i.to_string())
+            .collect::<Vec<_>>()
+            .join(", ")
     }
 
     pub fn as_index_vec(&self) -> Vec<usize> {
-        let mut v = Vec::new();
-        for i in 0usize..self.n_bits {
-            if self.bit(i) {
-                v.push(i)
-            }
-        }
-        v
+        (0usize..self.n_bits).filter(|&i| self.bit(i)).collect()
     }
 }
 
@@ -232,7 +222,7 @@ impl Package {
             .finish()
             .map_err(|_| PldmPackageError::new_format("can't parse devices"))?;
 
-        /* this is the first divegence in package format versions; the
+        /* this is the first divergence in package format versions; the
          * downstream device identification area is only present in 1.1.x
          */
         let r = match identifier {