Merge pull request #6 from mibes/clippy-fixes

Implemented Clippy recommendations

Author: kedars

boxslab/src/lib.rs

@@ -15,7 +15,6 @@
  *    limitations under the License.
  */
 
-
 use std::{
     mem::MaybeUninit,
     ops::{Deref, DerefMut},
@@ -115,7 +114,7 @@ impl<T: SlabPool> Slab<T> {
         }))
     }
 
-    pub fn new(new_object: T::SlabType) -> Option<BoxSlab<T>> {
+    pub fn try_new(new_object: T::SlabType) -> Option<BoxSlab<T>> {
         let slab = T::get_slab();
         let mut inner = slab.0.lock().unwrap();
         if let Some(index) = inner.map.first_false_index() {
@@ -180,40 +179,37 @@ mod tests {
     #[test]
     fn simple_alloc_free() {
         {
-            let a = Slab::<TestSlab>::new(Test { val: Arc::new(10) }).unwrap();
+            let a = Slab::<TestSlab>::try_new(Test { val: Arc::new(10) }).unwrap();
             assert_eq!(*a.val.deref(), 10);
             let inner = TestSlab::get_slab().0.lock().unwrap();
-            assert_eq!(inner.map.is_empty(), false);
+            assert!(!inner.map.is_empty());
         }
         // Validates that the 'Drop' got executed
         let inner = TestSlab::get_slab().0.lock().unwrap();
-        assert_eq!(inner.map.is_empty(), true);
+        assert!(inner.map.is_empty());
         println!("Box Size {}", std::mem::size_of::<Box<Test>>());
         println!("BoxSlab Size {}", std::mem::size_of::<BoxSlab<TestSlab>>());
     }
 
     #[test]
     fn alloc_full_block() {
         {
-            let a = Slab::<TestSlab>::new(Test { val: Arc::new(10) }).unwrap();
-            let b = Slab::<TestSlab>::new(Test { val: Arc::new(11) }).unwrap();
-            let c = Slab::<TestSlab>::new(Test { val: Arc::new(12) }).unwrap();
+            let a = Slab::<TestSlab>::try_new(Test { val: Arc::new(10) }).unwrap();
+            let b = Slab::<TestSlab>::try_new(Test { val: Arc::new(11) }).unwrap();
+            let c = Slab::<TestSlab>::try_new(Test { val: Arc::new(12) }).unwrap();
             // Test that at overflow, we return None
-            assert_eq!(
-                Slab::<TestSlab>::new(Test { val: Arc::new(13) }).is_none(),
-                true
-            );
+            assert!(Slab::<TestSlab>::try_new(Test { val: Arc::new(13) }).is_none(),);
             assert_eq!(*b.val.deref(), 11);
 
             {
                 let inner = TestSlab::get_slab().0.lock().unwrap();
                 // Test that the bitmap is marked as full
-                assert_eq!(inner.map.is_full(), true);
+                assert!(inner.map.is_full());
             }
 
             // Purposefully drop, to test that new allocation is possible
             std::mem::drop(b);
-            let d = Slab::<TestSlab>::new(Test { val: Arc::new(21) }).unwrap();
+            let d = Slab::<TestSlab>::try_new(Test { val: Arc::new(21) }).unwrap();
             assert_eq!(*d.val.deref(), 21);
 
             // Ensure older allocations are still valid
@@ -223,16 +219,16 @@ mod tests {
 
         // Validates that the 'Drop' got executed - test that the bitmap is empty
         let inner = TestSlab::get_slab().0.lock().unwrap();
-        assert_eq!(inner.map.is_empty(), true);
+        assert!(inner.map.is_empty());
     }
 
     #[test]
     fn test_drop_logic() {
         let root = Arc::new(10);
         {
-            let _a = Slab::<TestSlab>::new(Test { val: root.clone() }).unwrap();
-            let _b = Slab::<TestSlab>::new(Test { val: root.clone() }).unwrap();
-            let _c = Slab::<TestSlab>::new(Test { val: root.clone() }).unwrap();
+            let _a = Slab::<TestSlab>::try_new(Test { val: root.clone() }).unwrap();
+            let _b = Slab::<TestSlab>::try_new(Test { val: root.clone() }).unwrap();
+            let _c = Slab::<TestSlab>::try_new(Test { val: root.clone() }).unwrap();
             assert_eq!(Arc::strong_count(&root), 4);
         }
         // Test that Drop was correctly called on all the members of the pool

examples/onoff_light/src/main.rs

@@ -40,7 +40,7 @@ fn main() {
     };
     let dev_att = Box::new(dev_att::HardCodedDevAtt::new());
 
-    let mut matter = core::Matter::new(dev_info, dev_att, comm_data).unwrap();
+    let mut matter = core::Matter::new(&dev_info, dev_att, &comm_data).unwrap();
     let dm = matter.get_data_model();
     {
         let mut node = dm.node.write().unwrap();

matter/src/acl.rs

@@ -487,6 +487,7 @@ impl std::fmt::Display for AclMgr {
 }
 
 #[cfg(test)]
+#[allow(clippy::bool_assert_comparison)]
 mod tests {
     use crate::{
         data_model::objects::{Access, Privilege},

matter/src/cert/asn1_writer.rs

@@ -18,6 +18,7 @@
 use super::{CertConsumer, MAX_DEPTH};
 use crate::error::Error;
 use chrono::{TimeZone, Utc};
+use log::warn;
 
 #[derive(Debug)]
 pub struct ASN1Writer<'a> {
@@ -255,10 +256,22 @@ impl<'a> CertConsumer for ASN1Writer<'a> {
     }
 
     fn utctime(&mut self, _tag: &str, epoch: u32) -> Result<(), Error> {
-        let mut matter_epoch = Utc.ymd(2000, 1, 1).and_hms(0, 0, 0).timestamp();
+        let mut matter_epoch = Utc
+            .with_ymd_and_hms(2000, 1, 1, 0, 0, 0)
+            .unwrap()
+            .timestamp();
+
         matter_epoch += epoch as i64;
 
-        let dt = Utc.timestamp(matter_epoch, 0);
+        let dt = match Utc.timestamp_opt(matter_epoch, 0) {
+            chrono::LocalResult::None => return Err(Error::InvalidTime),
+            chrono::LocalResult::Single(s) => s,
+            chrono::LocalResult::Ambiguous(_, a) => {
+                warn!("Ambiguous time for epoch {epoch}; returning latest timestamp: {a}");
+                a
+            }
+        };
+
         let time_str = format!("{}Z", dt.format("%y%m%d%H%M%S"));
         self.write_str(0x17, time_str.as_bytes())
     }

matter/src/cert/mod.rs

@@ -348,20 +348,17 @@ impl DistNames {
         w.start_seq(tag)?;
         for (id, value) in &self.dn {
             if let Ok(tag) = num::FromPrimitive::from_u8(*id).ok_or(Error::InvalidData) {
-                match tag {
-                    DnTags::NocCat => {
-                        w.start_set("")?;
-                        w.start_seq("")?;
-                        w.oid("Chip NOC CAT Id:", &OID_MATTER_NOC_CAT_ID)?;
-                        w.utf8str("", format!("{:08X}", value).as_str())?;
-                        w.end_seq()?;
-                        w.end_set()?;
-                    }
-                    _ => {
-                        let index: usize = (*id as usize) - (DnTags::NodeId as usize);
-                        let this = &dn_encoding[index];
-                        encode_u64_dn(*value, this.0, this.1, w)?;
-                    }
+                if let DnTags::NocCat = tag {
+                    w.start_set("")?;
+                    w.start_seq("")?;
+                    w.oid("Chip NOC CAT Id:", &OID_MATTER_NOC_CAT_ID)?;
+                    w.utf8str("", format!("{:08X}", value).as_str())?;
+                    w.end_seq()?;
+                    w.end_set()?;
+                } else {
+                    let index: usize = (*id as usize) - (DnTags::NodeId as usize);
+                    let this = &dn_encoding[index];
+                    encode_u64_dn(*value, this.0, this.1, w)?;
                 }
             } else {
                 error!("Non Matter DNs are not yet supported {}", id);
@@ -452,7 +449,7 @@ impl Cert {
 
     pub fn as_asn1(&self, buf: &mut [u8]) -> Result<usize, Error> {
         let mut w = ASN1Writer::new(buf);
-        let _ = self.encode(&mut w)?;
+        self.encode(&mut w)?;
         Ok(w.as_slice().len())
     }
 
@@ -657,7 +654,7 @@ mod tests {
 
         for input in test_input.iter() {
             println!("Testing next input...");
-            let root = tlv::get_root_node(*input).unwrap();
+            let root = tlv::get_root_node(input).unwrap();
             let cert = Cert::from_tlv(&root).unwrap();
             let mut buf = [0u8; 1024];
             let buf_len = buf.len();

matter/src/cert/printer.rs

@@ -18,6 +18,7 @@
 use super::{CertConsumer, MAX_DEPTH};
 use crate::error::Error;
 use chrono::{TimeZone, Utc};
+use log::warn;
 use std::fmt;
 
 pub struct CertPrinter<'a, 'b> {
@@ -118,15 +119,23 @@ impl<'a, 'b> CertConsumer for CertPrinter<'a, 'b> {
         Ok(())
     }
     fn utctime(&mut self, tag: &str, epoch: u32) -> Result<(), Error> {
-        let mut matter_epoch = Utc.ymd(2000, 1, 1).and_hms(0, 0, 0).timestamp();
+        let mut matter_epoch = Utc
+            .with_ymd_and_hms(2000, 1, 1, 0, 0, 0)
+            .unwrap()
+            .timestamp();
+
         matter_epoch += epoch as i64;
-        let _ = writeln!(
-            self.f,
-            "{} {} {}",
-            SPACE[self.level],
-            tag,
-            Utc.timestamp(matter_epoch, 0)
-        );
+
+        let dt = match Utc.timestamp_opt(matter_epoch, 0) {
+            chrono::LocalResult::None => return Err(Error::InvalidTime),
+            chrono::LocalResult::Single(s) => s,
+            chrono::LocalResult::Ambiguous(_, a) => {
+                warn!("Ambiguous time for epoch {epoch}; returning latest timestamp: {a}");
+                a
+            }
+        };
+
+        let _ = writeln!(self.f, "{} {} {}", SPACE[self.level], tag, dt);
         Ok(())
     }
 }

matter/src/core.rs

@@ -57,9 +57,9 @@ impl Matter {
     /// requires a set of device attestation certificates and keys. It is the responsibility of
     /// this object to return the device attestation details when queried upon.
     pub fn new(
-        dev_det: BasicInfoConfig,
+        dev_det: &BasicInfoConfig,
         dev_att: Box<dyn DevAttDataFetcher>,
-        dev_comm: CommissioningData,
+        dev_comm: &CommissioningData,
     ) -> Result<Box<Matter>, Error> {
         let mdns = Mdns::get()?;
         mdns.set_values(dev_det.vid, dev_det.pid, dev_comm.discriminator);

matter/src/data_model/cluster_basic_information.rs

@@ -74,7 +74,7 @@ pub struct BasicInfoCluster {
 }
 
 impl BasicInfoCluster {
-    pub fn new(cfg: BasicInfoConfig) -> Result<Box<Self>, Error> {
+    pub fn new(cfg: &BasicInfoConfig) -> Result<Box<Self>, Error> {
         let mut cluster = Box::new(BasicInfoCluster {
             base: Cluster::new(ID)?,
         });

matter/src/data_model/core.rs

@@ -50,7 +50,7 @@ pub struct DataModel {
 
 impl DataModel {
     pub fn new(
-        dev_details: BasicInfoConfig,
+        dev_details: &BasicInfoConfig,
         dev_att: Box<dyn DevAttDataFetcher>,
         fabric_mgr: Arc<FabricMgr>,
         acl_mgr: Arc<AclMgr>,
@@ -171,7 +171,7 @@ impl DataModel {
             // Set the cluster's data version
             attr_encoder.set_data_ver(cluster_data_ver);
             let mut access_req = AccessReq::new(accessor, path, Access::READ);
-            Cluster::read_attribute(c, &mut access_req, attr_encoder, &attr_details);
+            Cluster::read_attribute(c, &mut access_req, attr_encoder, attr_details);
             Ok(())
         });
         if let Err(e) = result {

matter/src/data_model/device_types.rs

@@ -34,7 +34,7 @@ type WriteNode<'a> = RwLockWriteGuard<'a, Box<Node>>;
 
 pub fn device_type_add_root_node(
     node: &mut WriteNode,
-    dev_info: BasicInfoConfig,
+    dev_info: &BasicInfoConfig,
     dev_att: Box<dyn DevAttDataFetcher>,
     fabric_mgr: Arc<FabricMgr>,
     acl_mgr: Arc<AclMgr>,