Shift to idiomatic use of `map_err`

Test: keystore2_test
Test: legacykeystore_test
Flag: none, pure refactoring
Change-Id: I4b9f1b0d47145846764ff46676b10035f7f2fb6a
diff --git a/keystore2/src/apc.rs b/keystore2/src/apc.rs
index 8fda1f2..fc36a0c 100644
--- a/keystore2/src/apc.rs
+++ b/keystore2/src/apc.rs
@@ -73,29 +73,24 @@
     }
 }
 
-/// This function should be used by confirmation service calls to translate error conditions
-/// into service specific exceptions.
-///
-/// All error conditions get logged by this function.
+/// Translate an error into a service-specific exception, logging along the way.
 ///
 /// `Error::Rc(x)` variants get mapped onto a service specific error code of `x`.
 /// `selinux::Error::perm()` is mapped on `ResponseCode::PERMISSION_DENIED`.
 ///
 /// All non `Error` error conditions get mapped onto ResponseCode::SYSTEM_ERROR`.
-pub fn map_or_log_err<T>(result: Result<T>) -> BinderResult<T> {
-    result.map_err(|e| {
-        log::error!("{:#?}", e);
-        let root_cause = e.root_cause();
-        let rc = match root_cause.downcast_ref::<Error>() {
-            Some(Error::Rc(rcode)) => rcode.0,
-            Some(Error::Binder(_, _)) => ResponseCode::SYSTEM_ERROR.0,
-            None => match root_cause.downcast_ref::<selinux::Error>() {
-                Some(selinux::Error::PermissionDenied) => ResponseCode::PERMISSION_DENIED.0,
-                _ => ResponseCode::SYSTEM_ERROR.0,
-            },
-        };
-        BinderStatus::new_service_specific_error(rc, anyhow_error_to_cstring(&e).as_deref())
-    })
+pub fn into_logged_binder(e: anyhow::Error) -> BinderStatus {
+    log::error!("{:#?}", e);
+    let root_cause = e.root_cause();
+    let rc = match root_cause.downcast_ref::<Error>() {
+        Some(Error::Rc(rcode)) => rcode.0,
+        Some(Error::Binder(_, _)) => ResponseCode::SYSTEM_ERROR.0,
+        None => match root_cause.downcast_ref::<selinux::Error>() {
+            Some(selinux::Error::PermissionDenied) => ResponseCode::PERMISSION_DENIED.0,
+            _ => ResponseCode::SYSTEM_ERROR.0,
+        },
+    };
+    BinderStatus::new_service_specific_error(rc, anyhow_error_to_cstring(&e).as_deref())
 }
 
 /// Rate info records how many failed attempts a client has made to display a protected
@@ -341,23 +336,18 @@
     ) -> BinderResult<()> {
         // presentPrompt can take more time than other operations.
         let _wp = wd::watch_millis("IProtectedConfirmation::presentPrompt", 3000);
-        map_or_log_err(self.present_prompt(
-            listener,
-            prompt_text,
-            extra_data,
-            locale,
-            ui_option_flags,
-        ))
+        self.present_prompt(listener, prompt_text, extra_data, locale, ui_option_flags)
+            .map_err(into_logged_binder)
     }
     fn cancelPrompt(
         &self,
         listener: &binder::Strong<dyn IConfirmationCallback>,
     ) -> BinderResult<()> {
         let _wp = wd::watch("IProtectedConfirmation::cancelPrompt");
-        map_or_log_err(self.cancel_prompt(listener))
+        self.cancel_prompt(listener).map_err(into_logged_binder)
     }
     fn isSupported(&self) -> BinderResult<bool> {
         let _wp = wd::watch("IProtectedConfirmation::isSupported");
-        map_or_log_err(Self::is_supported())
+        Self::is_supported().map_err(into_logged_binder)
     }
 }
diff --git a/keystore2/src/authorization.rs b/keystore2/src/authorization.rs
index 4636a66..5a3fdbc 100644
--- a/keystore2/src/authorization.rs
+++ b/keystore2/src/authorization.rs
@@ -51,10 +51,7 @@
     Binder(ExceptionCode, i32),
 }
 
-/// This function should be used by authorization service calls to translate error conditions
-/// into service specific exceptions.
-///
-/// All error conditions get logged by this function.
+/// Translate an error into a service specific exception, logging along the way.
 ///
 /// `Error::Rc(x)` variants get mapped onto a service specific error code of `x`.
 /// Certain response codes may be returned from keystore/ResponseCode.aidl by the keystore2 modules,
@@ -64,38 +61,36 @@
 /// `selinux::Error::perm()` is mapped on `ResponseCode::PERMISSION_DENIED`.
 ///
 /// All non `Error` error conditions get mapped onto ResponseCode::SYSTEM_ERROR`.
