libavb_rs: turn on linting am: fead2a894f am: df9ecc69cb am: ab36a25302

Original change: https://android-review.googlesource.com/c/platform/external/avb/+/2853571

Change-Id: I3e683e9d728ace18b80d1464b29d7b0dd953e500
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

8 files changed, 197 insertions, 146 deletions

diff --git a/rust/Android.bp b/rust/Android.bp
index 61fe0db..517d87a 100644
--- a/rust/Android.bp
+++ b/rust/Android.bp
@@ -85,8 +85,8 @@ rust_defaults {
     name: "libavb_rs_common.defaults",
     crate_name: "avb",
     srcs: ["src/lib.rs"],
-    // Require unsafe blocks for inside unsafe functions.
-    flags: ["-Dunsafe_op_in_unsafe_fn"],
+    clippy_lints: "android",
+    lints: "android",
 }
 
 // No std, no features.
@@ -164,6 +164,13 @@ rust_library {
     defaults: ["libavb_rs.defaults"],
 }
 
+// Unit tests: std, no features.
+rust_test {
+    name: "libavb_rs_unittest",
+    defaults: ["libavb_rs.defaults"],
+    test_suites: ["general-tests"],
+}
+
 // lib: std, UUID feature.
 rust_library {
     name: "libavb_rs_uuid",
@@ -173,7 +180,17 @@ rust_library {
     ],
 }
 
-// device test defaults.
+// Unit tests: std, UUID feature.
+rust_test {
+    name: "libavb_rs_uuid_unittest",
+    defaults: [
+        "libavb_rs.defaults",
+        "libavb_rs.uuid.defaults",
+    ],
+    test_suites: ["general-tests"],
+}
+
+// Integration test defaults.
 rust_defaults {
     name: "libavb_rs_test.defaults",
     srcs: ["tests/tests.rs"],
@@ -187,16 +204,18 @@ rust_defaults {
         ":avbrs_test_vbmeta_persistent_digest",
     ],
     test_suites: ["general-tests"],
+    clippy_lints: "android",
+    lints: "android",
 }
 
-// device test: no features.
+// Integration test: no features.
 rust_test {
     name: "libavb_rs_test",
     defaults: ["libavb_rs_test.defaults"],
     rustlibs: ["libavb_rs"],
 }
 
-// device test: UUID feature.
+// Integration test: UUID feature.
 rust_test {
     name: "libavb_rs_uuid_test",
     defaults: [
diff --git a/rust/TEST_MAPPING b/rust/TEST_MAPPING
index cab6ba2..711ca9e 100644
--- a/rust/TEST_MAPPING
+++ b/rust/TEST_MAPPING
@@ -7,7 +7,13 @@
             "name": "libavb_rs_test"
         },
         {
+            "name": "libavb_rs_unittest"
+        },
+        {
             "name": "libavb_rs_uuid_test"
+        },
+        {
+            "name": "libavb_rs_uuid_unittest"
         }
     ]
 }
\ No newline at end of file
diff --git a/rust/src/error.rs b/rust/src/error.rs
index 1e78f5e..f60702b 100644
--- a/rust/src/error.rs
+++ b/rust/src/error.rs
@@ -105,7 +105,7 @@ impl<'a> fmt::Display for SlotVerifyError<'a> {
     }
 }
 
-/// Converts a bindgen `AvbSlotVerifyResult` enum to a `Result<>`, mapping
+/// Converts a bindgen `AvbSlotVerifyResult` enum to a `SlotVerifyNoDataResult<>`, mapping
 /// `AVB_SLOT_VERIFY_RESULT_OK` to the Rust equivalent `Ok(())` and errors to the corresponding
 /// `Err(SlotVerifyError)`.
 ///
