Merge branch 'clippy-fixes' into 'main'

zeenix · zeenix · commit 21ac31f602b5 · 2021-05-25T20:42:45.000Z
Silence clippy errors and warnings

See merge request dbus/zbus!305
diff --git a/zbus/src/azync/proxy.rs b/zbus/src/azync/proxy.rs
@@ -713,7 +713,6 @@ mod tests {
         let well_known = "org.freedesktop.zbus.async.ProxySignalConnectTest";
         let unique_name = conn.unique_name().unwrap().to_string();
         let name_owner_changed_id = {
-            let well_known = well_known.clone();
             let signaled = owner_change_signaled.clone();
 
             proxy
diff --git a/zbus/src/fdo.rs b/zbus/src/fdo.rs
@@ -635,7 +635,10 @@ mod tests {
     use ntest::timeout;
     use std::{
         convert::TryInto,
-        sync::{Arc, Mutex},
+        sync::{
+            atomic::{AtomicBool, Ordering},
+            Arc,
+        },
     };
     use test_env_log::test;
 
@@ -660,15 +663,14 @@ mod tests {
         // Register a well-known name with the session bus and ensure we get the appropriate
         // signals called for that.
         let conn = crate::Connection::new_session().unwrap();
-        let owner_change_signaled = Arc::new(Mutex::new(false));
-        let name_acquired_signaled = Arc::new(Mutex::new(false));
+        let owner_change_signaled = Arc::new(AtomicBool::new(false));
+        let name_acquired_signaled = Arc::new(AtomicBool::new(false));
 
         let proxy = fdo::DBusProxy::new(&conn);
 
         let well_known = "org.freedesktop.zbus.FdoSignalTest";
         let unique_name = conn.unique_name().unwrap().to_string();
         {
-            let well_known = well_known.clone();
             let signaled = owner_change_signaled.clone();
             proxy
                 .connect_name_owner_changed(move |name, _, new_owner| {
@@ -677,20 +679,20 @@ mod tests {
                         return Ok(());
                     }
                     assert_eq!(new_owner, unique_name);
-                    *signaled.lock().unwrap() = true;
+                    signaled.store(true, Ordering::Release);
 
                     Ok(())
                 })
                 .unwrap();
         }
         {
-            let signaled = name_acquired_signaled.clone();
             // `NameAcquired` is emitted twice, first when the unique name is assigned on
             // connection and secondly after we ask for a specific name.
+            let signaled = name_acquired_signaled.clone();
             proxy
                 .connect_name_acquired(move |name| {
                     if name == well_known {
-                        *signaled.lock().unwrap() = true;
+                        signaled.store(true, Ordering::Release);
                     }
 
                     Ok(())
@@ -705,7 +707,9 @@ mod tests {
         loop {
             proxy.next_signal().unwrap();
 
-            if *owner_change_signaled.lock().unwrap() && *name_acquired_signaled.lock().unwrap() {
+            if owner_change_signaled.load(Ordering::Acquire)
+                && name_acquired_signaled.load(Ordering::Acquire)
+            {
                 break;
             }
         }
diff --git a/zbus/src/lib.rs b/zbus/src/lib.rs
@@ -664,6 +664,7 @@ mod tests {
     }
 
     #[test]
+    #[allow(clippy::mutex_atomic)]
     fn issue_122() {
         let conn = Connection::new_session().unwrap();
         let conn_clone = conn.clone();
diff --git a/zbus/src/object_server.rs b/zbus/src/object_server.rs
@@ -559,6 +559,7 @@ impl ObjectServer {
 }
 
 #[cfg(test)]
+#[allow(clippy::blacklisted_name)]
 mod tests {
     use std::{
         cell::Cell,
diff --git a/zbus/src/proxy.rs b/zbus/src/proxy.rs
@@ -284,7 +284,7 @@ impl<'a> From<azync::Proxy<'a>> for Proxy<'a> {
 mod tests {
     use super::*;
     use ntest::timeout;
-    use std::sync::{Arc, Mutex};
+    use std::sync::atomic::{AtomicBool, Ordering};
     use test_env_log::test;
 
     #[test]
@@ -293,14 +293,13 @@ mod tests {
         // Register a well-known name with the session bus and ensure we get the appropriate
         // signals called for that.
         let conn = Connection::new_session().unwrap();
-        let owner_change_signaled = Arc::new(Mutex::new(false));
-        let name_acquired_signaled = Arc::new(Mutex::new(false));
+        let owner_change_signaled = Arc::new(AtomicBool::new(false));
+        let name_acquired_signaled = Arc::new(AtomicBool::new(false));
 
         let proxy = fdo::DBusProxy::new(&conn);
         let well_known = "org.freedesktop.zbus.ProxySignalTest";
         let unique_name = conn.unique_name().unwrap().to_string();
         {
-            let well_known = well_known.clone();
             let signaled = owner_change_signaled.clone();
             proxy
                 .connect_signal("NameOwnerChanged", move |m| {
@@ -310,7 +309,7 @@ mod tests {
                         return Ok(());
                     }
                     assert_eq!(new_owner, unique_name);
-                    *signaled.lock().unwrap() = true;
+                    signaled.store(true, Ordering::Release);
 
                     Ok(())
                 })
@@ -323,7 +322,7 @@ mod tests {
             proxy
                 .connect_signal("NameAcquired", move |m| {
                     if m.body::<&str>()? == well_known {
-                        *signaled.lock().unwrap() = true;
+                        signaled.store(true, Ordering::Release);
                     }
 
                     Ok(())
@@ -338,7 +337,9 @@ mod tests {
         loop {
             proxy.next_signal().unwrap();
 
-            if *owner_change_signaled.lock().unwrap() && *name_acquired_signaled.lock().unwrap() {
+            if owner_change_signaled.load(Ordering::Acquire)
+                && name_acquired_signaled.load(Ordering::Acquire)
+            {
                 break;
             }
         }
diff --git a/zvariant/benches/benchmarks.rs b/zvariant/benches/benchmarks.rs
@@ -56,8 +56,7 @@ fn big_array_ser_and_de(c: &mut Criterion) {
             idx, idx, idx, idx, idx, idx, idx, idx, idx, idx, idx, idx
         ));
     }
-    for idx in 0..1024 * 10 {
-        let s = &strings[idx];
+    for s in &strings {
         string_array.push(s.as_str());
         dict.insert(s.as_str(), Value::from(s.as_str()));
     }
diff --git a/zvariant/src/lib.rs b/zvariant/src/lib.rs
@@ -1,3 +1,4 @@
+#![allow(clippy::unusual_byte_groupings)]
 #![deny(rust_2018_idioms)]
 #![doc(
     html_logo_url = "https://storage.googleapis.com/fdo-gitlab-uploads/project/avatar/3213/zbus-logomark.png"
@@ -210,6 +211,7 @@ pub mod export {
 }
 
 #[cfg(test)]
+#[allow(clippy::blacklisted_name)]
 mod tests {
     use std::{
         collections::HashMap,
@@ -309,6 +311,62 @@ mod tests {
         }};
     }
 
+    fn f64_type_test(
+        format: EncodingFormat,
+        value: f64,
+        expected_len: usize,
+        expected_value_len: usize,
+    ) -> Vec<u8> {
+        // Lie that we're starting at byte 1 in the overall message to test padding
+        let ctxt = Context::<BE>::new(format, 1);
+        let (encoded, fds) = to_bytes_fds(ctxt, &value).unwrap();
+        let padding = crate::padding_for_n_bytes(1, 8);
+        assert_eq!(
+            encoded.len(),
+            expected_len + padding,
+            "invalid encoding using `to_bytes`"
+        );
+
+        let decoded: f64 = from_slice_fds(&encoded, Some(&fds), ctxt).unwrap();
+        assert!(
+            (decoded - value).abs() < f64::EPSILON,
+            "invalid decoding using `from_slice`"
+        );
+
+        // Now encode w/o padding
+        let ctxt = Context::<BE>::new(format, 0);
+        let (encoded, _) = to_bytes_fds(ctxt, &value).unwrap();
+        assert_eq!(
+            encoded.len(),
+            expected_len,
+            "invalid encoding using `to_bytes`"
+        );
+
+        f64_type_test_as_value(format, value, expected_value_len);
+        encoded
+    }
+
+    fn f64_type_test_as_value(format: EncodingFormat, value: f64, expected_value_len: usize) {
+        let v: Value<'_> = value.into();
+        assert_eq!(v.value_signature(), f64::SIGNATURE_STR);
+        assert_eq!(v, Value::F64(value));
+        f64_value_test(format, v.clone(), expected_value_len);
+        let v: f64 = v.try_into().unwrap();
+        assert!((v - value).abs() < f64::EPSILON);
+    }
+
+    fn f64_value_test(format: EncodingFormat, v: Value<'_>, expected_value_len: usize) {
+        let ctxt = Context::<LE>::new(format, 0);
+        let (encoded, fds) = to_bytes_fds(ctxt, &v).unwrap();
+        assert_eq!(
+            encoded.len(),
+            expected_value_len,
+            "invalid encoding using `to_bytes`"
+        );
+        let decoded: Value<'_> = from_slice_fds(&encoded, Some(&fds), ctxt).unwrap();
+        assert!(decoded == v, "invalid decoding using `from_slice`");
+    }
+
     #[cfg(feature = "gvariant")]
     fn decode_with_gvariant<B, T>(encoded: B) -> T
     where
@@ -411,15 +469,15 @@ mod tests {
 
     #[test]
     fn f64_value() {
-        let encoded = basic_type_test!(BE, DBus, 99999.99999_f64, 8, f64, 8, F64, 16);
-        assert_eq!(LE::read_f64(&encoded), -5759340900185448e-143);
+        let encoded = f64_type_test(EncodingFormat::DBus, 99999.99999_f64, 8, 16);
+        assert!((LE::read_f64(&encoded) - -5.759340900185448e-128).abs() < f64::EPSILON);
         #[cfg(feature = "gvariant")]
         {
-            assert_eq!(
-                decode_with_gvariant::<_, f64>(encoded),
-                -5759340900185448e-143
+            assert!(
+                (decode_with_gvariant::<_, f64>(encoded) - -5.759340900185448e-128).abs()
+                    < f64::EPSILON
             );
-            basic_type_test!(BE, GVariant, 99999.99999_f64, 8, f64, 8, F64, 10);
+            f64_type_test(EncodingFormat::GVariant, 99999.99999_f64, 8, 10);
         }
     }
 
@@ -456,8 +514,6 @@ mod tests {
             assert_eq!(variant.get_str().unwrap(), "hello world");
         }
 
-        let v: String = v.try_into().unwrap();
-        assert_eq!(v, "hello world");
         let v: String = v.try_into().unwrap();
         assert_eq!(v, "hello world");
 
@@ -569,8 +625,8 @@ mod tests {
         let ctxt = Context::<BE>::new_dbus(0);
         let (encoded, fds) = to_bytes_fds(ctxt, &()).unwrap();
         assert_eq!(encoded.len(), 0, "invalid encoding using `to_bytes`");
-        let decoded: () = from_slice_fds(&encoded, Some(&fds), ctxt).unwrap();
-        assert_eq!(decoded, (), "invalid decoding using `from_slice`");
+        let _decoded: () = from_slice_fds(&encoded, Some(&fds), ctxt)
+            .expect("invalid decoding using `from_slice`");
     }
 
     #[test]

Original file line number	Diff line number	Diff line change
`@@ -664,6 +664,7 @@ mod tests {`
`664`	`664`	`}`
`665`	`665`
`666`	`666`	`#[test]`
	`667`	`+ #[allow(clippy::mutex_atomic)]`
`667`	`668`	`fn issue_122() {`
`668`	`669`	`let conn = Connection::new_session().unwrap();`
`669`	`670`	`let conn_clone = conn.clone();`
Original file line number	Diff line number	Diff line change
`@@ -559,6 +559,7 @@ impl ObjectServer {`
`559`	`559`	`}`
`560`	`560`
`561`	`561`	`#[cfg(test)]`
	`562`	`+#[allow(clippy::blacklisted_name)]`
`562`	`563`	`mod tests {`
`563`	`564`	`use std::{`
`564`	`565`	`cell::Cell,`
Original file line number	Diff line number	Diff line change
`@@ -56,8 +56,7 @@ fn big_array_ser_and_de(c: &mut Criterion) {`
`56`	`56`	`idx, idx, idx, idx, idx, idx, idx, idx, idx, idx, idx, idx`
`57`	`57`	`));`
`58`	`58`	`}`
`59`		`- for idx in 0..1024 * 10 {`
`60`		`- let s = &strings[idx];`
	`59`	`+ for s in &strings {`
`61`	`60`	`string_array.push(s.as_str());`
`62`	`61`	`dict.insert(s.as_str(), Value::from(s.as_str()));`
`63`	`62`	`}`