-pub fn map_or_log_err<T>(result: Result<T>) -> BinderResult<T> {
-    result.map_err(|e| {
-        log::error!("{:#?}", e);
-        let root_cause = e.root_cause();
-        if let Some(KeystoreError::Rc(ks_rcode)) = root_cause.downcast_ref::<KeystoreError>() {
-            let rc = match *ks_rcode {
-                // Although currently keystore2/ResponseCode.aidl and
-                // authorization/ResponseCode.aidl share the same integer values for the
-                // common response codes, this may deviate in the future, hence the
-                // conversion here.
-                KsResponseCode::SYSTEM_ERROR => ResponseCode::SYSTEM_ERROR.0,
-                KsResponseCode::KEY_NOT_FOUND => ResponseCode::KEY_NOT_FOUND.0,
-                KsResponseCode::VALUE_CORRUPTED => ResponseCode::VALUE_CORRUPTED.0,
-                KsResponseCode::INVALID_ARGUMENT => ResponseCode::INVALID_ARGUMENT.0,
-                // If the code paths of IKeystoreAuthorization aidl's methods happen to return
-                // other error codes from KsResponseCode in the future, they should be converted
-                // as well.
+pub fn into_logged_binder(e: anyhow::Error) -> BinderStatus {
+    log::error!("{:#?}", e);
+    let root_cause = e.root_cause();
+    if let Some(KeystoreError::Rc(ks_rcode)) = root_cause.downcast_ref::<KeystoreError>() {
+        let rc = match *ks_rcode {
+            // Although currently keystore2/ResponseCode.aidl and
+            // authorization/ResponseCode.aidl share the same integer values for the
+            // common response codes, this may deviate in the future, hence the
+            // conversion here.
+            KsResponseCode::SYSTEM_ERROR => ResponseCode::SYSTEM_ERROR.0,
+            KsResponseCode::KEY_NOT_FOUND => ResponseCode::KEY_NOT_FOUND.0,
+            KsResponseCode::VALUE_CORRUPTED => ResponseCode::VALUE_CORRUPTED.0,
+            KsResponseCode::INVALID_ARGUMENT => ResponseCode::INVALID_ARGUMENT.0,
+            // If the code paths of IKeystoreAuthorization aidl's methods happen to return
+            // other error codes from KsResponseCode in the future, they should be converted
+            // as well.
+            _ => ResponseCode::SYSTEM_ERROR.0,
+        };
+        BinderStatus::new_service_specific_error(rc, anyhow_error_to_cstring(&e).as_deref())
+    } else {
+        let rc = match root_cause.downcast_ref::<Error>() {
+            Some(Error::Rc(rcode)) => rcode.0,
+            Some(Error::Binder(_, _)) => ResponseCode::SYSTEM_ERROR.0,
+            None => match root_cause.downcast_ref::<selinux::Error>() {
+                Some(selinux::Error::PermissionDenied) => ResponseCode::PERMISSION_DENIED.0,
                 _ => ResponseCode::SYSTEM_ERROR.0,
-            };
-            BinderStatus::new_service_specific_error(rc, anyhow_error_to_cstring(&e).as_deref())
-        } else {
-            let rc = match root_cause.downcast_ref::<Error>() {
-                Some(Error::Rc(rcode)) => rcode.0,
-                Some(Error::Binder(_, _)) => ResponseCode::SYSTEM_ERROR.0,
-                None => match root_cause.downcast_ref::<selinux::Error>() {
-                    Some(selinux::Error::PermissionDenied) => ResponseCode::PERMISSION_DENIED.0,
-                    _ => ResponseCode::SYSTEM_ERROR.0,
-                },
-            };
-            BinderStatus::new_service_specific_error(rc, anyhow_error_to_cstring(&e).as_deref())
-        }
-    })
+            },
+        };
+        BinderStatus::new_service_specific_error(rc, anyhow_error_to_cstring(&e).as_deref())
+    }
 }
 
 /// This struct is defined to implement the aforementioned AIDL interface.
@@ -257,12 +252,12 @@
 impl IKeystoreAuthorization for AuthorizationManager {
     fn addAuthToken(&self, auth_token: &HardwareAuthToken) -> BinderResult<()> {
         let _wp = wd::watch("IKeystoreAuthorization::addAuthToken");
-        map_or_log_err(self.add_auth_token(auth_token))
+        self.add_auth_token(auth_token).map_err(into_logged_binder)
     }
 
     fn onDeviceUnlocked(&self, user_id: i32, password: Option<&[u8]>) -> BinderResult<()> {
         let _wp = wd::watch("IKeystoreAuthorization::onDeviceUnlocked");
-        map_or_log_err(self.on_device_unlocked(user_id, password.map(|pw| pw.into())))
+        self.on_device_unlocked(user_id, password.map(|pw| pw.into())).map_err(into_logged_binder)
     }
 
     fn onDeviceLocked(
@@ -272,17 +267,18 @@
         weak_unlock_enabled: bool,
     ) -> BinderResult<()> {
         let _wp = wd::watch("IKeystoreAuthorization::onDeviceLocked");
-        map_or_log_err(self.on_device_locked(user_id, unlocking_sids, weak_unlock_enabled))
+        self.on_device_locked(user_id, unlocking_sids, weak_unlock_enabled)
+            .map_err(into_logged_binder)
     }
 
     fn onWeakUnlockMethodsExpired(&self, user_id: i32) -> BinderResult<()> {
         let _wp = wd::watch("IKeystoreAuthorization::onWeakUnlockMethodsExpired");
-        map_or_log_err(self.on_weak_unlock_methods_expired(user_id))
+        self.on_weak_unlock_methods_expired(user_id).map_err(into_logged_binder)
     }
 
     fn onNonLskfUnlockMethodsExpired(&self, user_id: i32) -> BinderResult<()> {
         let _wp = wd::watch("IKeystoreAuthorization::onNonLskfUnlockMethodsExpired");
-        map_or_log_err(self.on_non_lskf_unlock_methods_expired(user_id))
+        self.on_non_lskf_unlock_methods_expired(user_id).map_err(into_logged_binder)
     }
 
     fn getAuthTokensForCredStore(
@@ -292,11 +288,8 @@
         auth_token_max_age_millis: i64,
     ) -> binder::Result<AuthorizationTokens> {
         let _wp = wd::watch("IKeystoreAuthorization::getAuthTokensForCredStore");
-        map_or_log_err(self.get_auth_tokens_for_credstore(
-            challenge,
-            secure_user_id,
-            auth_token_max_age_millis,
-        ))
+        self.get_auth_tokens_for_credstore(challenge, secure_user_id, auth_token_max_age_millis)
+            .map_err(into_logged_binder)
     }
 
     fn getLastAuthTime(
@@ -305,7 +298,7 @@
         auth_types: &[HardwareAuthenticatorType],
     ) -> binder::Result<i64> {
         if aconfig_android_hardware_biometrics_rust::last_authentication_time() {
-            map_or_log_err(self.get_last_auth_time(secure_user_id, auth_types))
+            self.get_last_auth_time(secure_user_id, auth_types).map_err(into_logged_binder)
         } else {
             Err(BinderStatus::new_service_specific_error(
                 ResponseCode::PERMISSION_DENIED.0,
diff --git a/keystore2/src/error.rs b/keystore2/src/error.rs
index b9a2291..cea4d6b 100644
--- a/keystore2/src/error.rs
+++ b/keystore2/src/error.rs
@@ -21,8 +21,8 @@
 //!
 //! `SerializedError` is used send error codes on the wire.
 //!
-//! `map_or_log_err` is a convenience method used to convert `anyhow::Error` into `SerializedError`
-//! wire type.
+//! `into_[logged_]binder` is a convenience method used to convert `anyhow::Error` into
+//! `SerializedError` wire type.
 //!
 //! Keystore functions should use `anyhow::Result` to return error conditions, and context should
 //! be added every time an error is forwarded.
@@ -128,14 +128,11 @@
                     // Non negative error codes cannot be KM error codes.
                     // So we create an `Error::Binder` variant to preserve
                     // the service specific error code for logging.
-                    // `map_or_log_err` will map this on a system error,
-                    // but not before logging the details to logcat.
                     Error::Binder(ExceptionCode::SERVICE_SPECIFIC, se)
                 }
             }
             // We create `Error::Binder` to preserve the exception code
             // for logging.
-            // `map_or_log_err` will map this on a system error.
             e_code => Error::Binder(e_code, 0),
         }
     })
@@ -163,21 +160,17 @@
     r.map_err(Error::BinderTransaction)
 }
 
