Clippy Fixes

Also add clippy to the build process

.github/workflows/build-one.yml

@@ -40,7 +40,9 @@ jobs:
 
       # install rust toolchain
       - name: Install Rust toolchain
-        run: rustup show
+        run: |
+          rustup show
+          rustup component add clippy
 
       - name: Cache build output
         uses: Swatinem/rust-cache@v2
@@ -81,6 +83,11 @@ jobs:
               target/release/humility -a target/${{ inputs.app_name }}/dist/build-${{ inputs.app_name }}-image-$image.zip manifest; \
           done
 
+      - name: Clippy
+        if: inputs.os == 'ubuntu-latest'
+        run: |
+          cargo xtask clippy ${{ inputs.app_toml}} -- --deny warnings
+
       # upload the output of our build
       - name: Upload build archive
         uses: actions/upload-artifact@v3

app/oxcon2023g0/src/main.rs

@@ -34,6 +34,7 @@ fn main() -> ! {
     rcc.pllsyscfgr.write(|w| {
         unsafe {
             w.pllsrc().bits(0b10); // HSI16, I promise
+            #[allow(clippy::eq_op)]
             w.pllm().bits(1 - 1);
             w.plln().bits(8); // _not_ an n-1 field
             w.pllr().bits(2 - 1);

build/i2c/src/lib.rs

@@ -1008,6 +1008,7 @@ impl ConfigGenerator {
             &mut self.output,
             r##"
         #[allow(dead_code)]
+        #[allow(clippy::match_single_binding)]
         pub fn lookup_controller(index: usize) -> Option<Controller> {{
             match index {{"##
         )?;
@@ -1038,6 +1039,7 @@ impl ConfigGenerator {
             &mut self.output,
             r##"
         #[allow(dead_code)]
+        #[allow(clippy::match_single_binding)]
         pub fn lookup_port(index: usize) -> Option<PortIndex> {{
             match index {{"##
         )?;
@@ -1219,6 +1221,7 @@ impl ConfigGenerator {
         }}
 
         #[allow(unused_variables)]
+        #[allow(clippy::match_single_binding)]
         pub fn validate(
             task: TaskId,
             index: usize,

build/lpc55pins/src/lib.rs

@@ -119,7 +119,7 @@ impl ToTokens for PinConfig {
 pub fn codegen(pins: Vec<PinConfig>) -> Result<()> {
     let out_dir = build_util::out_dir();
     let dest_path = out_dir.join("pin_config.rs");
-    let mut file = std::fs::File::create(&dest_path)?;
+    let mut file = std::fs::File::create(dest_path)?;
 
     let mut buf = BufWriter::new(Vec::new());
     if pins.iter().any(|p| p.name.is_some()) {

build/xtask/src/clippy.rs

@@ -68,12 +68,15 @@ pub fn run(
                 .ok_or_else(|| anyhow::anyhow!("Failed to get image name"))?;
 
             // Pick dummy entry points for each task
-            let entry_points = allocs
+            let mut entry_points: std::collections::HashMap<_, _> = allocs
                 .tasks
                 .iter()
                 .map(|(k, v)| (k.clone(), v["flash"].start))
                 .collect();
 
+            // add a dummy caboose point
+            entry_points.insert("caboose".to_string(), 0x0);
+
             let kconfig = crate::dist::make_kconfig(
                 &toml,
                 &allocs.tasks,
@@ -82,11 +85,18 @@ pub fn run(
             )?;
             let kconfig = ron::ser::to_string(&kconfig)?;
 
+            let flash_outputs = if let Some(o) = toml.outputs.get("flash") {
+                ron::ser::to_string(o)?
+            } else {
+                bail!("no 'flash' output regions defined in config toml");
+            };
+
             toml.kernel_build_config(
                 verbose,
                 &[
                     ("HUBRIS_KCONFIG", &kconfig),
                     ("HUBRIS_IMAGE_ID", "1234"), // dummy image ID
+                    ("HUBRIS_FLASH_OUTPUTS", &flash_outputs),
                 ],
                 None,
             )

drv/fpga-devices/src/ecp5_spi.rs

@@ -62,23 +62,26 @@ impl<S: SpiServer> Ecp5Driver for Ecp5UsingSpi<S> {
     }
 
     fn set_program_n(&self, asserted: bool) -> Result<(), Self::Error> {
-        Ok(self.sys.gpio_set_to(self.program_n, asserted))
+        self.sys.gpio_set_to(self.program_n, asserted);
+        Ok(())
     }
 
     fn init_n(&self) -> Result<bool, Self::Error> {
         Ok(self.sys.gpio_read(self.init_n) != 0)
     }
 
     fn set_init_n(&self, asserted: bool) -> Result<(), Self::Error> {
-        Ok(self.sys.gpio_set_to(self.init_n, !asserted))
+        self.sys.gpio_set_to(self.init_n, !asserted);
+        Ok(())
     }
 
     fn done(&self) -> Result<bool, Self::Error> {
         Ok(self.sys.gpio_read(self.done) != 0)
     }
 
     fn set_done(&self, asserted: bool) -> Result<(), Self::Error> {
-        Ok(self.sys.gpio_set_to(self.done, asserted))
+        self.sys.gpio_set_to(self.done, asserted);
+        Ok(())
     }
 
     fn user_design_reset_n(&self) -> Result<bool, Self::Error> {
@@ -89,35 +92,41 @@ impl<S: SpiServer> Ecp5Driver for Ecp5UsingSpi<S> {
         &self,
         asserted: bool,
     ) -> Result<(), Self::Error> {
-        Ok(self.sys.gpio_set_to(self.user_design_reset_n, asserted))
+        self.sys.gpio_set_to(self.user_design_reset_n, asserted);
+        Ok(())
     }
 
     fn user_design_reset_duration(&self) -> u64 {
         self.user_design_reset_duration
     }
 
     fn configuration_read(&self, data: &mut [u8]) -> Result<(), Self::Error> {
-        Ok(self.configuration_port.read(data)?)
+        self.configuration_port.read(data)?;
+        Ok(())
     }
 
     fn configuration_write(&self, data: &[u8]) -> Result<(), Self::Error> {
-        Ok(self.configuration_port.write(data)?)
+        self.configuration_port.write(data)?;
+        Ok(())
     }
 
     fn configuration_write_command(
         &self,
         c: Command,
     ) -> Result<(), Self::Error> {
         let buffer: [u8; 4] = [c as u8, 0, 0, 0];
-        Ok(self.configuration_port.write(&buffer)?)
+        self.configuration_port.write(&buffer)?;
+        Ok(())
     }
 
     fn configuration_lock(&self) -> Result<(), Self::Error> {
-        Ok(self.configuration_port.lock(spi_api::CsState::Asserted)?)
+        self.configuration_port.lock(spi_api::CsState::Asserted)?;
+        Ok(())
     }
 
     fn configuration_release(&self) -> Result<(), Self::Error> {
-        Ok(self.configuration_port.release()?)
+        self.configuration_port.release()?;
+        Ok(())
     }
 }
 

drv/gimlet-hf-server/src/main.rs

@@ -400,7 +400,7 @@ impl ServerImpl {
         if addr as usize / SECTOR_SIZE_BYTES == 0
             && !matches!(protect, HfProtectMode::AllowModificationsToSector0)
         {
-            return Err(HfError::Sector0IsReserved.into());
+            return Err(HfError::Sector0IsReserved);
         }
         self.check_muxed_to_sp()?;
         self.set_and_check_write_enable()?;

drv/i2c-devices/src/ltc4282.rs

@@ -264,6 +264,7 @@ impl Validate<ResponseCode> for Ltc4282 {
         // naturally need to change.)  We deliberately do not depend on the
         // mode settings because these can be strapped to different values.
         //
+        #[allow(clippy::bool_comparison)]
         Ok(control.on_fault_mask() == true
             && control.on_delay() == false
             && control.on_enb() == true

drv/lpc55-spi/src/lib.rs

@@ -338,11 +338,11 @@ impl Spi {
     ///
     /// Mixing and matching different frame sizes is not recommended.
     pub fn read_u16(&mut self) -> u16 {
-        self.reg.fiford.read().rxdata().bits() as u16
+        self.reg.fiford.read().rxdata().bits()
     }
 
     pub fn read_u16_with_sot(&self) -> (u16, bool) {
         let reader = self.reg.fiford.read();
-        (reader.rxdata().bits() as u16, reader.sot().bit_is_set())
+        (reader.rxdata().bits(), reader.sot().bit_is_set())
     }
 }

drv/lpc55-sprot-server/src/handler.rs

@@ -111,7 +111,7 @@ impl Handler {
                 size: blob_size,
             }) => {
                 let blob_size: usize = blob_size.try_into().unwrap_lite();
-                if blob_size as usize > drv_sprot_api::MAX_BLOB_SIZE {
+                if blob_size > drv_sprot_api::MAX_BLOB_SIZE {
                     // If there isn't enough room, then pack an error instead
                     Response::pack(
                         &Err(SprotError::Protocol(
@@ -125,9 +125,9 @@ impl Handler {
                             .read_raw_caboose(
                                 slot,
                                 start,
-                                &mut buf[..blob_size as usize],
+                                &mut buf[..blob_size],
                             )
-                            .map_err(|e| RspBody::Caboose(Err(e.into())))?;
+                            .map_err(|e| RspBody::Caboose(Err(e)))?;
                         Ok(blob_size)
                     }) {
                         Ok(size) => size,
@@ -182,7 +182,7 @@ impl Handler {
 
     pub fn handle_request(
         &mut self,
-        req: Request,
+        req: Request<'_>,
         stats: &mut RotIoStats,
     ) -> Result<(RspBody, Option<TrailingData>), SprotError> {
         match req.body {
@@ -195,7 +195,7 @@ impl Handler {
                 };
                 Ok((RspBody::Status(status), None))
             }
-            ReqBody::IoStats => Ok((RspBody::IoStats(stats.clone()), None)),
+            ReqBody::IoStats => Ok((RspBody::IoStats(*stats), None)),
             ReqBody::RotState => match self.update.status() {
                 Ok(state) => {
                     let msg = RotState::V1 {
@@ -240,7 +240,7 @@ impl Handler {
                 Ok((RspBody::Ok, None))
             }
             ReqBody::Update(UpdateReq::WriteBlock { block_num }) => {
-                self.update.write_one_block(block_num as usize, &req.blob)?;
+                self.update.write_one_block(block_num as usize, req.blob)?;
                 Ok((RspBody::Ok, None))
             }
             ReqBody::Update(UpdateReq::Abort) => {
@@ -266,7 +266,7 @@ impl Handler {
                 CabooseReq::Size { slot } => {
                     let rsp = match self.update.caboose_size(slot) {
                         Ok(v) => Ok(CabooseRsp::Size(v)),
-                        Err(e) => Err(e.into()),
+                        Err(e) => Err(e),
                     };
                     Ok((RspBody::Caboose(rsp), None))
                 }

drv/lpc55-sprot-server/src/main.rs

@@ -283,8 +283,12 @@ impl Io {
                 }
                 let upper = (read >> 8) as u8;
                 let lower = read as u8;
-                rx.next().map(|b| *b = upper);
-                rx.next().map(|b| *b = lower);
+                if let Some(b) = rx.next() {
+                    *b = upper;
+                }
+                if let Some(b) = rx.next() {
+                    *b = lower;
+                }
             }
             Ok(())
         };

drv/lpc55-swd/build.rs

@@ -19,7 +19,7 @@ struct TaskConfig {
 fn generate_swd_functions(config: &TaskConfig) -> Result<()> {
     let out_dir = build_util::out_dir();
     let dest_path = out_dir.join("swd.rs");
-    let mut file = std::fs::File::create(&dest_path)?;
+    let mut file = std::fs::File::create(dest_path)?;
 
     let out_cfg = &config.out_cfg;
     let in_cfg = &config.in_cfg;
@@ -53,7 +53,7 @@ fn generate_swd_functions(config: &TaskConfig) -> Result<()> {
                 use drv_lpc55_gpio_api::*;
 
                 let (pin, conf) = drv_lpc55_gpio_api::Pins::iocon_conf_val(#out_cfg);
-                let base = iocon_base + 4 * (pin as u32);
+                let base = iocon_base + 4 * pin;
                 unsafe {
                     core::ptr::write_volatile(base as *mut u32, conf);
                 }
@@ -67,7 +67,7 @@ fn generate_swd_functions(config: &TaskConfig) -> Result<()> {
             {
                 use drv_lpc55_gpio_api::*;
                 let (pin, conf) = drv_lpc55_gpio_api::Pins::iocon_conf_val(#in_cfg);
-                let base = iocon_base + 4 * (pin as u32);
+                let base = iocon_base + 4 * pin;
                 unsafe {
                     core::ptr::write_volatile(base as *mut u32, conf);
                 }

drv/lpc55-swd/src/main.rs

@@ -456,10 +456,10 @@ impl idl::InOrderSpCtrlImpl for ServerImpl {
 
             // S0-S31
             | 0b1000000..=0b1011111 => Ok::<u16, SpCtrlError>(register),
-            _ => Err(SpCtrlError::InvalidCoreRegister.into())
+            _ => Err(SpCtrlError::InvalidCoreRegister)
         }?;
 
-        if let Err(_) = self.write_single_target_addr(DCRSR, r as u32) {
+        if self.write_single_target_addr(DCRSR, r as u32).is_err() {
             return Err(SpCtrlError::Fault.into());
         }
 
@@ -674,7 +674,7 @@ impl ServerImpl {
     }
 
     fn write_word(&mut self, val: u32) {
-        let parity: u32 = if val.count_ones() % 2 == 0 { 0 } else { 1 };
+        let parity: u32 = u32::from(val.count_ones() % 2 != 0);
 
         let rev = val.reverse_bits();