commit 387c85b15f65d00075ab7aafcbda5b70cd82287f
author David Drysdale <drysdale@google.com> Mon Jun 10 14:40:45 2024 +0100
committer David Drysdale <drysdale@google.com> Thu Jun 13 11:34:22 2024 +0100
tree 60dd6881252822bdd314147b172d0d7b72d39dc5
parent e975a41eed2c126693782857adc58525fe230129

Simplify/clarify watchdog code

- Use Debug trait object rather than arbitrary closure
- Combine bool+Instant into Option<Instant>
- Disambiguate "timeout"
- Add comments
- Simplify multi-argument matches

Test: keystore2_test
Test: libwatchdog_rs.test
Flag: None, refactor
Change-Id: Ieb16257c763fc2e04d592d97f341fea27aad726f

keystore2/src/maintenance.rs

diff --git a/keystore2/src/maintenance.rs b/keystore2/src/maintenance.rs
index ba92399..6c07f0c 100644
--- a/keystore2/src/maintenance.rs
+++ b/keystore2/src/maintenance.rs
@@ -177,9 +177,7 @@
         let (km_dev, _, _) =
             get_keymint_device(&sec_level).context(ks_err!("getting keymint device"))?;
 
-        let _wp = wd::watch_millis_with("In call_with_watchdog", 500, move || {
-            format!("Seclevel: {:?} Op: {}", sec_level, name)
-        });
+        let _wp = wd::watch_millis_with("In call_with_watchdog", 500, (sec_level, name));
         map_km_error(op(km_dev)).with_context(|| ks_err!("calling {}", name))?;
         Ok(())
     }

keystore2/src/security_level.rs

diff --git a/keystore2/src/security_level.rs b/keystore2/src/security_level.rs
index 00e0480..8ce802e 100644
--- a/keystore2/src/security_level.rs
+++ b/keystore2/src/security_level.rs
@@ -109,14 +109,12 @@
 
     fn watch_millis(&self, id: &'static str, millis: u64) -> Option<wd::WatchPoint> {
         let sec_level = self.security_level;
-        wd::watch_millis_with(id, millis, move || format!("SecurityLevel {:?}", sec_level))
+        wd::watch_millis_with(id, millis, sec_level)
     }
 
     fn watch(&self, id: &'static str) -> Option<wd::WatchPoint> {
         let sec_level = self.security_level;
-        wd::watch_millis_with(id, wd::DEFAULT_TIMEOUT_MS, move || {
-            format!("SecurityLevel {:?}", sec_level)
-        })
+        wd::watch_millis_with(id, wd::DEFAULT_TIMEOUT_MS, sec_level)
     }
 
     fn store_new_key(

keystore2/src/service.rs

diff --git a/keystore2/src/service.rs b/keystore2/src/service.rs
index 3726358..b760a56 100644
--- a/keystore2/src/service.rs
+++ b/keystore2/src/service.rs
@@ -381,9 +381,7 @@
         &self,
         security_level: SecurityLevel,
     ) -> binder::Result<Strong<dyn IKeystoreSecurityLevel>> {
-        let _wp = wd::watch_millis_with("IKeystoreService::getSecurityLevel", 500, move || {
-            format!("security_level: {}", security_level.0)
-        });
+        let _wp = wd::watch_millis_with("IKeystoreService::getSecurityLevel", 500, security_level);
         self.get_security_level(security_level).map_err(into_logged_binder)
     }
     fn getKeyEntry(&self, key: &KeyDescriptor) -> binder::Result<KeyEntryResponse> {

keystore2/src/watchdog_helper.rs

diff --git a/keystore2/src/watchdog_helper.rs b/keystore2/src/watchdog_helper.rs
index 03c7740..1072ac0 100644
--- a/keystore2/src/watchdog_helper.rs
+++ b/keystore2/src/watchdog_helper.rs
@@ -43,14 +43,14 @@
         Watchdog::watch(&WD, id, DEFAULT_TIMEOUT)
     }
 
-    /// Like `watch_millis` but with a callback that is called every time a report
-    /// is printed about this watch point.
+    /// Like `watch_millis` but with context that is included every time a report is printed about
+    /// this watch point.
     pub fn watch_millis_with(
         id: &'static str,
         millis: u64,
-        callback: impl Fn() -> String + Send + 'static,
+        context: impl std::fmt::Debug + Send + 'static,
     ) -> Option<WatchPoint> {
-        Watchdog::watch_with(&WD, id, Duration::from_millis(millis), callback)
+        Watchdog::watch_with(&WD, id, Duration::from_millis(millis), context)
     }
 }
 
@@ -71,7 +71,7 @@
     pub fn watch_millis_with(
         _: &'static str,
         _: u64,
-        _: impl Fn() -> String + Send + 'static,
+        _: impl std::fmt::Debug + Send + 'static,
     ) -> Option<WatchPoint> {
         None
     }