-/// This function should be used by Keystore service calls to translate error conditions
-/// into service specific exceptions.
-///
-/// All error conditions get logged by this function, except for KEY_NOT_FOUND error.
-pub fn map_or_log_err<T>(result: anyhow::Result<T>) -> BinderResult<T> {
-    map_err_with(result, |e| {
-        // Make the key not found errors silent.
-        if !matches!(
-            e.root_cause().downcast_ref::<Error>(),
-            Some(Error::Rc(ResponseCode::KEY_NOT_FOUND))
-        ) {
-            log::error!("{:?}", e);
-        }
-        e
-    })
+/// Convert an [`anyhow::Error`] to a [`binder::Status`], logging the value
+/// along the way (except if it is `KEY_NOT_FOUND`).
+pub fn into_logged_binder(e: anyhow::Error) -> BinderStatus {
+    // Log everything except key not found.
+    if !matches!(
+        e.root_cause().downcast_ref::<Error>(),
+        Some(Error::Rc(ResponseCode::KEY_NOT_FOUND))
+    ) {
+        log::error!("{:?}", e);
+    }
+    into_binder(e)
 }
 
 /// This function turns an anyhow error into an optional CString.
@@ -194,18 +187,10 @@
     }
 }
 
-/// This function behaves similar to map_or_log_error, but it does not log the errors, instead
-/// it calls map_err on the error before mapping it to a binder result allowing callers to
-/// log or transform the error before mapping it.
-pub fn map_err_with<T, F1>(result: anyhow::Result<T>, map_err: F1) -> BinderResult<T>
-where
-    F1: FnOnce(anyhow::Error) -> anyhow::Error,
-{
-    result.map_err(|e| {
-        let e = map_err(e);
-        let rc = anyhow_error_to_serialized_error(&e);
-        BinderStatus::new_service_specific_error(rc.0, anyhow_error_to_cstring(&e).as_deref())
-    })
+/// Convert an [`anyhow::Error`] to a [`binder::Status`].
+pub fn into_binder(e: anyhow::Error) -> binder::Status {
+    let rc = anyhow_error_to_serialized_error(&e);
+    BinderStatus::new_service_specific_error(rc.0, anyhow_error_to_cstring(&e).as_deref())
 }
 
 /// This type is used to send error codes on the wire.
@@ -323,7 +308,9 @@
         for rc in ResponseCode::LOCKED.0..ResponseCode::BACKEND_BUSY.0 {
             assert_eq!(
                 Result::<(), i32>::Err(rc),
-                map_or_log_err(nested_rc(ResponseCode(rc))).map_err(|s| s.service_specific_error())
+                nested_rc(ResponseCode(rc))
+                    .map_err(into_logged_binder)
+                    .map_err(|s| s.service_specific_error())
             );
         }
 
@@ -332,7 +319,9 @@
         for ec in ErrorCode::UNKNOWN_ERROR.0..ErrorCode::ROOT_OF_TRUST_ALREADY_SET.0 {
             assert_eq!(
                 Result::<(), i32>::Err(ec),
-                map_or_log_err(nested_ec(ErrorCode(ec))).map_err(|s| s.service_specific_error())
+                nested_ec(ErrorCode(ec))
+                    .map_err(into_logged_binder)
+                    .map_err(|s| s.service_specific_error())
             );
         }
 
@@ -341,11 +330,10 @@
         for ec in ErrorCode::UNKNOWN_ERROR.0..ErrorCode::ROOT_OF_TRUST_ALREADY_SET.0 {
             assert_eq!(
                 Result::<(), i32>::Err(ec),
-                map_or_log_err(
-                    map_km_error(binder_sse_error(ec))
-                        .with_context(|| format!("Km error code: {}.", ec))
-                )
-                .map_err(|s| s.service_specific_error())
+                map_km_error(binder_sse_error(ec))
+                    .with_context(|| format!("Km error code: {}.", ec))
+                    .map_err(into_logged_binder)
+                    .map_err(|s| s.service_specific_error())
             );
         }
 
@@ -354,42 +342,50 @@
         // service specific error.
         let sse = map_km_error(binder_sse_error(1));
         assert_eq!(Err(Error::Binder(ExceptionCode::SERVICE_SPECIFIC, 1)), sse);
-        // map_or_log_err then maps it on a service specific error of ResponseCode::SYSTEM_ERROR.
+        // into_binder then maps it on a service specific error of ResponseCode::SYSTEM_ERROR.
         assert_eq!(
             Result::<(), ResponseCode>::Err(ResponseCode::SYSTEM_ERROR),
-            map_or_log_err(sse.context("Non negative service specific error."))
+            sse.context("Non negative service specific error.")
+                .map_err(into_logged_binder)
                 .map_err(|s| ResponseCode(s.service_specific_error()))
         );
 
         // map_km_error creates a Error::Binder variant storing the given exception code.
         let binder_exception = map_km_error(binder_exception(ExceptionCode::TRANSACTION_FAILED));
         assert_eq!(Err(Error::Binder(ExceptionCode::TRANSACTION_FAILED, 0)), binder_exception);
-        // map_or_log_err then maps it on a service specific error of ResponseCode::SYSTEM_ERROR.
+        // into_binder then maps it on a service specific error of ResponseCode::SYSTEM_ERROR.
         assert_eq!(
             Result::<(), ResponseCode>::Err(ResponseCode::SYSTEM_ERROR),
-            map_or_log_err(binder_exception.context("Binder Exception."))
+            binder_exception
+                .context("Binder Exception.")
+                .map_err(into_logged_binder)
                 .map_err(|s| ResponseCode(s.service_specific_error()))
         );
 
         // selinux::Error::Perm() needs to be mapped to ResponseCode::PERMISSION_DENIED
         assert_eq!(
             Result::<(), ResponseCode>::Err(ResponseCode::PERMISSION_DENIED),
-            map_or_log_err(nested_selinux_perm())
+            nested_selinux_perm()
+                .map_err(into_logged_binder)
                 .map_err(|s| ResponseCode(s.service_specific_error()))
         );
 
         // All other errors get mapped on System Error.
         assert_eq!(
             Result::<(), ResponseCode>::Err(ResponseCode::SYSTEM_ERROR),
-            map_or_log_err(nested_other_error())
+            nested_other_error()
+                .map_err(into_logged_binder)
                 .map_err(|s| ResponseCode(s.service_specific_error()))
         );
 
         // Result::Ok variants get passed to the ok handler.
-        assert_eq!(Ok(ResponseCode::LOCKED), map_or_log_err(nested_ok(ResponseCode::LOCKED)));
+        assert_eq!(
+            Ok(ResponseCode::LOCKED),
+            nested_ok(ResponseCode::LOCKED).map_err(into_logged_binder)
+        );
         assert_eq!(
             Ok(ResponseCode::SYSTEM_ERROR),
-            map_or_log_err(nested_ok(ResponseCode::SYSTEM_ERROR))
+            nested_ok(ResponseCode::SYSTEM_ERROR).map_err(into_logged_binder)
         );
 
         Ok(())
diff --git a/keystore2/src/km_compat.rs b/keystore2/src/km_compat.rs
index 02bcb1a..5e3bdfa 100644
--- a/keystore2/src/km_compat.rs
+++ b/keystore2/src/km_compat.rs
@@ -16,7 +16,7 @@
 //! be emulated on back-level devices.
 
 use crate::ks_err;
-use crate::error::{map_binder_status, map_binder_status_code, map_or_log_err, Error, ErrorCode};
+use crate::error::{map_binder_status, map_binder_status_code, into_logged_binder, Error, ErrorCode};
 use android_hardware_security_keymint::binder::{BinderFeatures, StatusCode, Strong};
 use android_hardware_security_secureclock::aidl::android::hardware::security::secureclock::TimeStampToken::TimeStampToken;
 use android_hardware_security_keymint::aidl::android::hardware::security::keymint::{
@@ -226,7 +226,7 @@
     ) -> binder::Result<KeyCreationResult> {
         if self.emu.emulation_required(key_params, &KeyImportData::None) {
             let mut result = self.soft.generateKey(key_params, attestation_key)?;
-            result.keyBlob = map_or_log_err(wrap_keyblob(&result.keyBlob))?;
+            result.keyBlob = wrap_keyblob(&result.keyBlob).map_err(into_logged_binder)?;
             Ok(result)
         } else {
             self.real.generateKey(key_params, attestation_key)
@@ -242,7 +242,7 @@
         if self.emu.emulation_required(key_params, &KeyImportData::new(key_format, key_data)?) {
             let mut result =
                 self.soft.importKey(key_params, key_format, key_data, attestation_key)?;
-            result.keyBlob = map_or_log_err(wrap_keyblob(&result.keyBlob))?;
+            result.keyBlob = wrap_keyblob(&result.keyBlob).map_err(into_logged_binder)?;
             Ok(result)
         } else {
             self.real.importKey(key_params, key_format, key_data, attestation_key)
@@ -281,7 +281,7 @@
             KeyBlob::Wrapped(keyblob) => {
                 // Re-wrap the upgraded keyblob.
                 let upgraded_keyblob = self.soft.upgradeKey(keyblob, upgrade_params)?;
-                map_or_log_err(wrap_keyblob(&upgraded_keyblob))
+                wrap_keyblob(&upgraded_keyblob).map_err(into_logged_binder)
             }
         }
     }
diff --git a/keystore2/src/maintenance.rs b/keystore2/src/maintenance.rs
index c384a03..ba92399 100644
--- a/keystore2/src/maintenance.rs
+++ b/keystore2/src/maintenance.rs
@@ -15,8 +15,8 @@
 //! This module implements IKeystoreMaintenance AIDL interface.
 
 use crate::database::{KeyEntryLoadBits, KeyType};
+use crate::error::into_logged_binder;
 use crate::error::map_km_error;
-use crate::error::map_or_log_err;
 use crate::error::Error;
 use crate::globals::get_keymint_device;
 use crate::globals::{DB, LEGACY_IMPORTER, SUPER_KEY};
@@ -303,13 +303,14 @@
             password.is_some()
         );
         let _wp = wd::watch("IKeystoreMaintenance::onUserPasswordChanged");
-        map_or_log_err(Self::on_user_password_changed(user_id, password.map(|pw| pw.into())))
+        Self::on_user_password_changed(user_id, password.map(|pw| pw.into()))
+            .map_err(into_logged_binder)
     }
 
     fn onUserAdded(&self, user_id: i32) -> BinderResult<()> {
         log::info!("onUserAdded(user={user_id})");
         let _wp = wd::watch("IKeystoreMaintenance::onUserAdded");
-        map_or_log_err(self.add_or_remove_user(user_id))
+        self.add_or_remove_user(user_id).map_err(into_logged_binder)
     }
 
     fn initUserSuperKeys(
@@ -320,31 +321,32 @@
     ) -> BinderResult<()> {
         log::info!("initUserSuperKeys(user={user_id}, allow_existing={allow_existing})");
         let _wp = wd::watch("IKeystoreMaintenance::initUserSuperKeys");
-        map_or_log_err(self.init_user_super_keys(user_id, password.into(), allow_existing))
+        self.init_user_super_keys(user_id, password.into(), allow_existing)
+            .map_err(into_logged_binder)
     }
 
     fn onUserRemoved(&self, user_id: i32) -> BinderResult<()> {
         log::info!("onUserRemoved(user={user_id})");
         let _wp = wd::watch("IKeystoreMaintenance::onUserRemoved");
-        map_or_log_err(self.add_or_remove_user(user_id))
+        self.add_or_remove_user(user_id).map_err(into_logged_binder)
     }
 
     fn onUserLskfRemoved(&self, user_id: i32) -> BinderResult<()> {
         log::info!("onUserLskfRemoved(user={user_id})");
         let _wp = wd::watch("IKeystoreMaintenance::onUserLskfRemoved");
-        map_or_log_err(Self::on_user_lskf_removed(user_id))
+        Self::on_user_lskf_removed(user_id).map_err(into_logged_binder)
     }
 
     fn clearNamespace(&self, domain: Domain, nspace: i64) -> BinderResult<()> {
         log::info!("clearNamespace({domain:?}, nspace={nspace})");
         let _wp = wd::watch("IKeystoreMaintenance::clearNamespace");
-        map_or_log_err(self.clear_namespace(domain, nspace))
+        self.clear_namespace(domain, nspace).map_err(into_logged_binder)
     }
 
     fn earlyBootEnded(&self) -> BinderResult<()> {
         log::info!("earlyBootEnded()");
         let _wp = wd::watch("IKeystoreMaintenance::earlyBootEnded");
-        map_or_log_err(Self::early_boot_ended())
+        Self::early_boot_ended().map_err(into_logged_binder)
     }
 
     fn migrateKeyNamespace(
@@ -354,13 +356,13 @@
     ) -> BinderResult<()> {
         log::info!("migrateKeyNamespace(src={source:?}, dest={destination:?})");
         let _wp = wd::watch("IKeystoreMaintenance::migrateKeyNamespace");
-        map_or_log_err(Self::migrate_key_namespace(source, destination))
+        Self::migrate_key_namespace(source, destination).map_err(into_logged_binder)
     }
 
     fn deleteAllKeys(&self) -> BinderResult<()> {
         log::warn!("deleteAllKeys()");
         let _wp = wd::watch("IKeystoreMaintenance::deleteAllKeys");
-        map_or_log_err(Self::delete_all_keys())
+        Self::delete_all_keys().map_err(into_logged_binder)
     }
 
     fn getAppUidsAffectedBySid(
@@ -370,6 +372,6 @@
     ) -> BinderResult<std::vec::Vec<i64>> {
         log::info!("getAppUidsAffectedBySid(secure_user_id={secure_user_id:?})");
         let _wp = wd::watch("IKeystoreMaintenance::getAppUidsAffectedBySid");
-        map_or_log_err(Self::get_app_uids_affected_by_sid(user_id, secure_user_id))
+        Self::get_app_uids_affected_by_sid(user_id, secure_user_id).map_err(into_logged_binder)
     }
 }