@@ -115,10 +115,9 @@ impl<'a> fmt::Display for SlotVerifyError<'a> {
 /// This function is also important to serve as a compile-time check that we're handling all the
 /// libavb enums; if a new one is added to (or removed from) the C code, this will fail to compile
 /// until it is updated to match.
-///
-/// TODO(b/290110273): this can be limited to pub(crate) once we've moved the full libavb wrapper
-/// here.
-pub fn slot_verify_enum_to_result(result: AvbSlotVerifyResult) -> SlotVerifyNoDataResult<()> {
+pub(crate) fn slot_verify_enum_to_result(
+    result: AvbSlotVerifyResult,
+) -> SlotVerifyNoDataResult<()> {
     match result {
         AvbSlotVerifyResult::AVB_SLOT_VERIFY_RESULT_OK => Ok(()),
         AvbSlotVerifyResult::AVB_SLOT_VERIFY_RESULT_ERROR_INVALID_ARGUMENT => {
@@ -190,27 +189,6 @@ impl From<Utf8Error> for IoError {
     }
 }
 
-// Converts a bindgen `AvbIOResult` enum to a `Result<>`, mapping `AVB_IO_RESULT_OK` to the Rust
-// equivalent `Ok(())` and errors to the corresponding `Err(IoError)`.
-//
-// This function is not currently used, but serves as a compile-time check that we're handling all
-// the libavb enums; if a new one is added to or removed from the C code, this will fail to compile
-// until it is updated to match.
-fn io_enum_to_result(result: AvbIOResult) -> IoResult<()> {
-    match result {
-        AvbIOResult::AVB_IO_RESULT_OK => Ok(()),
-        AvbIOResult::AVB_IO_RESULT_ERROR_OOM => Err(IoError::Oom),
-        AvbIOResult::AVB_IO_RESULT_ERROR_IO => Err(IoError::Io),
-        AvbIOResult::AVB_IO_RESULT_ERROR_NO_SUCH_PARTITION => Err(IoError::NoSuchPartition),
-        AvbIOResult::AVB_IO_RESULT_ERROR_RANGE_OUTSIDE_PARTITION => {
-            Err(IoError::RangeOutsidePartition)
-        }
-        AvbIOResult::AVB_IO_RESULT_ERROR_NO_SUCH_VALUE => Err(IoError::NoSuchValue),
-        AvbIOResult::AVB_IO_RESULT_ERROR_INVALID_VALUE_SIZE => Err(IoError::InvalidValueSize),
-        AvbIOResult::AVB_IO_RESULT_ERROR_INSUFFICIENT_SPACE => Err(IoError::InsufficientSpace(0)),
-    }
-}
-
 // Converts our `IoError` to the bindgen `AvbIOResult` enum.
 //
 // Unlike `SlotVerifyError` which gets generated by libavb and passed to the caller, `IoError` is
@@ -235,11 +213,8 @@ impl From<IoError> for AvbIOResult {
     }
 }
 
-// Converts a `Result<>` to the bindgen `AvbIOResult` enum.
-//
-// TODO(b/290110273): this can be limited to pub(crate) once we've moved the full libavb wrapper
-// here.
-pub fn result_to_io_enum(result: Result<(), IoError>) -> AvbIOResult {
+/// Converts an `IoResult<>` to the bindgen `AvbIOResult` enum.
+pub(crate) fn result_to_io_enum(result: IoResult<()>) -> AvbIOResult {
     result.map_or_else(|e| e.into(), |_| AvbIOResult::AVB_IO_RESULT_OK)
 }
 
@@ -273,7 +248,7 @@ impl fmt::Display for VbmetaVerifyError {
     }
 }
 
-// Converts a bindgen `AvbVBMetaVerifyResult` enum to a `Result<>`, mapping
+// Converts a bindgen `AvbVBMetaVerifyResult` enum to a `VbmetaVerifyResult<>`, mapping
 // `AVB_VBMETA_VERIFY_RESULT_OK` to the Rust equivalent `Ok(())` and errors to the corresponding
 // `Err(SlotVerifyError)`.
 //
@@ -306,7 +281,7 @@ mod tests {
     use super::*;
 
     #[test]
-    fn test_SlotVerifyError_display() {
+    fn display_slot_verify_error() {
         // The actual error message can change as needed, the point of the test is just to make sure
         // the fmt::Display trait is properly implemented.
         assert_eq!(
@@ -316,7 +291,7 @@ mod tests {
     }
 
     #[test]
-    fn test_SlotVerifyError_from_raw() {
+    fn convert_slot_verify_enum_to_result() {
         assert!(matches!(
             slot_verify_enum_to_result(AvbSlotVerifyResult::AVB_SLOT_VERIFY_RESULT_OK),
             Ok(())
@@ -328,7 +303,7 @@ mod tests {
     }
 
     #[test]
-    fn test_IoError_display() {
+    fn display_io_error() {
         // The actual error message can change as needed, the point of the test is just to make sure
         // the fmt::Display trait is properly implemented.
         assert_eq!(
@@ -338,16 +313,29 @@ mod tests {
     }
 
     #[test]
-    fn test_IoError_from_raw() {
-        assert_eq!(io_enum_to_result(AvbIOResult::AVB_IO_RESULT_OK), Ok(()));
-        assert_eq!(
-            io_enum_to_result(AvbIOResult::AVB_IO_RESULT_ERROR_IO),
-            Err(IoError::Io)
-        );
+    fn convert_io_enum_to_result() {
+        // This is a compile-time check that we handle all the `AvbIOResult` enum values. If any
+        // enums are added or removed this will break, indicating we need to update `IoError` to
+        // match.
+        assert!(match AvbIOResult::AVB_IO_RESULT_OK {
+            AvbIOResult::AVB_IO_RESULT_OK => Ok(()),
+            AvbIOResult::AVB_IO_RESULT_ERROR_OOM => Err(IoError::Oom),
+            AvbIOResult::AVB_IO_RESULT_ERROR_IO => Err(IoError::Io),
+            AvbIOResult::AVB_IO_RESULT_ERROR_NO_SUCH_PARTITION => Err(IoError::NoSuchPartition),
+            AvbIOResult::AVB_IO_RESULT_ERROR_RANGE_OUTSIDE_PARTITION => {
+                Err(IoError::RangeOutsidePartition)
+            }
+            AvbIOResult::AVB_IO_RESULT_ERROR_NO_SUCH_VALUE => Err(IoError::NoSuchValue),
+            AvbIOResult::AVB_IO_RESULT_ERROR_INVALID_VALUE_SIZE => Err(IoError::InvalidValueSize),
+            AvbIOResult::AVB_IO_RESULT_ERROR_INSUFFICIENT_SPACE => {
+                Err(IoError::InsufficientSpace(0))
+            }
+        }
+        .is_ok());
     }
 
     #[test]
-    fn test_IoError_to_raw() {
+    fn convert_io_result_to_enum() {
         assert_eq!(result_to_io_enum(Ok(())), AvbIOResult::AVB_IO_RESULT_OK);
         assert_eq!(
             result_to_io_enum(Err(IoError::Io)),
@@ -356,7 +344,7 @@ mod tests {
     }
 
     #[test]
-    fn test_VbmetaVerifyError_display() {
+    fn display_vmbeta_verify_error() {
         // The actual error message can change as needed, the point of the test is just to make sure
         // the fmt::Display trait is properly implemented.
         assert_eq!(
@@ -366,7 +354,7 @@ mod tests {
     }
 
     #[test]
-    fn test_VbmetaVerifyError_from_raw() {
+    fn convert_vbmeta_verify_enum_to_result() {
         assert_eq!(
             vbmeta_verify_enum_to_result(AvbVBMetaVerifyResult::AVB_VBMETA_VERIFY_RESULT_OK),
             Ok(())
diff --git a/rust/src/lib.rs b/rust/src/lib.rs
index 63c3818..2d52c38 100644
--- a/rust/src/lib.rs
+++ b/rust/src/lib.rs
@@ -38,13 +38,3 @@ pub use ops::{Ops, PublicKeyForPartitionInfo};
 pub use verify::{
     slot_verify, HashtreeErrorMode, PartitionData, SlotVerifyData, SlotVerifyFlags, VbmetaData,
 };
-
-/// APIs that will eventually be internal-only to this library, but while this library is split need
-/// to be exposed externally.
-//
-// TODO(b/290110273): remove this module once we've moved the full libavb wrapper here.
-pub mod internal {
-    use super::*;
-
-    pub use error::{result_to_io_enum, slot_verify_enum_to_result};
-}
diff --git a/rust/src/ops.rs b/rust/src/ops.rs
index 005aa98..e030dda 100644
--- a/rust/src/ops.rs
+++ b/rust/src/ops.rs
@@ -68,7 +68,7 @@ pub trait Ops {
     /// * `Err<IoError::NotImplemented>` if the requested partition has not been preloaded;
     ///   verification will next attempt to load the partition via `read_from_partition()`.
     /// * Any other `Err<IoError>` if an error occurred; verification will exit immediately.
-    fn get_preloaded_partition(&mut self, partition: &CStr) -> IoResult<&[u8]> {
+    fn get_preloaded_partition(&mut self, _partition: &CStr) -> IoResult<&[u8]> {
         Err(IoError::NotImplemented)
     }
 
@@ -375,14 +375,17 @@ unsafe extern "C" fn read_from_partition(
     buffer: *mut c_void,
     out_num_read: *mut usize,
 ) -> AvbIOResult {
-    result_to_io_enum(try_read_from_partition(
-        ops,
-        partition,
-        offset,
-        num_bytes,
-        buffer,
-        out_num_read,
-    ))
+    // SAFETY: see corresponding `try_*` function safety documentation.
+    unsafe {
+        result_to_io_enum(try_read_from_partition(
+            ops,
+            partition,
+            offset,
+            num_bytes,
+            buffer,
+            out_num_read,
+        ))
+    }
 }
 
 /// Bounces the C callback into the user-provided Rust implementation.
@@ -445,13 +448,16 @@ unsafe extern "C" fn get_preloaded_partition(
     out_pointer: *mut *mut u8,
     out_num_bytes_preloaded: *mut usize,
 ) -> AvbIOResult {
-    result_to_io_enum(try_get_preloaded_partition(
-        ops,
-        partition,
-        num_bytes,
-        out_pointer,
-        out_num_bytes_preloaded,
-    ))
+    // SAFETY: see corresponding `try_*` function safety documentation.
+    unsafe {
+        result_to_io_enum(try_get_preloaded_partition(
+            ops,
+            partition,
+            num_bytes,
+            out_pointer,
+            out_num_bytes_preloaded,
+        ))
+    }
 }
 
 /// Bounces the C callback into the user-provided Rust implementation.
@@ -530,14 +536,17 @@ unsafe extern "C" fn validate_vbmeta_public_key(
     public_key_metadata_length: usize,
     out_is_trusted: *mut bool,
 ) -> AvbIOResult {
-    result_to_io_enum(try_validate_vbmeta_public_key(
-        ops,
-        public_key_data,
-        public_key_length,
-        public_key_metadata,
-        public_key_metadata_length,
-        out_is_trusted,
-    ))
+    // SAFETY: see corresponding `try_*` function safety documentation.
+    unsafe {
+        result_to_io_enum(try_validate_vbmeta_public_key(
+            ops,
+            public_key_data,
+            public_key_length,
+            public_key_metadata,
+            public_key_metadata_length,
+            out_is_trusted,
+        ))
+    }
 }
 
 /// Bounces the C callback into the user-provided Rust implementation.
@@ -600,11 +609,14 @@ unsafe extern "C" fn read_rollback_index(
     rollback_index_location: usize,
     out_rollback_index: *mut u64,
 ) -> AvbIOResult {
-    result_to_io_enum(try_read_rollback_index(
-        ops,
-        rollback_index_location,
-        out_rollback_index,
-    ))
+    // SAFETY: see corresponding `try_*` function safety documentation.
+    unsafe {
+        result_to_io_enum(try_read_rollback_index(
+            ops,
+            rollback_index_location,
+            out_rollback_index,
+        ))
+    }
 }
 
 /// Bounces the C callback into the user-provided Rust implementation.
@@ -646,11 +658,14 @@ unsafe extern "C" fn write_rollback_index(
     rollback_index_location: usize,
     rollback_index: u64,
 ) -> AvbIOResult {
-    result_to_io_enum(try_write_rollback_index(
-        ops,
-        rollback_index_location,
-        rollback_index,
-    ))
+    // SAFETY: see corresponding `try_*` function safety documentation.
+    unsafe {
+        result_to_io_enum(try_write_rollback_index(
+            ops,
+            rollback_index_location,
+            rollback_index,
+        ))
+    }
 }
 
 /// Bounces the C callback into the user-provided Rust implementation.
@@ -676,7 +691,8 @@ unsafe extern "C" fn read_is_device_unlocked(
     ops: *mut AvbOps,
     out_is_unlocked: *mut bool,
 ) -> AvbIOResult {
-    result_to_io_enum(try_read_is_device_unlocked(ops, out_is_unlocked))
+    // SAFETY: see corresponding `try_*` function safety documentation.
+    unsafe { result_to_io_enum(try_read_is_device_unlocked(ops, out_is_unlocked)) }
 }
 
 /// Bounces the C callback into the user-provided Rust implementation.
@@ -718,41 +734,45 @@ unsafe extern "C" fn get_unique_guid_for_partition(
     guid_buf: *mut c_char,
     guid_buf_size: usize,
 ) -> AvbIOResult {
-    result_to_io_enum(try_get_unique_guid_for_partition(
-        ops,
-        partition,
-        guid_buf,
-        guid_buf_size,
-    ))
+    // SAFETY: see corresponding `try_*` function safety documentation.
+    unsafe {
+        result_to_io_enum(try_get_unique_guid_for_partition(
+            ops,
+            partition,
+            guid_buf,
+            guid_buf_size,
+        ))
+    }
 }
 
 /// Bounces the C callback into the user-provided Rust implementation.
 ///
+/// When the `uuid` feature is not enabled, this doesn't call into the user ops at all and instead
+/// gives the empty string for all partitions.
+///
 /// # Safety
 /// * `ops` must have been created via `ScopedAvbOps`.
 /// * `partition` must adhere to the requirements of `CStr::from_ptr()`.
 /// * `guid_buf` must adhere to the requirements of `slice::from_raw_parts_mut()`.
 unsafe fn try_get_unique_guid_for_partition(
-    ops: *mut AvbOps,
-    partition: *const c_char,
+    #[allow(unused_variables)] ops: *mut AvbOps,
+    #[allow(unused_variables)] partition: *const c_char,
     guid_buf: *mut c_char,
     guid_buf_size: usize,
 ) -> IoResult<()> {
-    check_nonnull(partition)?;
     check_nonnull(guid_buf)?;
 
-    // SAFETY:
-    // * we've checked that the pointer is non-NULL.
-    // * libavb gives us a properly-allocated and nul-terminated `partition`.
-    // * the string contents are not modified while the returned `&CStr` exists.
-    // * the returned `&CStr` is not held past the scope of this callback.
-    let partition = unsafe { CStr::from_ptr(partition) };
+    // On some architectures `c_char` is `u8`, and on others `i8`. We make sure it's `u8` here
+    // since that's what `CStr::to_bytes_with_nul()` always provides.
+    #[allow(clippy::unnecessary_cast)]
+    let guid_buf = guid_buf as *mut u8;
+
     // SAFETY:
     // * we've checked that the pointer is non-NULL.
     // * libavb gives us a properly-allocated `guid_buf` with size `guid_buf_size`.
     // * we only access the contents via the returned slice.
     // * the returned slice is not held past the scope of this callback.
-    let buffer = unsafe { slice::from_raw_parts_mut(guid_buf as *mut u8, guid_buf_size) };
+    let buffer = unsafe { slice::from_raw_parts_mut(guid_buf, guid_buf_size) };
 
     // Initialize the output buffer to the empty string.
     //
@@ -768,6 +788,15 @@ unsafe fn try_get_unique_guid_for_partition(
 
     #[cfg(feature = "uuid")]
     {
+        check_nonnull(partition)?;
+
+        // SAFETY:
+        // * we've checked that the pointer is non-NULL.
+        // * libavb gives us a properly-allocated and nul-terminated `partition`.
+        // * the string contents are not modified while the returned `&CStr` exists.
+        // * the returned `&CStr` is not held past the scope of this callback.
+        let partition = unsafe { CStr::from_ptr(partition) };
+
         // SAFETY:
         // * we only use `ops` objects created via `ScopedAvbOps` as required.
         // * `ops` is only extracted once and is dropped at the end of the callback.
@@ -778,7 +807,7 @@ unsafe fn try_get_unique_guid_for_partition(
         // `CString` to apply nul-termination.
         // This does allocate memory, but it's short-lived and discarded as soon as we copy the
         // properly-terminated string back to the buffer.
-        let mut encode_buffer = uuid::Uuid::encode_buffer();
+        let mut encode_buffer = Uuid::encode_buffer();
         let guid_str = guid.as_hyphenated().encode_lower(&mut encode_buffer);
         let guid_cstring = alloc::ffi::CString::new(guid_str.as_bytes()).or(Err(IoError::Io))?;
         let guid_bytes = guid_cstring.to_bytes_with_nul();
@@ -802,11 +831,14 @@ unsafe extern "C" fn get_size_of_partition(
     partition: *const c_char,
     out_size_num_bytes: *mut u64,
 ) -> AvbIOResult {
-    result_to_io_enum(try_get_size_of_partition(
-        ops,
-        partition,
-        out_size_num_bytes,
-    ))
+    // SAFETY: see corresponding `try_*` function safety documentation.
+    unsafe {
+        result_to_io_enum(try_get_size_of_partition(
+            ops,
+            partition,
+            out_size_num_bytes,
+        ))
+    }
 }
 
 /// Bounces the C callback into the user-provided Rust implementation.
@@ -858,13 +890,16 @@ unsafe extern "C" fn read_persistent_value(
     out_buffer: *mut u8,
     out_num_bytes_read: *mut usize,
 ) -> AvbIOResult {
-    result_to_io_enum(try_read_persistent_value(
-        ops,
-        name,
-        buffer_size,
-        out_buffer,
-        out_num_bytes_read,
-    ))
+    // SAFETY: see corresponding `try_*` function safety documentation.
+    unsafe {
+        result_to_io_enum(try_read_persistent_value(
+            ops,
+            name,
+            buffer_size,
+            out_buffer,
+            out_num_bytes_read,
+        ))
+    }
 }
 
 /// Bounces the C callback into the user-provided Rust implementation.
@@ -935,7 +970,8 @@ unsafe extern "C" fn write_persistent_value(
     value_size: usize,
     value: *const u8,
 ) -> AvbIOResult {
-    result_to_io_enum(try_write_persistent_value(ops, name, value_size, value))
+    // SAFETY: see corresponding `try_*` function safety documentation.
+    unsafe { result_to_io_enum(try_write_persistent_value(ops, name, value_size, value)) }
 }
 
 /// Bounces the C callback into the user-provided Rust implementation.
@@ -990,16 +1026,19 @@ unsafe extern "C" fn validate_public_key_for_partition(
     out_is_trusted: *mut bool,
     out_rollback_index_location: *mut u32,
 ) -> AvbIOResult {
-    result_to_io_enum(try_validate_public_key_for_partition(
-        ops,
-        partition,
-        public_key_data,
-        public_key_length,
-        public_key_metadata,
-        public_key_metadata_length,
-        out_is_trusted,
-        out_rollback_index_location,
-    ))
+    // SAFETY: see corresponding `try_*` function safety documentation.
+    unsafe {
+        result_to_io_enum(try_validate_public_key_for_partition(
+            ops,
+            partition,
+            public_key_data,
+            public_key_length,
+            public_key_metadata,
+            public_key_metadata_length,
+            out_is_trusted,
+            out_rollback_index_location,
+        ))
+    }
 }
 
 /// Bounces the C callback into the user-provided Rust implementation.
@@ -1009,6 +1048,7 @@ unsafe extern "C" fn validate_public_key_for_partition(
 /// * `partition` must adhere to the requirements of `CStr::from_ptr()`.
 /// * `public_key_*` args must adhere to the requirements of `slice::from_raw_parts()`.
 /// * `out_*` must adhere to the requirements of `ptr::write()`.
+#[allow(clippy::too_many_arguments)] // Mirroring libavb C API.
 unsafe fn try_validate_public_key_for_partition(
     ops: *mut AvbOps,
     partition: *const c_char,
diff --git a/rust/src/verify.rs b/rust/src/verify.rs
index 288aed2..0c6e9ad 100644
--- a/rust/src/verify.rs
+++ b/rust/src/verify.rs
@@ -22,7 +22,7 @@ use crate::{
         slot_verify_enum_to_result, vbmeta_verify_enum_to_result, SlotVerifyError,
         SlotVerifyNoDataResult, SlotVerifyResult, VbmetaVerifyResult,
     },
-    ops, IoError, Ops,
+    ops, Ops,
 };
 use avb_bindgen::{
     avb_slot_verify, avb_slot_verify_data_free, AvbPartitionData, AvbSlotVerifyData, AvbVBMetaData,
@@ -30,8 +30,7 @@ use avb_bindgen::{
 use core::{
     ffi::{c_char, CStr},
     fmt,
-    marker::PhantomData,
-    ptr::{null, null_mut, NonNull},
+    ptr::{self, null, null_mut, NonNull},
     slice,
 };
 
@@ -102,7 +101,7 @@ impl fmt::Display for VbmetaData {
 
 /// Forwards to `Display` formatting; the default `Debug` formatting implementation isn't very
 /// useful as it's mostly raw pointer addresses.
-impl<'a> fmt::Debug for VbmetaData {
+impl fmt::Debug for VbmetaData {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         fmt::Display::fmt(self, f)
     }
@@ -176,7 +175,7 @@ impl fmt::Display for PartitionData {
 
 /// Forwards to `Display` formatting; the default `Debug` formatting implementation isn't very
 /// useful as it's mostly raw pointer addresses.
-impl<'a> fmt::Debug for PartitionData {
+impl fmt::Debug for PartitionData {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         fmt::Display::fmt(self, f)
     }
@@ -185,16 +184,25 @@ impl<'a> fmt::Debug for PartitionData {
 /// Wraps a raw C `AvbSlotVerifyData` struct.
 ///
 /// This provides a Rust safe view over the raw data; no copies are made.
-#[derive(PartialEq, Eq)]
 pub struct SlotVerifyData<'a> {
     /// Internally owns the underlying data and deletes it on drop.
     raw_data: NonNull<AvbSlotVerifyData>,
 
-    /// This provides the necessary lifetime information so the compiler can make sure that
-    /// the `Ops` stays alive at least as long as we do.
-    _ops: PhantomData<&'a dyn Ops>,
+    /// This provides the necessary lifetime borrow so the compiler can make sure that the `Ops`
+    /// stays alive at least as long as we do, since it owns any preloaded partition data.
+    _ops: &'a dyn Ops,
 }
 
+// Useful so that `SlotVerifyError`, which may hold a `SlotVerifyData`, can derive `PartialEq`.
+impl<'a> PartialEq for SlotVerifyData<'a> {
+    fn eq(&self, other: &Self) -> bool {
+        // A `SlotVerifyData` uniquely owns the underlying data so is only equal to itself.
+        ptr::eq(self, other)
+    }
+}
+
+impl<'a> Eq for SlotVerifyData<'a> {}
+
 impl<'a> SlotVerifyData<'a> {
     /// Creates a `SlotVerifyData` wrapping the given raw `AvbSlotVerifyData`.
     ///
@@ -218,7 +226,7 @@ impl<'a> SlotVerifyData<'a> {
     ) -> SlotVerifyNoDataResult<Self> {
         let ret = Self {
             raw_data: NonNull::new(data).ok_or(SlotVerifyError::Internal)?,
-            _ops: PhantomData,
+            _ops: ops,
         };
 
         // Validate all the contained data here so accessors will never fail.
diff --git a/rust/tests/test_ops.rs b/rust/tests/test_ops.rs
index aa8c579..1c62c4d 100644
--- a/rust/tests/test_ops.rs
+++ b/rust/tests/test_ops.rs
@@ -310,7 +310,7 @@ impl Ops for TestOps {
             .ok_or(IoError::Io)?;
 
         if let Some(for_partition) = key.for_partition {
-            if (for_partition == partition.to_str()?) {
+            if for_partition == partition.to_str()? {
                 // The key is registered for this partition; return its info.
                 return Ok(key.info);
             }
diff --git a/rust/tests/verify_tests.rs b/rust/tests/verify_tests.rs
index 13d4912..353edf3 100644
--- a/rust/tests/verify_tests.rs
+++ b/rust/tests/verify_tests.rs
@@ -21,7 +21,7 @@ use avb::{
 };
 use std::{ffi::CString, fs};
 #[cfg(feature = "uuid")]
-use uuid::{uuid, Uuid};
+use uuid::uuid;
 
 // These constants must match the values used to create the images in Android.bp.
 const TEST_IMAGE_PATH: &str = "test_image.img";