Fix clippy warnings

lzrd · lzrd · commit 1d61739a41cd · 2025-12-09T12:21:05.000-08:00
Mostly mechanical fixes: removing needless borrows, adding lifetime
annotations, using modern numeric constants (i32::MAX), and suppressing
warnings for dropshot-generated dead code.

Notable equivalences:
- .as_bytes().len() to .len() on String: both return byte count
- .trim().split_whitespace() to .split_whitespace(): split_whitespace
  already skips leading/trailing whitespace
- .into_iter() to .iter() on slices: both yield &amp;T
diff --git a/agent/src/control/mod.rs b/agent/src/control/mod.rs
@@ -178,13 +178,13 @@ async fn cmd_address_list(mut l: Level<Stuff>) -> Result<()> {
                 let mut r = Row::default();
 
                 r.add_str("name", &addr.name);
-                r.add_str("cidr", &net.to_string());
-                r.add_str("first", &first.to_string());
-                r.add_str("last", &last.to_string());
+                r.add_str("cidr", net.to_string());
+                r.add_str("first", first.to_string());
+                r.add_str("last", last.to_string());
                 r.add_u64("count", addr.count.into());
                 r.add_str("family", "inet");
-                r.add_str("network", &net.network().to_string());
-                r.add_str("mask", &net.netmask().to_string());
+                r.add_str("network", net.network().to_string());
+                r.add_str("mask", net.netmask().to_string());
                 r.add_str("routed", if addr.routed { "yes" } else { "no" });
                 r.add_str("gateway", addr.gateway.as_deref().unwrap_or("-"));
 
diff --git a/agent/src/exec.rs b/agent/src/exec.rs
@@ -23,10 +23,7 @@ fn spawn_reader<T>(
 where
     T: Read + Send + 'static,
 {
-    let stream = match stream {
-        Some(stream) => stream,
-        None => return None,
-    };
+    let stream = stream?;
 
     Some(std::thread::spawn(move || {
         let mut r = BufReader::new(stream);
@@ -297,7 +294,7 @@ fn run_common(
                  * process.
                  */
                 if ab.bgproc.is_none() {
-                    tx.blocking_send(ab.exit(&start, &end, std::i32::MAX))
+                    tx.blocking_send(ab.exit(&start, &end, i32::MAX))
                         .unwrap();
                 }
 
@@ -332,7 +329,7 @@ fn run_common(
                 let code = if let Some(code) = es.code() {
                     code
                 } else {
-                    std::i32::MAX
+                    i32::MAX
                 };
                 tx.blocking_send(ab.exit(&start, &end, code)).unwrap();
                 stdio_warning
diff --git a/agent/src/main.rs b/agent/src/main.rs
@@ -1093,7 +1093,7 @@ async fn cmd_install(mut l: Level<Agent>) -> Result<()> {
         let status = Command::new("/sbin/zfs")
             .arg("create")
             .arg("-o")
-            .arg(&format!("mountpoint={}", INPUT_PATH))
+            .arg(format!("mountpoint={}", INPUT_PATH))
             .arg(INPUT_DATASET)
             .env_clear()
             .current_dir("/")
diff --git a/bin/src/main.rs b/bin/src/main.rs
@@ -26,11 +26,11 @@ mod config;
 
 trait FlagsExt {
     #[must_use]
-    fn add_flags(&mut self, name: &'static str) -> Flags;
+    fn add_flags(&mut self, name: &'static str) -> Flags<'_>;
 }
 
 impl FlagsExt for Row {
-    fn add_flags(&mut self, name: &'static str) -> Flags {
+    fn add_flags(&mut self, name: &'static str) -> Flags<'_> {
         Flags { row: self, name, out: String::new() }
     }
 }
@@ -1226,7 +1226,7 @@ async fn do_job_store_list(mut l: Level<Stuff>) -> Result<()> {
         );
         r.add_str(
             "updated",
-            &ent.time_update.to_rfc3339_opts(chrono::SecondsFormat::Secs, true),
+            ent.time_update.to_rfc3339_opts(chrono::SecondsFormat::Secs, true),
         );
 
         t.add_row(r);
@@ -1351,7 +1351,7 @@ async fn do_user_list(mut l: Level<Stuff>) -> Result<()> {
         r.add_str("name", &u.name);
         r.add_str(
             "creation",
-            &u.time_create.to_rfc3339_opts(chrono::SecondsFormat::Secs, true),
+            u.time_create.to_rfc3339_opts(chrono::SecondsFormat::Secs, true),
         );
         r.add_age("age", u.time_create.age());
         t.add_row(r);
diff --git a/download/src/kinds/url.rs b/download/src/kinds/url.rs
@@ -27,6 +27,7 @@ const BUF_COUNT: usize = 10;
  * assessed against the actual file size and a partial (206) response will be
  * returned, for both GET and HEAD.
  */
+#[allow(clippy::too_many_arguments)]
 pub async fn stream_from_url(
     log: &Logger,
     info: String,
diff --git a/download/src/lib.rs b/download/src/lib.rs
@@ -43,7 +43,7 @@ fn make_get_response<E>(
     rx: mpsc::Receiver<std::result::Result<Frame<Bytes>, E>>,
 ) -> Result<Response<Body>>
 where
-    E: Into<Box<(dyn std::error::Error + Send + Sync + 'static)>>
+    E: Into<Box<dyn std::error::Error + Send + Sync + 'static>>
         + Send
         + Sync
         + 'static,
diff --git a/factory/aws/src/aws.rs b/factory/aws/src/aws.rs
@@ -70,7 +70,7 @@ impl Instance {
                     .try_into()
                     .unwrap();
 
-                (if when <= now { now - when } else { 0 }) / 1000
+                now.saturating_sub(when) / 1000
             })
             .unwrap_or(0)
     }
@@ -188,7 +188,7 @@ async fn create_instance(
 
     let mut instances = res
         .instances()
-        .into_iter()
+        .iter()
         .map(|i| Instance::from((i, config.aws.tag.as_str())))
         .collect::<Vec<_>>();
 
@@ -218,13 +218,12 @@ async fn instances(
     Ok(res
         .reservations()
         .iter()
-        .map(|r| {
+        .flat_map(|r| {
             r.instances().iter().map(|i| {
                 let i = Instance::from((i, tag));
                 (i.id.to_string(), i)
             })
         })
-        .flatten()
         .collect())
 }
 
@@ -421,7 +420,7 @@ async fn aws_worker_one(
              * There is a record of a particular instance ID for this worker.
              * Check to see if that instance exists.
              */
-            if let Some(i) = insts.get(&instance_id.to_string()) {
+            if let Some(i) = insts.get(instance_id) {
                 if i.state == "terminated" {
                     /*
                      * The instance exists, but is terminated.  Delete the
diff --git a/factory/aws/src/main.rs b/factory/aws/src/main.rs
@@ -72,7 +72,7 @@ impl Central {
          * Allow the per-target diagnostic configuration to override the base
          * diagnostic configuration.
          */
-        Ok(self.config.diag.apply_overrides(&t.diag).build()?)
+        self.config.diag.apply_overrides(&t.diag).build()
     }
 }
 
diff --git a/factory/gimlet/src/cleanup.rs b/factory/gimlet/src/cleanup.rs
@@ -110,7 +110,7 @@ pub fn setup() -> Result<()> {
             bail!("giving up after {MAX_TIME} seconds");
         }
 
-        if let Ok(st) = svcs(&fmri) {
+        if let Ok(st) = svcs(fmri) {
             if st.next.is_none() && st.current == "ON" {
                 println!(" * {fmri} now online!");
                 break;
@@ -125,7 +125,7 @@ pub fn setup() -> Result<()> {
      * only be one!
      */
     let pools = zpool_unimported_list()?;
-    if pools.len() == 0 {
+    if pools.is_empty() {
         bail!("no unimported pool found!");
     } else if pools.len() > 1 {
         bail!("more than one unimported pool found!");
@@ -137,14 +137,14 @@ pub fn setup() -> Result<()> {
     };
 
     println!(" * importing pool {pool:?}...");
-    zpool_import(&pool)?;
+    zpool_import(pool)?;
 
     /*
      * Update the BSU symlink:
      */
     std::fs::create_dir_all("/pool/bsu")?;
     std::fs::remove_file("/pool/bsu/0").ok();
-    std::os::unix::fs::symlink(&format!("../int/{pool_id}"), "/pool/bsu/0")?;
+    std::os::unix::fs::symlink(format!("../int/{pool_id}"), "/pool/bsu/0")?;
 
     /*
      * Create the swap device:
@@ -252,14 +252,14 @@ fn prepare_m2() -> Result<()> {
         let p = &mut vtoc.parts_mut()[i];
         p.p_tag = efi::sys::V_USR;
         p.p_start = next_start;
-        p.p_size = size.into();
+        p.p_size = size;
         next_start = next_start.checked_add(size).unwrap();
     }
 
     /*
      * Create the dump slice:
      */
-    let size = (1 * 1024 * 1024 * 1024 * 1024) / vtoc.block_size();
+    let size = (1024 * 1024 * 1024 * 1024) / vtoc.block_size();
     let p = &mut vtoc.parts_mut()[4];
     p.p_tag = efi::sys::V_USR;
     p.p_start = next_start;
@@ -287,11 +287,11 @@ fn prepare_m2() -> Result<()> {
         .env_clear()
         .arg("create")
         .arg("-O")
-        .arg(&format!("mountpoint=/pool/int/{id}"))
+        .arg(format!("mountpoint=/pool/int/{id}"))
         .arg("-O")
         .arg("compress=on")
         .arg(&pool)
-        .arg(&format!("{}s5", d.name))
+        .arg(format!("{}s5", d.name))
         .output()?;
 
     if !out.status.success() {
diff --git a/factory/gimlet/src/disks.rs b/factory/gimlet/src/disks.rs
@@ -121,7 +121,7 @@ pub fn list_disks() -> Result<Disks> {
                            slice: u32|
      -> Result<Option<devinfo::DevLink>> {
         let mut paths = links
-            .links_for_path(&device)?
+            .links_for_path(device)?
             .into_iter()
             .filter(|l| {
                 l.linktype() == devinfo::DevLinkType::Primary
@@ -193,18 +193,18 @@ pub fn list_disks() -> Result<Disks> {
                  */
                 let device = m.devfs_path()?;
 
-                Ok((0..=8)
+                (0..=8)
                     .map(|num| {
                         Ok(slice_from_path(&device, num)?.map(|link| {
                             (num, m.spec_type(), link.path().to_owned())
                         }))
                     })
-                    .collect::<Result<Vec<_>>>()?)
+                    .collect::<Result<Vec<_>>>()
             })
             .collect::<Result<Vec<_>>>()?
             .into_iter()
             .flatten()
-            .filter_map(|v| v)
+            .flatten()
             .collect::<Vec<_>>();
 
         /*
diff --git a/factory/gimlet/src/host.rs b/factory/gimlet/src/host.rs
@@ -624,7 +624,7 @@ impl HostManager {
                 Ok(())
             }
             HostState::Cleaning(cst) => {
-                let mut new_cst = cst.clone();
+                let mut new_cst = *cst;
 
                 {
                     let mut l = self.locked.lock().unwrap();
@@ -700,7 +700,7 @@ impl HostManager {
                 Ok(())
             }
             HostState::Starting(sst, hgs) => {
-                let mut new_sst = sst.clone();
+                let mut new_sst = *sst;
 
                 {
                     let mut l = self.locked.lock().unwrap();
@@ -726,7 +726,7 @@ impl HostManager {
                     }
                 }
 
-                if let Err(e) = self.thread_starting(&mut new_sst, &hgs) {
+                if let Err(e) = self.thread_starting(&mut new_sst, hgs) {
                     error!(log, "starting error: {e}");
                 }
 
@@ -794,7 +794,7 @@ impl HostManager {
 
                 let pb = sys
                     .ssh_exec("svcs -Ho sta,nsta svc:/site/postboot:default")?;
-                let t = pb.out.trim().split_whitespace().collect::<Vec<_>>();
+                let t = pb.out.split_whitespace().collect::<Vec<_>>();
                 if t.len() != 2 || t[0] == "ON" || t[1] == "-" {
                     bail!("postboot not ready: {t:?}");
                 }
@@ -847,7 +847,7 @@ impl HostManager {
 
                 let pb = sys
                     .ssh_exec("svcs -Ho sta,nsta svc:/site/postboot:default")?;
-                let t = pb.out.trim().split_whitespace().collect::<Vec<_>>();
+                let t = pb.out.split_whitespace().collect::<Vec<_>>();
                 if t.len() != 2 || t[0] == "ON" || t[1] == "-" {
                     bail!("postboot not ready: {t:?}");
                 }
diff --git a/factory/gimlet/src/humility.rs b/factory/gimlet/src/humility.rs
@@ -17,6 +17,7 @@ use slog::{info, warn, Logger};
 use crate::pipe::*;
 use buildomat_common::OutputExt;
 
+#[allow(clippy::wrong_self_convention)]
 pub trait ValueExt {
     fn is_unit(&self) -> bool;
     fn from_hex(&self) -> Result<u64>;
@@ -57,7 +58,7 @@ impl ValueExt for Value {
                                 bail!("expected a hex number, got {t:?}");
                             };
 
-                            out.push(u8::from_str_radix(&hex, 16)?);
+                            out.push(u8::from_str_radix(hex, 16)?);
                         }
                         _ => bail!("list should only contain terms"),
                     }
diff --git a/factory/gimlet/src/instance.rs b/factory/gimlet/src/instance.rs
@@ -83,7 +83,7 @@ async fn instance_worker_start(
         }
         instances_with_workers.insert(id.clone());
 
-        let c = Arc::clone(&c);
+        let c = Arc::clone(c);
         let log = log.new(o!(
             "component" => "instance_worker",
             "instance" => id.to_string(),
diff --git a/factory/lab/src/main.rs b/factory/lab/src/main.rs
@@ -2,6 +2,8 @@
  * Copyright 2023 Oxide Computer Company
  */
 
+#![allow(dead_code)]
+
 use std::{
     collections::HashMap,
     sync::{Arc, Mutex},
diff --git a/factory/lab/src/minder.rs b/factory/lab/src/minder.rs
@@ -74,7 +74,7 @@ impl FormatScript {
             .replace("%BASEURL%", &self.host_config.lab_baseurl)
             .replace("%HOST%", &self.host_config.nodename)
             .replace("%CONSOLE%", &self.host_config.console)
-            .replace("%BOOTARGS%", &bootargs)
+            .replace("%BOOTARGS%", bootargs)
             .replace("%COREURL%", &self.coreurl);
 
         if let Some(key) = self.key.as_deref() {
@@ -149,7 +149,7 @@ impl CentralExt for Central {
         &self,
         name: &str,
     ) -> HResult<&super::config::ConfigFileHost> {
-        if let Some(h) = self.hosts.get(&name.to_string()) {
+        if let Some(h) = self.hosts.get(name) {
             Ok(&h.config)
         } else {
             Err(dropshot::HttpError::for_client_error(
diff --git a/factory/propolis/src/config.rs b/factory/propolis/src/config.rs
@@ -93,7 +93,7 @@ impl ConfigFile {
         &self,
         slot: u32,
         id: &InstanceId,
-    ) -> Result<InstanceInSlot> {
+    ) -> Result<InstanceInSlot<'_>> {
         Ok(InstanceInSlot { config: self, slot, id: id.clone() })
     }
 
diff --git a/factory/propolis/src/main.rs b/factory/propolis/src/main.rs
@@ -39,7 +39,7 @@ impl Central {
          * Allow the per-target diagnostic configuration to override the base
          * diagnostic configuration.
          */
-        Ok(self.config.diag.apply_overrides(&t.diag).build()?)
+        self.config.diag.apply_overrides(&t.diag).build()
     }
 }
 
diff --git a/factory/propolis/src/vm.rs b/factory/propolis/src/vm.rs
@@ -461,8 +461,8 @@ async fn instance_worker_one(
                     include_str!("../smf/serial.xml"),
                 ),
             ] {
-                std::fs::write(&vmdir.join(&format!("{name}.sh")), script)?;
-                std::fs::write(&smfdir.join(format!("{name}.xml")), bundle)?;
+                std::fs::write(vmdir.join(format!("{name}.sh")), script)?;
+                std::fs::write(smfdir.join(format!("{name}.xml")), bundle)?;
             }
 
             std::fs::write(&siteprofile, include_str!("../smf/site.xml"))?;
diff --git a/github/database/src/tables/check_suite.rs b/github/database/src/tables/check_suite.rs
@@ -61,7 +61,7 @@ impl From<jobfile::JobFile> for JobFile {
             content,
             dependencies: dependencies
                 .into_iter()
-                .map(|(a, b)| (a.into(), b.into()))
+                .map(|(a, b)| (a, b.into()))
                 .collect(),
         }
     }
diff --git a/github/dbtool/src/main.rs b/github/dbtool/src/main.rs
diff --git a/github/server/src/http.rs b/github/server/src/http.rs
diff --git a/github/server/src/main.rs b/github/server/src/main.rs
diff --git a/github/server/src/variety/basic.rs b/github/server/src/variety/basic.rs
diff --git a/jobsh/src/variety/basic.rs b/jobsh/src/variety/basic.rs
diff --git a/server/src/db/mod.rs b/server/src/db/mod.rs
diff --git a/server/src/main.rs b/server/src/main.rs
diff --git a/server/src/workers.rs b/server/src/workers.rs
diff --git a/sse/src/lib.rs b/sse/src/lib.rs

Original file line number	Diff line number	Diff line change
`@@ -72,7 +72,7 @@ impl Central {`
`72`	`72`	`* Allow the per-target diagnostic configuration to override the base`
`73`	`73`	`* diagnostic configuration.`
`74`	`74`	`*/`
`75`		`- Ok(self.config.diag.apply_overrides(&t.diag).build()?)`
	`75`	`+ self.config.diag.apply_overrides(&t.diag).build()`
`76`	`76`	`}`
`77`	`77`	`}`
`78`	`78`