diff --git a/keystore2/src/metrics.rs b/keystore2/src/metrics.rs
index bbe12aa..4757739 100644
--- a/keystore2/src/metrics.rs
+++ b/keystore2/src/metrics.rs
@@ -14,7 +14,7 @@
 
 //! This module implements the IKeystoreMetrics AIDL interface, which exposes the API method for the
 //! proxy in the system server to pull the aggregated metrics in keystore.
-use crate::error::map_or_log_err;
+use crate::error::into_logged_binder;
 use crate::ks_err;
 use crate::metrics_store::METRICS_STORE;
 use crate::permission::KeystorePerm;
@@ -52,6 +52,6 @@
 impl IKeystoreMetrics for Metrics {
     fn pullMetrics(&self, atom_id: AtomID) -> BinderResult<Vec<KeystoreAtom>> {
         let _wp = wd::watch("IKeystoreMetrics::pullMetrics");
-        map_or_log_err(self.pull_metrics(atom_id))
+        self.pull_metrics(atom_id).map_err(into_logged_binder)
     }
 }
diff --git a/keystore2/src/operation.rs b/keystore2/src/operation.rs
index 94bd7c3..7d988e1 100644
--- a/keystore2/src/operation.rs
+++ b/keystore2/src/operation.rs
@@ -127,7 +127,7 @@
 
 use crate::enforcements::AuthInfo;
 use crate::error::{
-    error_to_serialized_error, map_err_with, map_km_error, map_or_log_err, Error, ErrorCode,
+    error_to_serialized_error, into_binder, into_logged_binder, map_km_error, Error, ErrorCode,
     ResponseCode, SerializedError,
 };
 use crate::ks_err;
