Assorted refactoring
Mostly removing duplication. Also added a comment as suggested by
Andrew.
Bug: 161471326
Test: composd_cmd async-odrefresh
Change-Id: I7da83cb825de7b4fd2267b1e496808a423544c16
diff --git a/compos/common/Android.bp b/compos/common/Android.bp
index 7c61d94..39e7c0a 100644
--- a/compos/common/Android.bp
+++ b/compos/common/Android.bp
@@ -11,6 +11,7 @@
"android.system.virtualizationservice-rust",
"compos_aidl_interface-rust",
"libanyhow",
+ "libbinder_common",
"libbinder_rpc_unstable_bindgen",
"libbinder_rs",
"liblog_rust",
diff --git a/compos/composd/src/util.rs b/compos/common/binder.rs
similarity index 62%
rename from compos/composd/src/util.rs
rename to compos/common/binder.rs
index 54d7751..6bd3957 100644
--- a/compos/composd/src/util.rs
+++ b/compos/common/binder.rs
@@ -14,16 +14,21 @@
* limitations under the License.
*/
-use android_system_composd::binder::Result as BinderResult;
+//! Helper for converting Error types to what Binder expects
+
use anyhow::Result;
-use binder_common::new_binder_service_specific_error;
-use log::error;
+use binder::public_api::{ExceptionCode, Result as BinderResult};
+use binder_common::new_binder_exception;
+use log::warn;
use std::fmt::Debug;
+/// Convert a Result<T, E> to BinderResult<T> to allow it to be returned from a binder RPC,
+/// preserving the content as far as possible.
+/// Also log the error if there is one.
pub fn to_binder_result<T, E: Debug>(result: Result<T, E>) -> BinderResult<T> {
result.map_err(|e| {
let message = format!("{:?}", e);
- error!("Returning binder error: {}", &message);
- new_binder_service_specific_error(-1, message)
+ warn!("Returning binder error: {}", &message);
+ new_binder_exception(ExceptionCode::SERVICE_SPECIFIC, message)
})
}
diff --git a/compos/common/lib.rs b/compos/common/lib.rs
index 5c28379..6e397a2 100644
--- a/compos/common/lib.rs
+++ b/compos/common/lib.rs
@@ -16,6 +16,7 @@
//! Common items used by CompOS server and/or clients
+pub mod binder;
pub mod compos_client;
pub mod odrefresh;
pub mod timeouts;
diff --git a/compos/common/odrefresh.rs b/compos/common/odrefresh.rs
index 7838b69..7fe6ed5 100644
--- a/compos/common/odrefresh.rs
+++ b/compos/common/odrefresh.rs
@@ -16,12 +16,15 @@
//! Helpers for running odrefresh
+use anyhow::{anyhow, Result};
use num_derive::FromPrimitive;
use num_traits::FromPrimitive;
/// The path to the odrefresh binary
pub const ODREFRESH_PATH: &str = "/apex/com.android.art/bin/odrefresh";
+// The highest "standard" exit code defined in sysexits.h (as EX__MAX); odrefresh error codes
+// start above here to avoid clashing.
// TODO: What if this changes?
const EX_MAX: i8 = 78;
@@ -30,7 +33,7 @@
#[repr(i8)]
pub enum ExitCode {
/// No compilation required, all artifacts look good
- Okay = 0i8,
+ Okay = 0,
/// Compilation required
CompilationRequired = EX_MAX + 1,
/// New artifacts successfully generated
@@ -43,7 +46,8 @@
impl ExitCode {
/// Map an integer to the corresponding ExitCode enum, if there is one
- pub fn from_i32(exit_code: i32) -> Option<Self> {
+ pub fn from_i32(exit_code: i32) -> Result<Self> {
FromPrimitive::from_i32(exit_code)
+ .ok_or_else(|| anyhow!("Unexpected odrefresh exit code: {}", exit_code))
}
}
diff --git a/compos/composd/src/composd_main.rs b/compos/composd/src/composd_main.rs
index 2915a58..6f1e951 100644
--- a/compos/composd/src/composd_main.rs
+++ b/compos/composd/src/composd_main.rs
@@ -26,7 +26,6 @@
mod odrefresh;
mod odrefresh_task;
mod service;
-mod util;
use crate::instance_manager::InstanceManager;
use android_system_composd::binder::{register_lazy_service, ProcessState};
diff --git a/compos/composd/src/internal_service.rs b/compos/composd/src/internal_service.rs
index ebebaae..3fad6d4 100644
--- a/compos/composd/src/internal_service.rs
+++ b/compos/composd/src/internal_service.rs
@@ -17,7 +17,7 @@
//! Implementation of ICompilationInternal, called from odrefresh during compilation.
use crate::instance_manager::InstanceManager;
-use crate::util::to_binder_result;
+use compos_common::binder::to_binder_result;
use android_system_composd_internal::aidl::android::system::composd::internal::ICompilationInternal::{
BnCompilationInternal, ICompilationInternal,
};
diff --git a/compos/composd/src/odrefresh.rs b/compos/composd/src/odrefresh.rs
index f06a4b2..a6ca995 100644
--- a/compos/composd/src/odrefresh.rs
+++ b/compos/composd/src/odrefresh.rs
@@ -63,8 +63,8 @@
pub fn wait_for_exit(&self) -> Result<ExitCode> {
// No timeout here - but clients can kill the process, which will end the wait.
let status = self.child.wait()?;
- if let Some(exit_code) = status.code().and_then(ExitCode::from_i32) {
- Ok(exit_code)
+ if let Some(exit_code) = status.code() {
+ ExitCode::from_i32(exit_code)
} else {
bail!("odrefresh exited with {}", status)
}
diff --git a/compos/composd/src/odrefresh_task.rs b/compos/composd/src/odrefresh_task.rs
index b7de479..56b697e 100644
--- a/compos/composd/src/odrefresh_task.rs
+++ b/compos/composd/src/odrefresh_task.rs
@@ -22,7 +22,7 @@
ICompilationTask::ICompilationTask, ICompilationTaskCallback::ICompilationTaskCallback,
};
use android_system_composd::binder::{Interface, Result as BinderResult, Strong};
-use anyhow::{bail, Context, Result};
+use anyhow::{Context, Result};
use compos_aidl_interface::aidl::com::android::compos::ICompOsService::ICompOsService;
use compos_common::odrefresh::ExitCode;
use log::{error, warn};
@@ -140,11 +140,7 @@
)?;
drop(fd_server_raii);
- if let Some(exit_code) = ExitCode::from_i32(exit_code.into()) {
- Ok(exit_code)
- } else {
- bail!("odrefresh exited with {}", exit_code)
- }
+ ExitCode::from_i32(exit_code.into())
}
/// Returns an owned FD of the directory. It currently returns a `File` as a FD owner, but
diff --git a/compos/composd/src/service.rs b/compos/composd/src/service.rs
index 23c411b..69cf008 100644
--- a/compos/composd/src/service.rs
+++ b/compos/composd/src/service.rs
@@ -20,7 +20,6 @@
use crate::compilation_task::CompilationTask;
use crate::instance_manager::InstanceManager;
use crate::odrefresh_task::OdrefreshTask;
-use crate::util::to_binder_result;
use android_system_composd::aidl::android::system::composd::{
ICompilationTask::{BnCompilationTask, ICompilationTask},
ICompilationTaskCallback::ICompilationTaskCallback,
@@ -30,6 +29,7 @@
self, BinderFeatures, ExceptionCode, Interface, Status, Strong, ThreadState,
};
use anyhow::{Context, Result};
+use compos_common::binder::to_binder_result;
use rustutils::{users::AID_ROOT, users::AID_SYSTEM};
use std::sync::Arc;
diff --git a/compos/src/compilation.rs b/compos/src/compilation.rs
index 7eaae5d..2ca4dd4 100644
--- a/compos/src/compilation.rs
+++ b/compos/src/compilation.rs
@@ -148,8 +148,7 @@
}
}?;
- let exit_code = ExitCode::from_i32(exit_code.into())
- .ok_or_else(|| anyhow!("Unexpected odrefresh exit code: {}", exit_code))?;
+ let exit_code = ExitCode::from_i32(exit_code.into())?;
info!("odrefresh exited with {:?}", exit_code);
if exit_code == ExitCode::CompilationSuccess {
diff --git a/compos/src/compsvc.rs b/compos/src/compsvc.rs
index aa4b9bd..0d32841 100644
--- a/compos/src/compsvc.rs
+++ b/compos/src/compsvc.rs
@@ -18,8 +18,9 @@
//! file descriptors backed by authfs (via authfs_service) and pass the file descriptors to the
//! actual compiler.
-use anyhow::Result;
+use anyhow::{Context, Result};
use binder_common::new_binder_exception;
+use compos_common::binder::to_binder_result;
use log::warn;
use std::default::Default;
use std::env;
@@ -68,9 +69,7 @@
fsverity_digest: &fsverity::Sha256Digest,
) -> BinderResult<Vec<u8>> {
let formatted_digest = fsverity::to_formatted_digest(fsverity_digest);
- self.new_signer()?
- .sign(&formatted_digest[..])
- .map_err(|e| new_binder_exception(ExceptionCode::SERVICE_SPECIFIC, e.to_string()))
+ to_binder_result(self.new_signer()?.sign(&formatted_digest[..]))
}
fn new_signer(&self) -> BinderResult<Signer> {
@@ -117,26 +116,19 @@
target_dir_name: &str,
zygote_arch: &str,
) -> BinderResult<i8> {
- let context = OdrefreshContext::new(
+ let context = to_binder_result(OdrefreshContext::new(
system_dir_fd,
output_dir_fd,
staging_dir_fd,
target_dir_name,
zygote_arch,
- )
- .map_err(|e| new_binder_exception(ExceptionCode::ILLEGAL_ARGUMENT, e.to_string()))?;
+ ))?;
let authfs_service = get_authfs_service()?;
- let exit_code =
- odrefresh(&self.odrefresh_path, context, authfs_service, self.new_signer()?).map_err(
- |e| {
- warn!("odrefresh failed: {:?}", e);
- new_binder_exception(
- ExceptionCode::SERVICE_SPECIFIC,
- format!("odrefresh failed: {}", e),
- )
- },
- )?;
+ let exit_code = to_binder_result(
+ odrefresh(&self.odrefresh_path, context, authfs_service, self.new_signer()?)
+ .context("odrefresh failed"),
+ )?;
Ok(exit_code as i8)
}
@@ -146,13 +138,10 @@
fd_annotation: &FdAnnotation,
) -> BinderResult<CompilationResult> {
let authfs_service = get_authfs_service()?;
- let output =
- compile_cmd(&self.dex2oat_path, args, authfs_service, fd_annotation).map_err(|e| {
- new_binder_exception(
- ExceptionCode::SERVICE_SPECIFIC,
- format!("Compilation failed: {}", e),
- )
- })?;
+ let output = to_binder_result(
+ compile_cmd(&self.dex2oat_path, args, authfs_service, fd_annotation)
+ .context("Compilation failed"),
+ )?;
match output {
CompilerOutput::Digests { oat, vdex, image } => {
let oat_signature = self.generate_raw_fsverity_signature(&oat)?;
@@ -176,14 +165,12 @@
}
fn generateSigningKey(&self) -> BinderResult<CompOsKeyData> {
- self.key_service
- .generate()
- .map_err(|e| new_binder_exception(ExceptionCode::ILLEGAL_STATE, e.to_string()))
+ to_binder_result(self.key_service.generate())
}
fn verifySigningKey(&self, key_blob: &[u8], public_key: &[u8]) -> BinderResult<bool> {
Ok(if let Err(e) = self.key_service.verify(key_blob, public_key) {
- warn!("Signing key verification failed: {}", e.to_string());
+ warn!("Signing key verification failed: {:?}", e);
false
} else {
true
@@ -191,9 +178,7 @@
}
fn sign(&self, data: &[u8]) -> BinderResult<Vec<u8>> {
- self.new_signer()?
- .sign(data)
- .map_err(|e| new_binder_exception(ExceptionCode::ILLEGAL_STATE, e.to_string()))
+ to_binder_result(self.new_signer()?.sign(data))
}
}