@@ -822,18 +822,20 @@
 impl IKeystoreOperation for KeystoreOperation {
     fn updateAad(&self, aad_input: &[u8]) -> binder::Result<()> {
         let _wp = wd::watch("IKeystoreOperation::updateAad");
-        map_or_log_err(self.with_locked_operation(
+        self.with_locked_operation(
             |op| op.update_aad(aad_input).context(ks_err!("KeystoreOperation::updateAad")),
             false,
-        ))
+        )
+        .map_err(into_logged_binder)
     }
 
     fn update(&self, input: &[u8]) -> binder::Result<Option<Vec<u8>>> {
         let _wp = wd::watch("IKeystoreOperation::update");
-        map_or_log_err(self.with_locked_operation(
+        self.with_locked_operation(
             |op| op.update(input).context(ks_err!("KeystoreOperation::update")),
             false,
-        ))
+        )
+        .map_err(into_logged_binder)
     }
     fn finish(
         &self,
@@ -841,29 +843,28 @@
         signature: Option<&[u8]>,
     ) -> binder::Result<Option<Vec<u8>>> {
         let _wp = wd::watch("IKeystoreOperation::finish");
-        map_or_log_err(self.with_locked_operation(
+        self.with_locked_operation(
             |op| op.finish(input, signature).context(ks_err!("KeystoreOperation::finish")),
             true,
-        ))
+        )
+        .map_err(into_logged_binder)
     }
 
     fn abort(&self) -> binder::Result<()> {
         let _wp = wd::watch("IKeystoreOperation::abort");
-        map_err_with(
-            self.with_locked_operation(
-                |op| op.abort(Outcome::Abort).context(ks_err!("KeystoreOperation::abort")),
-                true,
-            ),
-            |e| {
-                match e.root_cause().downcast_ref::<Error>() {
-                    // Calling abort on expired operations is something very common.
-                    // There is no reason to clutter the log with it. It is never the cause
-                    // for a true problem.
-                    Some(Error::Km(ErrorCode::INVALID_OPERATION_HANDLE)) => {}
-                    _ => log::error!("{:?}", e),
-                };
-                e
-            },
-        )
+        let result = self.with_locked_operation(
+            |op| op.abort(Outcome::Abort).context(ks_err!("KeystoreOperation::abort")),
+            true,
+        );
+        result.map_err(|e| {
+            match e.root_cause().downcast_ref::<Error>() {
+                // Calling abort on expired operations is something very common.
+                // There is no reason to clutter the log with it. It is never the cause
+                // for a true problem.
+                Some(Error::Km(ErrorCode::INVALID_OPERATION_HANDLE)) => {}
+                _ => log::error!("{:?}", e),
+            };
+            into_binder(e)
+        })
     }
 }
diff --git a/keystore2/src/security_level.rs b/keystore2/src/security_level.rs
index 71d6dba..00e0480 100644
--- a/keystore2/src/security_level.rs
+++ b/keystore2/src/security_level.rs
@@ -20,7 +20,7 @@
 };
 use crate::database::{BlobInfo, CertificateInfo, KeyIdGuard};
 use crate::error::{
-    self, map_km_error, map_or_log_err, wrapped_rkpd_error_to_ks_error, Error, ErrorCode,
+    self, into_logged_binder, map_km_error, wrapped_rkpd_error_to_ks_error, Error, ErrorCode,
 };
 use crate::globals::{
     get_remotely_provisioned_component_name, DB, ENFORCEMENTS, LEGACY_IMPORTER, SUPER_KEY,
@@ -996,7 +996,7 @@
         forced: bool,
     ) -> binder::Result<CreateOperationResponse> {
         let _wp = self.watch("IKeystoreSecurityLevel::createOperation");
-        map_or_log_err(self.create_operation(key, operation_parameters, forced))
+        self.create_operation(key, operation_parameters, forced).map_err(into_logged_binder)
     }
     fn generateKey(
         &self,
@@ -1012,7 +1012,7 @@
         let result = self.generate_key(key, attestation_key, params, flags, entropy);
         log_key_creation_event_stats(self.security_level, params, &result);
         log_key_generated(key, ThreadState::get_calling_uid(), result.is_ok());
-        map_or_log_err(result)
+        result.map_err(into_logged_binder)
     }
     fn importKey(
         &self,
@@ -1026,7 +1026,7 @@
         let result = self.import_key(key, attestation_key, params, flags, key_data);
         log_key_creation_event_stats(self.security_level, params, &result);
         log_key_imported(key, ThreadState::get_calling_uid(), result.is_ok());
-        map_or_log_err(result)
+        result.map_err(into_logged_binder)
     }
     fn importWrappedKey(
         &self,
@@ -1041,20 +1041,20 @@
             self.import_wrapped_key(key, wrapping_key, masking_key, params, authenticators);
         log_key_creation_event_stats(self.security_level, params, &result);
         log_key_imported(key, ThreadState::get_calling_uid(), result.is_ok());
-        map_or_log_err(result)
+        result.map_err(into_logged_binder)
     }
     fn convertStorageKeyToEphemeral(
         &self,
         storage_key: &KeyDescriptor,
     ) -> binder::Result<EphemeralStorageKeyResponse> {
         let _wp = self.watch("IKeystoreSecurityLevel::convertStorageKeyToEphemeral");
-        map_or_log_err(self.convert_storage_key_to_ephemeral(storage_key))
+        self.convert_storage_key_to_ephemeral(storage_key).map_err(into_logged_binder)
     }
     fn deleteKey(&self, key: &KeyDescriptor) -> binder::Result<()> {
         let _wp = self.watch("IKeystoreSecurityLevel::deleteKey");
         let result = self.delete_key(key);
         log_key_deleted(key, ThreadState::get_calling_uid(), result.is_ok());
-        map_or_log_err(result)
+        result.map_err(into_logged_binder)
     }
 }
 
diff --git a/keystore2/src/service.rs b/keystore2/src/service.rs
index 2ca9ccf..3726358 100644
--- a/keystore2/src/service.rs
+++ b/keystore2/src/service.rs
@@ -35,7 +35,7 @@
     error::ResponseCode,
 };
 use crate::{
-    error::{self, map_or_log_err, ErrorCode},
+    error::{self, into_logged_binder, ErrorCode},
     id_rotation::IdRotationState,
 };
 use android_hardware_security_keymint::aidl::android::hardware::security::keymint::SecurityLevel::SecurityLevel;
@@ -384,11 +384,11 @@
         let _wp = wd::watch_millis_with("IKeystoreService::getSecurityLevel", 500, move || {
             format!("security_level: {}", security_level.0)
         });
-        map_or_log_err(self.get_security_level(security_level))
+        self.get_security_level(security_level).map_err(into_logged_binder)
     }
     fn getKeyEntry(&self, key: &KeyDescriptor) -> binder::Result<KeyEntryResponse> {
         let _wp = wd::watch("IKeystoreService::get_key_entry");
-        map_or_log_err(self.get_key_entry(key))
+        self.get_key_entry(key).map_err(into_logged_binder)
     }
     fn updateSubcomponent(
         &self,
@@ -397,17 +397,17 @@
         certificate_chain: Option<&[u8]>,
     ) -> binder::Result<()> {
         let _wp = wd::watch("IKeystoreService::updateSubcomponent");
-        map_or_log_err(self.update_subcomponent(key, public_cert, certificate_chain))
+        self.update_subcomponent(key, public_cert, certificate_chain).map_err(into_logged_binder)
     }
     fn listEntries(&self, domain: Domain, namespace: i64) -> binder::Result<Vec<KeyDescriptor>> {
         let _wp = wd::watch("IKeystoreService::listEntries");
-        map_or_log_err(self.list_entries(domain, namespace))
+        self.list_entries(domain, namespace).map_err(into_logged_binder)
     }
     fn deleteKey(&self, key: &KeyDescriptor) -> binder::Result<()> {
         let _wp = wd::watch("IKeystoreService::deleteKey");
         let result = self.delete_key(key);
         log_key_deleted(key, ThreadState::get_calling_uid(), result.is_ok());
-        map_or_log_err(result)
+        result.map_err(into_logged_binder)
     }
     fn grant(
         &self,
@@ -416,11 +416,11 @@
         access_vector: i32,
     ) -> binder::Result<KeyDescriptor> {
         let _wp = wd::watch("IKeystoreService::grant");
-        map_or_log_err(self.grant(key, grantee_uid, access_vector.into()))
+        self.grant(key, grantee_uid, access_vector.into()).map_err(into_logged_binder)
     }
     fn ungrant(&self, key: &KeyDescriptor, grantee_uid: i32) -> binder::Result<()> {
         let _wp = wd::watch("IKeystoreService::ungrant");
-        map_or_log_err(self.ungrant(key, grantee_uid))
+        self.ungrant(key, grantee_uid).map_err(into_logged_binder)
     }
     fn listEntriesBatched(
         &self,
@@ -429,11 +429,11 @@
         start_past_alias: Option<&str>,
     ) -> binder::Result<Vec<KeyDescriptor>> {
         let _wp = wd::watch("IKeystoreService::listEntriesBatched");
-        map_or_log_err(self.list_entries_batched(domain, namespace, start_past_alias))
+        self.list_entries_batched(domain, namespace, start_past_alias).map_err(into_logged_binder)
     }
 
     fn getNumberOfEntries(&self, domain: Domain, namespace: i64) -> binder::Result<i32> {
         let _wp = wd::watch("IKeystoreService::getNumberOfEntries");
-        map_or_log_err(self.count_num_entries(domain, namespace))
+        self.count_num_entries(domain, namespace).map_err(into_logged_binder)
     }
 }