Standardise safety comments for unsafe blocks, and add some more.
These will soon be required by a lint.
Bug: 290018030
Test: m vm virtmgr
Change-Id: Ifd034f53ef1009a312796a5282760c12762844ee
diff --git a/libs/binder/rust/rpcbinder/src/server.rs b/libs/binder/rust/rpcbinder/src/server.rs
index 81f68f5..6fda878 100644
--- a/libs/binder/rust/rpcbinder/src/server.rs
+++ b/libs/binder/rust/rpcbinder/src/server.rs
@@ -33,9 +33,9 @@
pub struct RpcServerRef;
}
-/// SAFETY - The opaque handle can be cloned freely.
+/// SAFETY: The opaque handle can be cloned freely.
unsafe impl Send for RpcServer {}
-/// SAFETY - The underlying C++ RpcServer class is thread-safe.
+/// SAFETY: The underlying C++ RpcServer class is thread-safe.
unsafe impl Sync for RpcServer {}
impl RpcServer {
@@ -59,7 +59,10 @@
/// Creates a binder RPC server, serving the supplied binder service implementation on the given
/// socket file descriptor. The socket should be bound to an address before calling this
/// function.
- pub fn new_bound_socket(mut service: SpIBinder, socket_fd: OwnedFd) -> Result<RpcServer, Error> {
+ pub fn new_bound_socket(
+ mut service: SpIBinder,
+ socket_fd: OwnedFd,
+ ) -> Result<RpcServer, Error> {
let service = service.as_native_mut();
// SAFETY: Service ownership is transferring to the server and won't be valid afterward.
@@ -67,7 +70,8 @@
// The server takes ownership of the socket FD.
unsafe {
Self::checked_from_ptr(binder_rpc_unstable_bindgen::ARpcServer_newBoundSocket(
- service, socket_fd.into_raw_fd(),
+ service,
+ socket_fd.into_raw_fd(),
))
}
}
@@ -120,7 +124,9 @@
if ptr.is_null() {
return Err(Error::new(ErrorKind::Other, "Failed to start server"));
}
- Ok(RpcServer::from_ptr(ptr))
+ // SAFETY: Our caller must pass us a valid or null pointer, and we've checked that it's not
+ // null.
+ Ok(unsafe { RpcServer::from_ptr(ptr) })
}
}
@@ -130,7 +136,7 @@
&self,
modes: &[FileDescriptorTransportMode],
) {
- // SAFETY - Does not keep the pointer after returning does, nor does it
+ // SAFETY: Does not keep the pointer after returning does, nor does it
// read past its boundary. Only passes the 'self' pointer as an opaque handle.
unsafe {
binder_rpc_unstable_bindgen::ARpcServer_setSupportedFileDescriptorTransportModes(
@@ -143,18 +149,21 @@
/// Starts a new background thread and calls join(). Returns immediately.
pub fn start(&self) {
+ // SAFETY: RpcServerRef wraps a valid pointer to an ARpcServer.
unsafe { binder_rpc_unstable_bindgen::ARpcServer_start(self.as_ptr()) };
}
/// Joins the RpcServer thread. The call blocks until the server terminates.
/// This must be called from exactly one thread.
pub fn join(&self) {
+ // SAFETY: RpcServerRef wraps a valid pointer to an ARpcServer.
unsafe { binder_rpc_unstable_bindgen::ARpcServer_join(self.as_ptr()) };
}
/// Shuts down the running RpcServer. Can be called multiple times and from
/// multiple threads. Called automatically during drop().
pub fn shutdown(&self) -> Result<(), Error> {
+ // SAFETY: RpcServerRef wraps a valid pointer to an ARpcServer.
if unsafe { binder_rpc_unstable_bindgen::ARpcServer_shutdown(self.as_ptr()) } {
Ok(())
} else {
diff --git a/libs/binder/rust/rpcbinder/src/session.rs b/libs/binder/rust/rpcbinder/src/session.rs
index 28c5390..79a9510 100644
--- a/libs/binder/rust/rpcbinder/src/session.rs
+++ b/libs/binder/rust/rpcbinder/src/session.rs
@@ -36,15 +36,15 @@
pub struct RpcSessionRef;
}
-/// SAFETY - The opaque handle can be cloned freely.
+/// SAFETY: The opaque handle can be cloned freely.
unsafe impl Send for RpcSession {}
-/// SAFETY - The underlying C++ RpcSession class is thread-safe.
+/// SAFETY: The underlying C++ RpcSession class is thread-safe.
unsafe impl Sync for RpcSession {}
impl RpcSession {
/// Allocates a new RpcSession object.
pub fn new() -> RpcSession {
- // SAFETY - Takes ownership of the returned handle, which has correct refcount.
+ // SAFETY: Takes ownership of the returned handle, which has correct refcount.
unsafe { RpcSession::from_ptr(binder_rpc_unstable_bindgen::ARpcSession_new()) }
}
}
@@ -58,7 +58,7 @@
impl RpcSessionRef {
/// Sets the file descriptor transport mode for this session.
pub fn set_file_descriptor_transport_mode(&self, mode: FileDescriptorTransportMode) {
- // SAFETY - Only passes the 'self' pointer as an opaque handle.
+ // SAFETY: Only passes the 'self' pointer as an opaque handle.
unsafe {
binder_rpc_unstable_bindgen::ARpcSession_setFileDescriptorTransportMode(
self.as_ptr(),
@@ -69,7 +69,7 @@
/// Sets the maximum number of incoming threads.
pub fn set_max_incoming_threads(&self, threads: usize) {
- // SAFETY - Only passes the 'self' pointer as an opaque handle.
+ // SAFETY: Only passes the 'self' pointer as an opaque handle.
unsafe {
binder_rpc_unstable_bindgen::ARpcSession_setMaxIncomingThreads(self.as_ptr(), threads)
};
@@ -77,7 +77,7 @@
/// Sets the maximum number of outgoing connections.
pub fn set_max_outgoing_connections(&self, connections: usize) {
- // SAFETY - Only passes the 'self' pointer as an opaque handle.
+ // SAFETY: Only passes the 'self' pointer as an opaque handle.
unsafe {
binder_rpc_unstable_bindgen::ARpcSession_setMaxOutgoingConnections(
self.as_ptr(),
@@ -210,10 +210,10 @@
type RequestFd<'a> = &'a mut dyn FnMut() -> Option<RawFd>;
unsafe extern "C" fn request_fd_wrapper(param: *mut c_void) -> c_int {
+ let request_fd_ptr = param as *mut RequestFd;
// SAFETY: This is only ever called by RpcPreconnectedClient, within the lifetime of the
// BinderFdFactory reference, with param being a properly aligned non-null pointer to an
// initialized instance.
- let request_fd_ptr = param as *mut RequestFd;
- let request_fd = request_fd_ptr.as_mut().unwrap();
+ let request_fd = unsafe { request_fd_ptr.as_mut().unwrap() };
request_fd().unwrap_or(-1)
}
diff --git a/libs/binder/rust/src/binder.rs b/libs/binder/rust/src/binder.rs
index 993bdca..5f7a044 100644
--- a/libs/binder/rust/src/binder.rs
+++ b/libs/binder/rust/src/binder.rs
@@ -300,15 +300,14 @@
/// [`declare_binder_interface!`].
pub fn new<I: InterfaceClassMethods>() -> InterfaceClass {
let descriptor = CString::new(I::get_descriptor()).unwrap();
+ // Safety: `AIBinder_Class_define` expects a valid C string, and three
+ // valid callback functions, all non-null pointers. The C string is
+ // copied and need not be valid for longer than the call, so we can drop
+ // it after the call. We can safely assign null to the onDump and
+ // handleShellCommand callbacks as long as the class pointer was
+ // non-null. Rust None for a Option<fn> is guaranteed to be a NULL
+ // pointer. Rust retains ownership of the pointer after it is defined.
let ptr = unsafe {
- // Safety: `AIBinder_Class_define` expects a valid C string, and
- // three valid callback functions, all non-null pointers. The C
- // string is copied and need not be valid for longer than the call,
- // so we can drop it after the call. We can safely assign null to
- // the onDump and handleShellCommand callbacks as long as the class
- // pointer was non-null. Rust None for a Option<fn> is guaranteed to
- // be a NULL pointer. Rust retains ownership of the pointer after it
- // is defined.
let class = sys::AIBinder_Class_define(
descriptor.as_ptr(),
Some(I::on_create),
@@ -338,13 +337,12 @@
/// Get the interface descriptor string of this class.
pub fn get_descriptor(&self) -> String {
+ // SAFETY: The descriptor returned by AIBinder_Class_getDescriptor is
+ // always a two-byte null terminated sequence of u16s. Thus, we can
+ // continue reading from the pointer until we hit a null value, and this
+ // pointer can be a valid slice if the slice length is <= the number of
+ // u16 elements before the null terminator.
unsafe {
- // SAFETY: The descriptor returned by AIBinder_Class_getDescriptor
- // is always a two-byte null terminated sequence of u16s. Thus, we
- // can continue reading from the pointer until we hit a null value,
- // and this pointer can be a valid slice if the slice length is <=
- // the number of u16 elements before the null terminator.
-
let raw_descriptor: *const c_char = sys::AIBinder_Class_getDescriptor(self.0);
CStr::from_ptr(raw_descriptor)
.to_str()
@@ -542,17 +540,15 @@
static CLASS_INIT: std::sync::Once = std::sync::Once::new();
static mut CLASS: Option<$crate::binder_impl::InterfaceClass> = None;
+ // Safety: This assignment is guarded by the `CLASS_INIT` `Once`
+ // variable, and therefore is thread-safe, as it can only occur
+ // once.
CLASS_INIT.call_once(|| unsafe {
- // Safety: This assignment is guarded by the `CLASS_INIT` `Once`
- // variable, and therefore is thread-safe, as it can only occur
- // once.
CLASS = Some($constructor);
});
- unsafe {
- // Safety: The `CLASS` variable can only be mutated once, above,
- // and is subsequently safe to read from any thread.
- CLASS.unwrap()
- }
+ // Safety: The `CLASS` variable can only be mutated once, above, and
+ // is subsequently safe to read from any thread.
+ unsafe { CLASS.unwrap() }
}
};
}
@@ -664,6 +660,8 @@
fn as_native_mut(&mut self) -> *mut T;
}
+// Safety: If V is a valid Android C++ type then we can either use that or a
+// null pointer.
unsafe impl<T, V: AsNative<T>> AsNative<T> for Option<V> {
fn as_native(&self) -> *const T {
self.as_ref().map_or(ptr::null(), |v| v.as_native())
@@ -924,15 +922,15 @@
static CLASS_INIT: std::sync::Once = std::sync::Once::new();
static mut CLASS: Option<$crate::binder_impl::InterfaceClass> = None;
+ // Safety: This assignment is guarded by the `CLASS_INIT` `Once`
+ // variable, and therefore is thread-safe, as it can only occur
+ // once.
CLASS_INIT.call_once(|| unsafe {
- // Safety: This assignment is guarded by the `CLASS_INIT` `Once`
- // variable, and therefore is thread-safe, as it can only occur
- // once.
CLASS = Some($crate::binder_impl::InterfaceClass::new::<$crate::binder_impl::Binder<$native>>());
});
+ // Safety: The `CLASS` variable can only be mutated once, above,
+ // and is subsequently safe to read from any thread.
unsafe {
- // Safety: The `CLASS` variable can only be mutated once, above,
- // and is subsequently safe to read from any thread.
CLASS.unwrap()
}
}
diff --git a/libs/binder/rust/src/error.rs b/libs/binder/rust/src/error.rs
index ba26062..8d9ce0e 100644
--- a/libs/binder/rust/src/error.rs
+++ b/libs/binder/rust/src/error.rs
@@ -112,41 +112,35 @@
impl Status {
/// Create a status object representing a successful transaction.
pub fn ok() -> Self {
- let ptr = unsafe {
- // Safety: `AStatus_newOk` always returns a new, heap allocated
- // pointer to an `ASTatus` object, so we know this pointer will be
- // valid.
- //
- // Rust takes ownership of the returned pointer.
- sys::AStatus_newOk()
- };
+ // Safety: `AStatus_newOk` always returns a new, heap allocated
+ // pointer to an `ASTatus` object, so we know this pointer will be
+ // valid.
+ //
+ // Rust takes ownership of the returned pointer.
+ let ptr = unsafe { sys::AStatus_newOk() };
Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer"))
}
/// Create a status object from a service specific error
pub fn new_service_specific_error(err: i32, message: Option<&CStr>) -> Status {
let ptr = if let Some(message) = message {
- unsafe {
- // Safety: Any i32 is a valid service specific error for the
- // error code parameter. We construct a valid, null-terminated
- // `CString` from the message, which must be a valid C-style
- // string to pass as the message. This function always returns a
- // new, heap allocated pointer to an `AStatus` object, so we
- // know the returned pointer will be valid.
- //
- // Rust takes ownership of the returned pointer.
- sys::AStatus_fromServiceSpecificErrorWithMessage(err, message.as_ptr())
- }
+ // Safety: Any i32 is a valid service specific error for the
+ // error code parameter. We construct a valid, null-terminated
+ // `CString` from the message, which must be a valid C-style
+ // string to pass as the message. This function always returns a
+ // new, heap allocated pointer to an `AStatus` object, so we
+ // know the returned pointer will be valid.
+ //
+ // Rust takes ownership of the returned pointer.
+ unsafe { sys::AStatus_fromServiceSpecificErrorWithMessage(err, message.as_ptr()) }
} else {
- unsafe {
- // Safety: Any i32 is a valid service specific error for the
- // error code parameter. This function always returns a new,
- // heap allocated pointer to an `AStatus` object, so we know the
- // returned pointer will be valid.
- //
- // Rust takes ownership of the returned pointer.
- sys::AStatus_fromServiceSpecificError(err)
- }
+ // Safety: Any i32 is a valid service specific error for the
+ // error code parameter. This function always returns a new,
+ // heap allocated pointer to an `AStatus` object, so we know the
+ // returned pointer will be valid.
+ //
+ // Rust takes ownership of the returned pointer.
+ unsafe { sys::AStatus_fromServiceSpecificError(err) }
};
Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer"))
}
@@ -159,6 +153,8 @@
/// Create a status object from an exception code
pub fn new_exception(exception: ExceptionCode, message: Option<&CStr>) -> Status {
if let Some(message) = message {
+ // Safety: the C string pointer is valid and not retained by the
+ // function.
let ptr = unsafe {
sys::AStatus_fromExceptionCodeWithMessage(exception as i32, message.as_ptr())
};
@@ -187,37 +183,31 @@
/// Returns `true` if this status represents a successful transaction.
pub fn is_ok(&self) -> bool {
- unsafe {
- // Safety: `Status` always contains a valid `AStatus` pointer, so we
- // are always passing a valid pointer to `AStatus_isOk` here.
- sys::AStatus_isOk(self.as_native())
- }
+ // Safety: `Status` always contains a valid `AStatus` pointer, so we
+ // are always passing a valid pointer to `AStatus_isOk` here.
+ unsafe { sys::AStatus_isOk(self.as_native()) }
}
/// Returns a description of the status.
pub fn get_description(&self) -> String {
- let description_ptr = unsafe {
- // Safety: `Status` always contains a valid `AStatus` pointer, so we
- // are always passing a valid pointer to `AStatus_getDescription`
- // here.
- //
- // `AStatus_getDescription` always returns a valid pointer to a null
- // terminated C string. Rust is responsible for freeing this pointer
- // via `AStatus_deleteDescription`.
- sys::AStatus_getDescription(self.as_native())
- };
- let description = unsafe {
- // Safety: `AStatus_getDescription` always returns a valid C string,
- // which can be safely converted to a `CStr`.
- CStr::from_ptr(description_ptr)
- };
+ // Safety: `Status` always contains a valid `AStatus` pointer, so we
+ // are always passing a valid pointer to `AStatus_getDescription`
+ // here.
+ //
+ // `AStatus_getDescription` always returns a valid pointer to a null
+ // terminated C string. Rust is responsible for freeing this pointer
+ // via `AStatus_deleteDescription`.
+ let description_ptr = unsafe { sys::AStatus_getDescription(self.as_native()) };
+ // Safety: `AStatus_getDescription` always returns a valid C string,
+ // which can be safely converted to a `CStr`.
+ let description = unsafe { CStr::from_ptr(description_ptr) };
let description = description.to_string_lossy().to_string();
+ // Safety: `description_ptr` was returned from
+ // `AStatus_getDescription` above, and must be freed via
+ // `AStatus_deleteDescription`. We must not access the pointer after
+ // this call, so we copy it into an owned string above and return
+ // that string.
unsafe {
- // Safety: `description_ptr` was returned from
- // `AStatus_getDescription` above, and must be freed via
- // `AStatus_deleteDescription`. We must not access the pointer after
- // this call, so we copy it into an owned string above and return
- // that string.
sys::AStatus_deleteDescription(description_ptr);
}
description
@@ -225,12 +215,10 @@
/// Returns the exception code of the status.
pub fn exception_code(&self) -> ExceptionCode {
- let code = unsafe {
- // Safety: `Status` always contains a valid `AStatus` pointer, so we
- // are always passing a valid pointer to `AStatus_getExceptionCode`
- // here.
- sys::AStatus_getExceptionCode(self.as_native())
- };
+ // Safety: `Status` always contains a valid `AStatus` pointer, so we
+ // are always passing a valid pointer to `AStatus_getExceptionCode`
+ // here.
+ let code = unsafe { sys::AStatus_getExceptionCode(self.as_native()) };
parse_exception_code(code)
}
@@ -241,11 +229,9 @@
/// exception or a service specific error. To find out if this transaction
/// as a whole is okay, use [`is_ok`](Self::is_ok) instead.
pub fn transaction_error(&self) -> StatusCode {
- let code = unsafe {
- // Safety: `Status` always contains a valid `AStatus` pointer, so we
- // are always passing a valid pointer to `AStatus_getStatus` here.
- sys::AStatus_getStatus(self.as_native())
- };
+ // Safety: `Status` always contains a valid `AStatus` pointer, so we
+ // are always passing a valid pointer to `AStatus_getStatus` here.
+ let code = unsafe { sys::AStatus_getStatus(self.as_native()) };
parse_status_code(code)
}
@@ -258,12 +244,10 @@
/// find out if this transaction as a whole is okay, use
/// [`is_ok`](Self::is_ok) instead.
pub fn service_specific_error(&self) -> i32 {
- unsafe {
- // Safety: `Status` always contains a valid `AStatus` pointer, so we
- // are always passing a valid pointer to
- // `AStatus_getServiceSpecificError` here.
- sys::AStatus_getServiceSpecificError(self.as_native())
- }
+ // Safety: `Status` always contains a valid `AStatus` pointer, so we
+ // are always passing a valid pointer to
+ // `AStatus_getServiceSpecificError` here.
+ unsafe { sys::AStatus_getServiceSpecificError(self.as_native()) }
}
/// Calls `op` if the status was ok, otherwise returns an `Err` value of
@@ -321,24 +305,20 @@
impl From<status_t> for Status {
fn from(status: status_t) -> Status {
- let ptr = unsafe {
- // Safety: `AStatus_fromStatus` expects any `status_t` integer, so
- // this is a safe FFI call. Unknown values will be coerced into
- // UNKNOWN_ERROR.
- sys::AStatus_fromStatus(status)
- };
+ // Safety: `AStatus_fromStatus` expects any `status_t` integer, so
+ // this is a safe FFI call. Unknown values will be coerced into
+ // UNKNOWN_ERROR.
+ let ptr = unsafe { sys::AStatus_fromStatus(status) };
Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer"))
}
}
impl From<ExceptionCode> for Status {
fn from(code: ExceptionCode) -> Status {
- let ptr = unsafe {
- // Safety: `AStatus_fromExceptionCode` expects any
- // `binder_exception_t` (i32) integer, so this is a safe FFI call.
- // Unknown values will be coerced into EX_TRANSACTION_FAILED.
- sys::AStatus_fromExceptionCode(code as i32)
- };
+ // Safety: `AStatus_fromExceptionCode` expects any
+ // `binder_exception_t` (i32) integer, so this is a safe FFI call.
+ // Unknown values will be coerced into EX_TRANSACTION_FAILED.
+ let ptr = unsafe { sys::AStatus_fromExceptionCode(code as i32) };
Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer"))
}
}
@@ -363,20 +343,18 @@
impl Drop for Status {
fn drop(&mut self) {
+ // Safety: `Status` manages the lifetime of its inner `AStatus`
+ // pointee, so we need to delete it here. We know that the pointer
+ // will be valid here since `Status` always contains a valid pointer
+ // while it is alive.
unsafe {
- // Safety: `Status` manages the lifetime of its inner `AStatus`
- // pointee, so we need to delete it here. We know that the pointer
- // will be valid here since `Status` always contains a valid pointer
- // while it is alive.
sys::AStatus_delete(self.0.as_mut());
}
}
}
-/// # Safety
-///
-/// `Status` always contains a valid pointer to an `AStatus` object, so we can
-/// trivially convert it to a correctly-typed raw pointer.
+/// Safety: `Status` always contains a valid pointer to an `AStatus` object, so
+/// we can trivially convert it to a correctly-typed raw pointer.
///
/// Care must be taken that the returned pointer is only dereferenced while the
/// `Status` object is still alive.
@@ -386,11 +364,9 @@
}
fn as_native_mut(&mut self) -> *mut sys::AStatus {
- unsafe {
- // Safety: The pointer will be valid here since `Status` always
- // contains a valid and initialized pointer while it is alive.
- self.0.as_mut()
- }
+ // Safety: The pointer will be valid here since `Status` always contains
+ // a valid and initialized pointer while it is alive.
+ unsafe { self.0.as_mut() }
}
}
diff --git a/libs/binder/rust/src/native.rs b/libs/binder/rust/src/native.rs
index 5557168..b248f5e 100644
--- a/libs/binder/rust/src/native.rs
+++ b/libs/binder/rust/src/native.rs
@@ -42,7 +42,7 @@
rust_object: *mut T,
}
-/// # Safety
+/// Safety:
///
/// A `Binder<T>` is a pair of unique owning pointers to two values:
/// * a C++ ABBinder which the C++ API guarantees can be passed between threads
@@ -54,7 +54,7 @@
/// to how `Box<T>` is `Send` if `T` is `Send`.
unsafe impl<T: Remotable> Send for Binder<T> {}
-/// # Safety
+/// Safety:
///
/// A `Binder<T>` is a pair of unique owning pointers to two values:
/// * a C++ ABBinder which is thread-safe, i.e. `Send + Sync`
@@ -89,15 +89,13 @@
pub fn new_with_stability(rust_object: T, stability: Stability) -> Binder<T> {
let class = T::get_class();
let rust_object = Box::into_raw(Box::new(rust_object));
- let ibinder = unsafe {
- // Safety: `AIBinder_new` expects a valid class pointer (which we
- // initialize via `get_class`), and an arbitrary pointer
- // argument. The caller owns the returned `AIBinder` pointer, which
- // is a strong reference to a `BBinder`. This reference should be
- // decremented via `AIBinder_decStrong` when the reference lifetime
- // ends.
- sys::AIBinder_new(class.into(), rust_object as *mut c_void)
- };
+ // Safety: `AIBinder_new` expects a valid class pointer (which we
+ // initialize via `get_class`), and an arbitrary pointer
+ // argument. The caller owns the returned `AIBinder` pointer, which
+ // is a strong reference to a `BBinder`. This reference should be
+ // decremented via `AIBinder_decStrong` when the reference lifetime
+ // ends.
+ let ibinder = unsafe { sys::AIBinder_new(class.into(), rust_object as *mut c_void) };
let mut binder = Binder { ibinder, rust_object };
binder.mark_stability(stability);
binder
@@ -176,15 +174,14 @@
/// }
/// # }
pub fn set_extension(&mut self, extension: &mut SpIBinder) -> Result<()> {
- let status = unsafe {
- // Safety: `AIBinder_setExtension` expects two valid, mutable
- // `AIBinder` pointers. We are guaranteed that both `self` and
- // `extension` contain valid `AIBinder` pointers, because they
- // cannot be initialized without a valid
- // pointer. `AIBinder_setExtension` does not take ownership of
- // either parameter.
- sys::AIBinder_setExtension(self.as_native_mut(), extension.as_native_mut())
- };
+ let status =
+ // Safety: `AIBinder_setExtension` expects two valid, mutable
+ // `AIBinder` pointers. We are guaranteed that both `self` and
+ // `extension` contain valid `AIBinder` pointers, because they
+ // cannot be initialized without a valid
+ // pointer. `AIBinder_setExtension` does not take ownership of
+ // either parameter.
+ unsafe { sys::AIBinder_setExtension(self.as_native_mut(), extension.as_native_mut()) };
status_result(status)
}
@@ -199,9 +196,9 @@
match stability {
Stability::Local => self.mark_local_stability(),
Stability::Vintf => {
+ // Safety: Self always contains a valid `AIBinder` pointer, so
+ // we can always call this C API safely.
unsafe {
- // Safety: Self always contains a valid `AIBinder` pointer, so
- // we can always call this C API safely.
sys::AIBinder_markVintfStability(self.as_native_mut());
}
}
@@ -212,9 +209,9 @@
/// building for android_vendor and system otherwise.
#[cfg(android_vendor)]
fn mark_local_stability(&mut self) {
+ // Safety: Self always contains a valid `AIBinder` pointer, so we can
+ // always call this C API safely.
unsafe {
- // Safety: Self always contains a valid `AIBinder` pointer, so
- // we can always call this C API safely.
sys::AIBinder_markVendorStability(self.as_native_mut());
}
}
@@ -223,9 +220,9 @@
/// building for android_vendor and system otherwise.
#[cfg(not(android_vendor))]
fn mark_local_stability(&mut self) {
+ // Safety: Self always contains a valid `AIBinder` pointer, so we can
+ // always call this C API safely.
unsafe {
- // Safety: Self always contains a valid `AIBinder` pointer, so
- // we can always call this C API safely.
sys::AIBinder_markSystemStability(self.as_native_mut());
}
}
@@ -239,13 +236,13 @@
/// remotable object, which will prevent the object from being dropped while
/// the `SpIBinder` is alive.
fn as_binder(&self) -> SpIBinder {
+ // Safety: `self.ibinder` is guaranteed to always be a valid pointer
+ // to an `AIBinder` by the `Binder` constructor. We are creating a
+ // copy of the `self.ibinder` strong reference, but
+ // `SpIBinder::from_raw` assumes it receives an owned pointer with
+ // its own strong reference. We first increment the reference count,
+ // so that the new `SpIBinder` will be tracked as a new reference.
unsafe {
- // Safety: `self.ibinder` is guaranteed to always be a valid pointer
- // to an `AIBinder` by the `Binder` constructor. We are creating a
- // copy of the `self.ibinder` strong reference, but
- // `SpIBinder::from_raw` assumes it receives an owned pointer with
- // its own strong reference. We first increment the reference count,
- // so that the new `SpIBinder` will be tracked as a new reference.
sys::AIBinder_incStrong(self.ibinder);
SpIBinder::from_raw(self.ibinder).unwrap()
}
@@ -275,10 +272,20 @@
reply: *mut sys::AParcel,
) -> status_t {
let res = {
- let mut reply = BorrowedParcel::from_raw(reply).unwrap();
- let data = BorrowedParcel::from_raw(data as *mut sys::AParcel).unwrap();
- let object = sys::AIBinder_getUserData(binder);
- let binder: &T = &*(object as *const T);
+ // Safety: The caller must give us a parcel pointer which is either
+ // null or valid at least for the duration of this function call. We
+ // don't keep the resulting value beyond the function.
+ let mut reply = unsafe { BorrowedParcel::from_raw(reply).unwrap() };
+ // Safety: The caller must give us a parcel pointer which is either
+ // null or valid at least for the duration of this function call. We
+ // don't keep the resulting value beyond the function.
+ let data = unsafe { BorrowedParcel::from_raw(data as *mut sys::AParcel).unwrap() };
+ // Safety: Our caller promised that `binder` is a non-null, valid
+ // pointer to a local `AIBinder`.
+ let object = unsafe { sys::AIBinder_getUserData(binder) };
+ // Safety: Our caller promised that the binder has a `T` pointer in
+ // its user data.
+ let binder: &T = unsafe { &*(object as *const T) };
binder.on_transact(code, &data, &mut reply)
};
match res {
@@ -295,7 +302,9 @@
/// Must be called with a valid pointer to a `T` object. After this call,
/// the pointer will be invalid and should not be dereferenced.
unsafe extern "C" fn on_destroy(object: *mut c_void) {
- drop(Box::from_raw(object as *mut T));
+ // Safety: Our caller promised that `object` is a valid pointer to a
+ // `T`.
+ drop(unsafe { Box::from_raw(object as *mut T) });
}
/// Called whenever a new, local `AIBinder` object is needed of a specific
@@ -320,7 +329,7 @@
/// Must be called with a non-null, valid pointer to a local `AIBinder` that
/// contains a `T` pointer in its user data. fd should be a non-owned file
/// descriptor, and args must be an array of null-terminated string
- /// poiinters with length num_args.
+ /// pointers with length num_args.
unsafe extern "C" fn on_dump(
binder: *mut sys::AIBinder,
fd: i32,
@@ -330,8 +339,9 @@
if fd < 0 {
return StatusCode::UNEXPECTED_NULL as status_t;
}
- // We don't own this file, so we need to be careful not to drop it.
- let file = ManuallyDrop::new(File::from_raw_fd(fd));
+ // Safety: Our caller promised that fd is a file descriptor. We don't
+ // own this file descriptor, so we need to be careful not to drop it.
+ let file = unsafe { ManuallyDrop::new(File::from_raw_fd(fd)) };
if args.is_null() && num_args != 0 {
return StatusCode::UNEXPECTED_NULL as status_t;
@@ -340,14 +350,22 @@
let args = if args.is_null() || num_args == 0 {
vec![]
} else {
- slice::from_raw_parts(args, num_args as usize)
- .iter()
- .map(|s| CStr::from_ptr(*s))
- .collect()
+ // Safety: Our caller promised that `args` is an array of
+ // null-terminated string pointers with length `num_args`.
+ unsafe {
+ slice::from_raw_parts(args, num_args as usize)
+ .iter()
+ .map(|s| CStr::from_ptr(*s))
+ .collect()
+ }
};
- let object = sys::AIBinder_getUserData(binder);
- let binder: &T = &*(object as *const T);
+ // Safety: Our caller promised that `binder` is a non-null, valid
+ // pointer to a local `AIBinder`.
+ let object = unsafe { sys::AIBinder_getUserData(binder) };
+ // Safety: Our caller promised that the binder has a `T` pointer in its
+ // user data.
+ let binder: &T = unsafe { &*(object as *const T) };
let res = binder.on_dump(&file, &args);
match res {
@@ -363,11 +381,11 @@
// actually destroys the object, it calls `on_destroy` and we can drop the
// `rust_object` then.
fn drop(&mut self) {
+ // Safety: When `self` is dropped, we can no longer access the
+ // reference, so can decrement the reference count. `self.ibinder` is
+ // always a valid `AIBinder` pointer, so is valid to pass to
+ // `AIBinder_decStrong`.
unsafe {
- // Safety: When `self` is dropped, we can no longer access the
- // reference, so can decrement the reference count. `self.ibinder`
- // is always a valid `AIBinder` pointer, so is valid to pass to
- // `AIBinder_decStrong`.
sys::AIBinder_decStrong(self.ibinder);
}
}
@@ -377,14 +395,11 @@
type Target = T;
fn deref(&self) -> &Self::Target {
- unsafe {
- // Safety: While `self` is alive, the reference count of the
- // underlying object is > 0 and therefore `on_destroy` cannot be
- // called. Therefore while `self` is alive, we know that
- // `rust_object` is still a valid pointer to a heap allocated object
- // of type `T`.
- &*self.rust_object
- }
+ // Safety: While `self` is alive, the reference count of the underlying
+ // object is > 0 and therefore `on_destroy` cannot be called. Therefore
+ // while `self` is alive, we know that `rust_object` is still a valid
+ // pointer to a heap allocated object of type `T`.
+ unsafe { &*self.rust_object }
}
}
@@ -405,13 +420,10 @@
if Some(class) != ibinder.get_class() {
return Err(StatusCode::BAD_TYPE);
}
- let userdata = unsafe {
- // Safety: `SpIBinder` always holds a valid pointer pointer to an
- // `AIBinder`, which we can safely pass to
- // `AIBinder_getUserData`. `ibinder` retains ownership of the
- // returned pointer.
- sys::AIBinder_getUserData(ibinder.as_native_mut())
- };
+ // Safety: `SpIBinder` always holds a valid pointer pointer to an
+ // `AIBinder`, which we can safely pass to `AIBinder_getUserData`.
+ // `ibinder` retains ownership of the returned pointer.
+ let userdata = unsafe { sys::AIBinder_getUserData(ibinder.as_native_mut()) };
if userdata.is_null() {
return Err(StatusCode::UNEXPECTED_NULL);
}
@@ -422,12 +434,10 @@
}
}
-/// # Safety
-///
-/// The constructor for `Binder` guarantees that `self.ibinder` will contain a
-/// valid, non-null pointer to an `AIBinder`, so this implementation is type
-/// safe. `self.ibinder` will remain valid for the entire lifetime of `self`
-/// because we hold a strong reference to the `AIBinder` until `self` is
+/// Safety: The constructor for `Binder` guarantees that `self.ibinder` will
+/// contain a valid, non-null pointer to an `AIBinder`, so this implementation
+/// is type safe. `self.ibinder` will remain valid for the entire lifetime of
+/// `self` because we hold a strong reference to the `AIBinder` until `self` is
/// dropped.
unsafe impl<B: Remotable> AsNative<sys::AIBinder> for Binder<B> {
fn as_native(&self) -> *const sys::AIBinder {
@@ -447,14 +457,12 @@
/// This function will panic if the identifier contains a 0 byte (NUL).
pub fn add_service(identifier: &str, mut binder: SpIBinder) -> Result<()> {
let instance = CString::new(identifier).unwrap();
- let status = unsafe {
- // Safety: `AServiceManager_addService` expects valid `AIBinder` and C
- // string pointers. Caller retains ownership of both
- // pointers. `AServiceManager_addService` creates a new strong reference
- // and copies the string, so both pointers need only be valid until the
- // call returns.
- sys::AServiceManager_addService(binder.as_native_mut(), instance.as_ptr())
- };
+ let status =
+ // Safety: `AServiceManager_addService` expects valid `AIBinder` and C
+ // string pointers. Caller retains ownership of both pointers.
+ // `AServiceManager_addService` creates a new strong reference and copies
+ // the string, so both pointers need only be valid until the call returns.
+ unsafe { sys::AServiceManager_addService(binder.as_native_mut(), instance.as_ptr()) };
status_result(status)
}
@@ -470,13 +478,12 @@
/// This function will panic if the identifier contains a 0 byte (NUL).
pub fn register_lazy_service(identifier: &str, mut binder: SpIBinder) -> Result<()> {
let instance = CString::new(identifier).unwrap();
+ // Safety: `AServiceManager_registerLazyService` expects valid `AIBinder` and C
+ // string pointers. Caller retains ownership of both
+ // pointers. `AServiceManager_registerLazyService` creates a new strong reference
+ // and copies the string, so both pointers need only be valid until the
+ // call returns.
let status = unsafe {
- // Safety: `AServiceManager_registerLazyService` expects valid `AIBinder` and C
- // string pointers. Caller retains ownership of both
- // pointers. `AServiceManager_registerLazyService` creates a new strong reference
- // and copies the string, so both pointers need only be valid until the
- // call returns.
-
sys::AServiceManager_registerLazyService(binder.as_native_mut(), instance.as_ptr())
};
status_result(status)
@@ -491,10 +498,8 @@
///
/// Consider using [`LazyServiceGuard`] rather than calling this directly.
pub fn force_lazy_services_persist(persist: bool) {
- unsafe {
- // Safety: No borrowing or transfer of ownership occurs here.
- sys::AServiceManager_forceLazyServicesPersist(persist)
- }
+ // Safety: No borrowing or transfer of ownership occurs here.
+ unsafe { sys::AServiceManager_forceLazyServicesPersist(persist) }
}
/// An RAII object to ensure a process which registers lazy services is not killed. During the
@@ -576,8 +581,6 @@
/// Determine whether the current thread is currently executing an incoming
/// transaction.
pub fn is_handling_transaction() -> bool {
- unsafe {
- // Safety: This method is always safe to call.
- sys::AIBinder_isHandlingTransaction()
- }
+ // Safety: This method is always safe to call.
+ unsafe { sys::AIBinder_isHandlingTransaction() }
}
diff --git a/libs/binder/rust/src/parcel.rs b/libs/binder/rust/src/parcel.rs
index e4c568e..f09aef6 100644
--- a/libs/binder/rust/src/parcel.rs
+++ b/libs/binder/rust/src/parcel.rs
@@ -52,11 +52,8 @@
ptr: NonNull<sys::AParcel>,
}
-/// # Safety
-///
-/// This type guarantees that it owns the AParcel and that all access to
-/// the AParcel happens through the Parcel, so it is ok to send across
-/// threads.
+/// Safety: This type guarantees that it owns the AParcel and that all access to
+/// the AParcel happens through the Parcel, so it is ok to send across threads.
unsafe impl Send for Parcel {}
/// Container for a message (data and object references) that can be sent
@@ -73,11 +70,9 @@
impl Parcel {
/// Create a new empty `Parcel`.
pub fn new() -> Parcel {
- let ptr = unsafe {
- // Safety: If `AParcel_create` succeeds, it always returns
- // a valid pointer. If it fails, the process will crash.
- sys::AParcel_create()
- };
+ // Safety: If `AParcel_create` succeeds, it always returns
+ // a valid pointer. If it fails, the process will crash.
+ let ptr = unsafe { sys::AParcel_create() };
Self { ptr: NonNull::new(ptr).expect("AParcel_create returned null pointer") }
}
@@ -171,10 +166,8 @@
}
}
-/// # Safety
-///
-/// The `Parcel` constructors guarantee that a `Parcel` object will always
-/// contain a valid pointer to an `AParcel`.
+/// Safety: The `Parcel` constructors guarantee that a `Parcel` object will
+/// always contain a valid pointer to an `AParcel`.
unsafe impl AsNative<sys::AParcel> for Parcel {
fn as_native(&self) -> *const sys::AParcel {
self.ptr.as_ptr()
@@ -185,10 +178,8 @@
}
}
-/// # Safety
-///
-/// The `BorrowedParcel` constructors guarantee that a `BorrowedParcel` object
-/// will always contain a valid pointer to an `AParcel`.
+/// Safety: The `BorrowedParcel` constructors guarantee that a `BorrowedParcel`
+/// object will always contain a valid pointer to an `AParcel`.
unsafe impl<'a> AsNative<sys::AParcel> for BorrowedParcel<'a> {
fn as_native(&self) -> *const sys::AParcel {
self.ptr.as_ptr()
@@ -203,10 +194,8 @@
impl<'a> BorrowedParcel<'a> {
/// Data written to parcelable is zero'd before being deleted or reallocated.
pub fn mark_sensitive(&mut self) {
- unsafe {
- // Safety: guaranteed to have a parcel object, and this method never fails
- sys::AParcel_markSensitive(self.as_native())
- }
+ // Safety: guaranteed to have a parcel object, and this method never fails
+ unsafe { sys::AParcel_markSensitive(self.as_native()) }
}
/// Write a type that implements [`Serialize`] to the parcel.
@@ -265,11 +254,15 @@
f(&mut subparcel)?;
}
let end = self.get_data_position();
+ // Safety: start is less than the current size of the parcel data
+ // buffer, because we just got it with `get_data_position`.
unsafe {
self.set_data_position(start)?;
}
assert!(end >= start);
self.write(&(end - start))?;
+ // Safety: end is less than the current size of the parcel data
+ // buffer, because we just got it with `get_data_position`.
unsafe {
self.set_data_position(end)?;
}
@@ -278,20 +271,16 @@
/// Returns the current position in the parcel data.
pub fn get_data_position(&self) -> i32 {
- unsafe {
- // Safety: `BorrowedParcel` always contains a valid pointer to an
- // `AParcel`, and this call is otherwise safe.
- sys::AParcel_getDataPosition(self.as_native())
- }
+ // Safety: `BorrowedParcel` always contains a valid pointer to an
+ // `AParcel`, and this call is otherwise safe.
+ unsafe { sys::AParcel_getDataPosition(self.as_native()) }
}
/// Returns the total size of the parcel.
pub fn get_data_size(&self) -> i32 {
- unsafe {
- // Safety: `BorrowedParcel` always contains a valid pointer to an
- // `AParcel`, and this call is otherwise safe.
- sys::AParcel_getDataSize(self.as_native())
- }
+ // Safety: `BorrowedParcel` always contains a valid pointer to an
+ // `AParcel`, and this call is otherwise safe.
+ unsafe { sys::AParcel_getDataSize(self.as_native()) }
}
/// Move the current read/write position in the parcel.
@@ -304,7 +293,9 @@
/// accesses are bounds checked, this call is still safe, but we can't rely
/// on that.
pub unsafe fn set_data_position(&self, pos: i32) -> Result<()> {
- status_result(sys::AParcel_setDataPosition(self.as_native(), pos))
+ // Safety: `BorrowedParcel` always contains a valid pointer to an
+ // `AParcel`, and the caller guarantees that `pos` is within bounds.
+ status_result(unsafe { sys::AParcel_setDataPosition(self.as_native(), pos) })
}
/// Append a subset of another parcel.
@@ -317,10 +308,10 @@
start: i32,
size: i32,
) -> Result<()> {
+ // Safety: `Parcel::appendFrom` from C++ checks that `start`
+ // and `size` are in bounds, and returns an error otherwise.
+ // Both `self` and `other` always contain valid pointers.
let status = unsafe {
- // Safety: `Parcel::appendFrom` from C++ checks that `start`
- // and `size` are in bounds, and returns an error otherwise.
- // Both `self` and `other` always contain valid pointers.
sys::AParcel_appendFrom(other.as_native(), self.as_native_mut(), start, size)
};
status_result(status)
@@ -418,7 +409,9 @@
/// accesses are bounds checked, this call is still safe, but we can't rely
/// on that.
pub unsafe fn set_data_position(&self, pos: i32) -> Result<()> {
- self.borrowed_ref().set_data_position(pos)
+ // Safety: We have the same safety requirements as
+ // `BorrowedParcel::set_data_position`.
+ unsafe { self.borrowed_ref().set_data_position(pos) }
}
/// Append a subset of another parcel.
@@ -504,7 +497,10 @@
f(subparcel)?;
// Advance the data position to the actual end,
- // in case the closure read less data than was available
+ // in case the closure read less data than was available.
+ //
+ // Safety: end must be less than the current size of the parcel, because
+ // we checked above against `get_data_size`.
unsafe {
self.set_data_position(end)?;
}
@@ -649,17 +645,17 @@
// Internal APIs
impl<'a> BorrowedParcel<'a> {
pub(crate) fn write_binder(&mut self, binder: Option<&SpIBinder>) -> Result<()> {
+ // Safety: `BorrowedParcel` always contains a valid pointer to an
+ // `AParcel`. `AsNative` for `Option<SpIBinder`> will either return
+ // null or a valid pointer to an `AIBinder`, both of which are
+ // valid, safe inputs to `AParcel_writeStrongBinder`.
+ //
+ // This call does not take ownership of the binder. However, it does
+ // require a mutable pointer, which we cannot extract from an
+ // immutable reference, so we clone the binder, incrementing the
+ // refcount before the call. The refcount will be immediately
+ // decremented when this temporary is dropped.
unsafe {
- // Safety: `BorrowedParcel` always contains a valid pointer to an
- // `AParcel`. `AsNative` for `Option<SpIBinder`> will either return
- // null or a valid pointer to an `AIBinder`, both of which are
- // valid, safe inputs to `AParcel_writeStrongBinder`.
- //
- // This call does not take ownership of the binder. However, it does
- // require a mutable pointer, which we cannot extract from an
- // immutable reference, so we clone the binder, incrementing the
- // refcount before the call. The refcount will be immediately
- // decremented when this temporary is dropped.
status_result(sys::AParcel_writeStrongBinder(
self.as_native_mut(),
binder.cloned().as_native_mut(),
@@ -669,33 +665,28 @@
pub(crate) fn read_binder(&self) -> Result<Option<SpIBinder>> {
let mut binder = ptr::null_mut();
- let status = unsafe {
- // Safety: `BorrowedParcel` always contains a valid pointer to an
- // `AParcel`. We pass a valid, mutable out pointer to the `binder`
- // parameter. After this call, `binder` will be either null or a
- // valid pointer to an `AIBinder` owned by the caller.
- sys::AParcel_readStrongBinder(self.as_native(), &mut binder)
- };
+ // Safety: `BorrowedParcel` always contains a valid pointer to an
+ // `AParcel`. We pass a valid, mutable out pointer to the `binder`
+ // parameter. After this call, `binder` will be either null or a
+ // valid pointer to an `AIBinder` owned by the caller.
+ let status = unsafe { sys::AParcel_readStrongBinder(self.as_native(), &mut binder) };
status_result(status)?;
- Ok(unsafe {
- // Safety: `binder` is either null or a valid, owned pointer at this
- // point, so can be safely passed to `SpIBinder::from_raw`.
- SpIBinder::from_raw(binder)
- })
+ // Safety: `binder` is either null or a valid, owned pointer at this
+ // point, so can be safely passed to `SpIBinder::from_raw`.
+ Ok(unsafe { SpIBinder::from_raw(binder) })
}
}
impl Drop for Parcel {
fn drop(&mut self) {
// Run the C++ Parcel complete object destructor
- unsafe {
- // Safety: `Parcel` always contains a valid pointer to an
- // `AParcel`. Since we own the parcel, we can safely delete it
- // here.
- sys::AParcel_delete(self.ptr.as_ptr())
- }
+ //
+ // Safety: `Parcel` always contains a valid pointer to an
+ // `AParcel`. Since we own the parcel, we can safely delete it
+ // here.
+ unsafe { sys::AParcel_delete(self.ptr.as_ptr()) }
}
}
diff --git a/libs/binder/rust/src/parcel/file_descriptor.rs b/libs/binder/rust/src/parcel/file_descriptor.rs
index 7fe37f3..5c688fa 100644
--- a/libs/binder/rust/src/parcel/file_descriptor.rs
+++ b/libs/binder/rust/src/parcel/file_descriptor.rs
@@ -73,14 +73,12 @@
impl Serialize for ParcelFileDescriptor {
fn serialize(&self, parcel: &mut BorrowedParcel<'_>) -> Result<()> {
let fd = self.0.as_raw_fd();
- let status = unsafe {
- // Safety: `Parcel` always contains a valid pointer to an
- // `AParcel`. Likewise, `ParcelFileDescriptor` always contains a
- // valid file, so we can borrow a valid file
- // descriptor. `AParcel_writeParcelFileDescriptor` does NOT take
- // ownership of the fd, so we need not duplicate it first.
- sys::AParcel_writeParcelFileDescriptor(parcel.as_native_mut(), fd)
- };
+ // Safety: `Parcel` always contains a valid pointer to an
+ // `AParcel`. Likewise, `ParcelFileDescriptor` always contains a
+ // valid file, so we can borrow a valid file
+ // descriptor. `AParcel_writeParcelFileDescriptor` does NOT take
+ // ownership of the fd, so we need not duplicate it first.
+ let status = unsafe { sys::AParcel_writeParcelFileDescriptor(parcel.as_native_mut(), fd) };
status_result(status)
}
}
@@ -92,13 +90,12 @@
if let Some(f) = this {
f.serialize(parcel)
} else {
- let status = unsafe {
- // Safety: `Parcel` always contains a valid pointer to an
- // `AParcel`. `AParcel_writeParcelFileDescriptor` accepts the
- // value `-1` as the file descriptor to signify serializing a
- // null file descriptor.
- sys::AParcel_writeParcelFileDescriptor(parcel.as_native_mut(), -1i32)
- };
+ let status =
+ // Safety: `Parcel` always contains a valid pointer to an
+ // `AParcel`. `AParcel_writeParcelFileDescriptor` accepts the
+ // value `-1` as the file descriptor to signify serializing a
+ // null file descriptor.
+ unsafe { sys::AParcel_writeParcelFileDescriptor(parcel.as_native_mut(), -1i32) };
status_result(status)
}
}
@@ -107,25 +104,23 @@
impl DeserializeOption for ParcelFileDescriptor {
fn deserialize_option(parcel: &BorrowedParcel<'_>) -> Result<Option<Self>> {
let mut fd = -1i32;
+ // Safety: `Parcel` always contains a valid pointer to an
+ // `AParcel`. We pass a valid mutable pointer to an i32, which
+ // `AParcel_readParcelFileDescriptor` assigns the valid file
+ // descriptor into, or `-1` if deserializing a null file
+ // descriptor. The read function passes ownership of the file
+ // descriptor to its caller if it was non-null, so we must take
+ // ownership of the file and ensure that it is eventually closed.
unsafe {
- // Safety: `Parcel` always contains a valid pointer to an
- // `AParcel`. We pass a valid mutable pointer to an i32, which
- // `AParcel_readParcelFileDescriptor` assigns the valid file
- // descriptor into, or `-1` if deserializing a null file
- // descriptor. The read function passes ownership of the file
- // descriptor to its caller if it was non-null, so we must take
- // ownership of the file and ensure that it is eventually closed.
status_result(sys::AParcel_readParcelFileDescriptor(parcel.as_native(), &mut fd))?;
}
if fd < 0 {
Ok(None)
} else {
- let file = unsafe {
- // Safety: At this point, we know that the file descriptor was
- // not -1, so must be a valid, owned file descriptor which we
- // can safely turn into a `File`.
- File::from_raw_fd(fd)
- };
+ // Safety: At this point, we know that the file descriptor was
+ // not -1, so must be a valid, owned file descriptor which we
+ // can safely turn into a `File`.
+ let file = unsafe { File::from_raw_fd(fd) };
Ok(Some(ParcelFileDescriptor::new(file)))
}
}
diff --git a/libs/binder/rust/src/parcel/parcelable.rs b/libs/binder/rust/src/parcel/parcelable.rs
index 5d8c11c..be7350a 100644
--- a/libs/binder/rust/src/parcel/parcelable.rs
+++ b/libs/binder/rust/src/parcel/parcelable.rs
@@ -102,8 +102,8 @@
pub trait SerializeArray: Serialize + Sized {
/// Serialize an array of this type into the given parcel.
fn serialize_array(slice: &[Self], parcel: &mut BorrowedParcel<'_>) -> Result<()> {
+ // Safety: Safe FFI, slice will always be a safe pointer to pass.
let res = unsafe {
- // Safety: Safe FFI, slice will always be a safe pointer to pass.
sys::AParcel_writeParcelableArray(
parcel.as_native_mut(),
slice.as_ptr() as *const c_void,
@@ -117,7 +117,9 @@
/// Callback to serialize an element of a generic parcelable array.
///
-/// Safety: We are relying on binder_ndk to not overrun our slice. As long as it
+/// # Safety
+///
+/// We are relying on binder_ndk to not overrun our slice. As long as it
/// doesn't provide an index larger than the length of the original slice in
/// serialize_array, this operation is safe. The index provided is zero-based.
unsafe extern "C" fn serialize_element<T: Serialize>(
@@ -125,9 +127,14 @@
array: *const c_void,
index: usize,
) -> status_t {
- let slice: &[T] = slice::from_raw_parts(array.cast(), index + 1);
+ // Safety: The caller guarantees that `array` is a valid pointer of the
+ // appropriate type.
+ let slice: &[T] = unsafe { slice::from_raw_parts(array.cast(), index + 1) };
- let mut parcel = match BorrowedParcel::from_raw(parcel) {
+ // Safety: The caller must give us a parcel pointer which is either null or
+ // valid at least for the duration of this function call. We don't keep the
+ // resulting value beyond the function.
+ let mut parcel = match unsafe { BorrowedParcel::from_raw(parcel) } {
None => return StatusCode::UNEXPECTED_NULL as status_t,
Some(p) => p,
};
@@ -142,9 +149,9 @@
/// Deserialize an array of type from the given parcel.
fn deserialize_array(parcel: &BorrowedParcel<'_>) -> Result<Option<Vec<Self>>> {
let mut vec: Option<Vec<Self::UninitType>> = None;
+ // Safety: Safe FFI, vec is the correct opaque type expected by
+ // allocate_vec and deserialize_element.
let res = unsafe {
- // Safety: Safe FFI, vec is the correct opaque type expected by
- // allocate_vec and deserialize_element.
sys::AParcel_readParcelableArray(
parcel.as_native(),
&mut vec as *mut _ as *mut c_void,
@@ -153,21 +160,21 @@
)
};
status_result(res)?;
- let vec: Option<Vec<Self>> = unsafe {
- // Safety: We are assuming that the NDK correctly initialized every
- // element of the vector by now, so we know that all the
- // UninitTypes are now properly initialized. We can transmute from
- // Vec<T::UninitType> to Vec<T> because T::UninitType has the same
- // alignment and size as T, so the pointer to the vector allocation
- // will be compatible.
- mem::transmute(vec)
- };
+ // Safety: We are assuming that the NDK correctly initialized every
+ // element of the vector by now, so we know that all the
+ // UninitTypes are now properly initialized. We can transmute from
+ // Vec<T::UninitType> to Vec<T> because T::UninitType has the same
+ // alignment and size as T, so the pointer to the vector allocation
+ // will be compatible.
+ let vec: Option<Vec<Self>> = unsafe { mem::transmute(vec) };
Ok(vec)
}
}
/// Callback to deserialize a parcelable element.
///
+/// # Safety
+///
/// The opaque array data pointer must be a mutable pointer to an
/// `Option<Vec<T::UninitType>>` with at least enough elements for `index` to be valid
/// (zero-based).
@@ -176,13 +183,18 @@
array: *mut c_void,
index: usize,
) -> status_t {
- let vec = &mut *(array as *mut Option<Vec<T::UninitType>>);
+ // Safety: The caller guarantees that `array` is a valid pointer of the
+ // appropriate type.
+ let vec = unsafe { &mut *(array as *mut Option<Vec<T::UninitType>>) };
let vec = match vec {
Some(v) => v,
None => return StatusCode::BAD_INDEX as status_t,
};
- let parcel = match BorrowedParcel::from_raw(parcel as *mut _) {
+ // Safety: The caller must give us a parcel pointer which is either null or
+ // valid at least for the duration of this function call. We don't keep the
+ // resulting value beyond the function.
+ let parcel = match unsafe { BorrowedParcel::from_raw(parcel as *mut _) } {
None => return StatusCode::UNEXPECTED_NULL as status_t,
Some(p) => p,
};
@@ -254,16 +266,21 @@
///
/// The opaque data pointer passed to the array read function must be a mutable
/// pointer to an `Option<Vec<T::UninitType>>`. `buffer` will be assigned a mutable pointer
-/// to the allocated vector data if this function returns true.
+/// to the allocated vector data if this function returns true. `buffer` must be a valid pointer.
unsafe extern "C" fn allocate_vec_with_buffer<T: Deserialize>(
data: *mut c_void,
len: i32,
buffer: *mut *mut T,
) -> bool {
- let res = allocate_vec::<T>(data, len);
- let vec = &mut *(data as *mut Option<Vec<T::UninitType>>);
+ // Safety: We have the same safety requirements as `allocate_vec` for `data`.
+ let res = unsafe { allocate_vec::<T>(data, len) };
+ // Safety: The caller guarantees that `data` is a valid mutable pointer to the appropriate type.
+ let vec = unsafe { &mut *(data as *mut Option<Vec<T::UninitType>>) };
if let Some(new_vec) = vec {
- *buffer = new_vec.as_mut_ptr() as *mut T;
+ // Safety: The caller guarantees that `buffer` is a valid pointer.
+ unsafe {
+ *buffer = new_vec.as_mut_ptr() as *mut T;
+ }
}
res
}
@@ -275,7 +292,8 @@
/// The opaque data pointer passed to the array read function must be a mutable
/// pointer to an `Option<Vec<T::UninitType>>`.
unsafe extern "C" fn allocate_vec<T: Deserialize>(data: *mut c_void, len: i32) -> bool {
- let vec = &mut *(data as *mut Option<Vec<T::UninitType>>);
+ // Safety: The caller guarantees that `data` is a valid mutable pointer to the appropriate type.
+ let vec = unsafe { &mut *(data as *mut Option<Vec<T::UninitType>>) };
if len < 0 {
*vec = None;
return true;
@@ -286,7 +304,10 @@
let mut new_vec: Vec<T::UninitType> = Vec::with_capacity(len as usize);
new_vec.resize_with(len as usize, T::uninit);
- ptr::write(vec, Some(new_vec));
+ // Safety: The caller guarantees that vec is a valid mutable pointer to the appropriate type.
+ unsafe {
+ ptr::write(vec, Some(new_vec));
+ }
true
}
@@ -305,21 +326,21 @@
// Assert at compile time that `T` and `T::UninitType` have the same size and alignment.
let _ = T::ASSERT_UNINIT_SIZE_AND_ALIGNMENT;
- // We can convert from Vec<T::UninitType> to Vec<T> because T::UninitType
- // has the same alignment and size as T, so the pointer to the vector
- // allocation will be compatible.
let mut vec = ManuallyDrop::new(vec);
- Vec::from_raw_parts(vec.as_mut_ptr().cast(), vec.len(), vec.capacity())
+ // Safety: We can convert from Vec<T::UninitType> to Vec<T> because
+ // T::UninitType has the same alignment and size as T, so the pointer to the
+ // vector allocation will be compatible.
+ unsafe { Vec::from_raw_parts(vec.as_mut_ptr().cast(), vec.len(), vec.capacity()) }
}
macro_rules! impl_parcelable {
{Serialize, $ty:ty, $write_fn:path} => {
impl Serialize for $ty {
fn serialize(&self, parcel: &mut BorrowedParcel<'_>) -> Result<()> {
+ // Safety: `Parcel` always contains a valid pointer to an
+ // `AParcel`, and any `$ty` literal value is safe to pass to
+ // `$write_fn`.
unsafe {
- // Safety: `Parcel` always contains a valid pointer to an
- // `AParcel`, and any `$ty` literal value is safe to pass to
- // `$write_fn`.
status_result($write_fn(parcel.as_native_mut(), *self))
}
}
@@ -333,11 +354,11 @@
fn from_init(value: Self) -> Self::UninitType { value }
fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> {
let mut val = Self::default();
+ // Safety: `Parcel` always contains a valid pointer to an
+ // `AParcel`. We pass a valid, mutable pointer to `val`, a
+ // literal of type `$ty`, and `$read_fn` will write the
+ // value read into `val` if successful
unsafe {
- // Safety: `Parcel` always contains a valid pointer to an
- // `AParcel`. We pass a valid, mutable pointer to `val`, a
- // literal of type `$ty`, and `$read_fn` will write the
- // value read into `val` if successful
status_result($read_fn(parcel.as_native(), &mut val))?
};
Ok(val)
@@ -348,13 +369,13 @@
{SerializeArray, $ty:ty, $write_array_fn:path} => {
impl SerializeArray for $ty {
fn serialize_array(slice: &[Self], parcel: &mut BorrowedParcel<'_>) -> Result<()> {
+ // Safety: `Parcel` always contains a valid pointer to an
+ // `AParcel`. If the slice is > 0 length, `slice.as_ptr()`
+ // will be a valid pointer to an array of elements of type
+ // `$ty`. If the slice length is 0, `slice.as_ptr()` may be
+ // dangling, but this is safe since the pointer is not
+ // dereferenced if the length parameter is 0.
let status = unsafe {
- // Safety: `Parcel` always contains a valid pointer to an
- // `AParcel`. If the slice is > 0 length, `slice.as_ptr()`
- // will be a valid pointer to an array of elements of type
- // `$ty`. If the slice length is 0, `slice.as_ptr()` may be
- // dangling, but this is safe since the pointer is not
- // dereferenced if the length parameter is 0.
$write_array_fn(
parcel.as_native_mut(),
slice.as_ptr(),
@@ -373,11 +394,11 @@
impl DeserializeArray for $ty {
fn deserialize_array(parcel: &BorrowedParcel<'_>) -> Result<Option<Vec<Self>>> {
let mut vec: Option<Vec<Self::UninitType>> = None;
+ // Safety: `Parcel` always contains a valid pointer to an
+ // `AParcel`. `allocate_vec<T>` expects the opaque pointer to
+ // be of type `*mut Option<Vec<T::UninitType>>`, so `&mut vec` is
+ // correct for it.
let status = unsafe {
- // Safety: `Parcel` always contains a valid pointer to an
- // `AParcel`. `allocate_vec<T>` expects the opaque pointer to
- // be of type `*mut Option<Vec<T::UninitType>>`, so `&mut vec` is
- // correct for it.
$read_array_fn(
parcel.as_native(),
&mut vec as *mut _ as *mut c_void,
@@ -385,11 +406,11 @@
)
};
status_result(status)?;
+ // Safety: We are assuming that the NDK correctly
+ // initialized every element of the vector by now, so we
+ // know that all the UninitTypes are now properly
+ // initialized.
let vec: Option<Vec<Self>> = unsafe {
- // Safety: We are assuming that the NDK correctly
- // initialized every element of the vector by now, so we
- // know that all the UninitTypes are now properly
- // initialized.
vec.map(|vec| vec_assume_init(vec))
};
Ok(vec)
@@ -479,13 +500,13 @@
impl SerializeArray for u8 {
fn serialize_array(slice: &[Self], parcel: &mut BorrowedParcel<'_>) -> Result<()> {
+ // Safety: `Parcel` always contains a valid pointer to an
+ // `AParcel`. If the slice is > 0 length, `slice.as_ptr()` will be a
+ // valid pointer to an array of elements of type `$ty`. If the slice
+ // length is 0, `slice.as_ptr()` may be dangling, but this is safe
+ // since the pointer is not dereferenced if the length parameter is
+ // 0.
let status = unsafe {
- // Safety: `Parcel` always contains a valid pointer to an
- // `AParcel`. If the slice is > 0 length, `slice.as_ptr()` will be a
- // valid pointer to an array of elements of type `$ty`. If the slice
- // length is 0, `slice.as_ptr()` may be dangling, but this is safe
- // since the pointer is not dereferenced if the length parameter is
- // 0.
sys::AParcel_writeByteArray(
parcel.as_native_mut(),
slice.as_ptr() as *const i8,
@@ -518,13 +539,13 @@
impl SerializeArray for i16 {
fn serialize_array(slice: &[Self], parcel: &mut BorrowedParcel<'_>) -> Result<()> {
+ // Safety: `Parcel` always contains a valid pointer to an
+ // `AParcel`. If the slice is > 0 length, `slice.as_ptr()` will be a
+ // valid pointer to an array of elements of type `$ty`. If the slice
+ // length is 0, `slice.as_ptr()` may be dangling, but this is safe
+ // since the pointer is not dereferenced if the length parameter is
+ // 0.
let status = unsafe {
- // Safety: `Parcel` always contains a valid pointer to an
- // `AParcel`. If the slice is > 0 length, `slice.as_ptr()` will be a
- // valid pointer to an array of elements of type `$ty`. If the slice
- // length is 0, `slice.as_ptr()` may be dangling, but this is safe
- // since the pointer is not dereferenced if the length parameter is
- // 0.
sys::AParcel_writeCharArray(
parcel.as_native_mut(),
slice.as_ptr() as *const u16,
@@ -538,22 +559,22 @@
impl SerializeOption for str {
fn serialize_option(this: Option<&Self>, parcel: &mut BorrowedParcel<'_>) -> Result<()> {
match this {
+ // Safety: `Parcel` always contains a valid pointer to an
+ // `AParcel`. If the string pointer is null,
+ // `AParcel_writeString` requires that the length is -1 to
+ // indicate that we want to serialize a null string.
None => unsafe {
- // Safety: `Parcel` always contains a valid pointer to an
- // `AParcel`. If the string pointer is null,
- // `AParcel_writeString` requires that the length is -1 to
- // indicate that we want to serialize a null string.
status_result(sys::AParcel_writeString(parcel.as_native_mut(), ptr::null(), -1))
},
+ // Safety: `Parcel` always contains a valid pointer to an
+ // `AParcel`. `AParcel_writeString` assumes that we pass a utf-8
+ // string pointer of `length` bytes, which is what str in Rust
+ // is. The docstring for `AParcel_writeString` says that the
+ // string input should be null-terminated, but it doesn't
+ // actually rely on that fact in the code. If this ever becomes
+ // necessary, we will need to null-terminate the str buffer
+ // before sending it.
Some(s) => unsafe {
- // Safety: `Parcel` always contains a valid pointer to an
- // `AParcel`. `AParcel_writeString` assumes that we pass a utf-8
- // string pointer of `length` bytes, which is what str in Rust
- // is. The docstring for `AParcel_writeString` says that the
- // string input should be null-terminated, but it doesn't
- // actually rely on that fact in the code. If this ever becomes
- // necessary, we will need to null-terminate the str buffer
- // before sending it.
status_result(sys::AParcel_writeString(
parcel.as_native_mut(),
s.as_ptr() as *const c_char,
@@ -597,11 +618,11 @@
fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> {
let mut vec: Option<Vec<u8>> = None;
+ // Safety: `Parcel` always contains a valid pointer to an `AParcel`.
+ // `Option<Vec<u8>>` is equivalent to the expected `Option<Vec<i8>>`
+ // for `allocate_vec`, so `vec` is safe to pass as the opaque data
+ // pointer on platforms where char is signed.
let status = unsafe {
- // Safety: `Parcel` always contains a valid pointer to an `AParcel`.
- // `Option<Vec<u8>>` is equivalent to the expected `Option<Vec<i8>>`
- // for `allocate_vec`, so `vec` is safe to pass as the opaque data
- // pointer on platforms where char is signed.
sys::AParcel_readString(
parcel.as_native(),
&mut vec as *mut _ as *mut c_void,
@@ -751,11 +772,11 @@
impl Serialize for Status {
fn serialize(&self, parcel: &mut BorrowedParcel<'_>) -> Result<()> {
+ // Safety: `Parcel` always contains a valid pointer to an `AParcel`
+ // and `Status` always contains a valid pointer to an `AStatus`, so
+ // both parameters are valid and safe. This call does not take
+ // ownership of either of its parameters.
unsafe {
- // Safety: `Parcel` always contains a valid pointer to an `AParcel`
- // and `Status` always contains a valid pointer to an `AStatus`, so
- // both parameters are valid and safe. This call does not take
- // ownership of either of its parameters.
status_result(sys::AParcel_writeStatusHeader(parcel.as_native_mut(), self.as_native()))
}
}
@@ -772,21 +793,18 @@
fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> {
let mut status_ptr = ptr::null_mut();
- let ret_status = unsafe {
- // Safety: `Parcel` always contains a valid pointer to an
- // `AParcel`. We pass a mutable out pointer which will be
- // assigned a valid `AStatus` pointer if the function returns
- // status OK. This function passes ownership of the status
- // pointer to the caller, if it was assigned.
- sys::AParcel_readStatusHeader(parcel.as_native(), &mut status_ptr)
- };
+ let ret_status =
+ // Safety: `Parcel` always contains a valid pointer to an
+ // `AParcel`. We pass a mutable out pointer which will be
+ // assigned a valid `AStatus` pointer if the function returns
+ // status OK. This function passes ownership of the status
+ // pointer to the caller, if it was assigned.
+ unsafe { sys::AParcel_readStatusHeader(parcel.as_native(), &mut status_ptr) };
status_result(ret_status)?;
- Ok(unsafe {
- // Safety: At this point, the return status of the read call was ok,
- // so we know that `status_ptr` is a valid, owned pointer to an
- // `AStatus`, from which we can safely construct a `Status` object.
- Status::from_ptr(status_ptr)
- })
+ // Safety: At this point, the return status of the read call was ok,
+ // so we know that `status_ptr` is a valid, owned pointer to an
+ // `AStatus`, from which we can safely construct a `Status` object.
+ Ok(unsafe { Status::from_ptr(status_ptr) })
}
}
diff --git a/libs/binder/rust/src/parcel/parcelable_holder.rs b/libs/binder/rust/src/parcel/parcelable_holder.rs
index eb82fb7..f906113 100644
--- a/libs/binder/rust/src/parcel/parcelable_holder.rs
+++ b/libs/binder/rust/src/parcel/parcelable_holder.rs
@@ -133,8 +133,8 @@
}
}
ParcelableHolderData::Parcel(ref mut parcel) => {
+ // Safety: 0 should always be a valid position.
unsafe {
- // Safety: 0 should always be a valid position.
parcel.set_data_position(0)?;
}
@@ -214,15 +214,15 @@
parcelable.write_to_parcel(parcel)?;
let end = parcel.get_data_position();
+ // Safety: we got the position from `get_data_position`.
unsafe {
- // Safety: we got the position from `get_data_position`.
parcel.set_data_position(length_start)?;
}
assert!(end >= data_start);
parcel.write(&(end - data_start))?;
+ // Safety: we got the position from `get_data_position`.
unsafe {
- // Safety: we got the position from `get_data_position`.
parcel.set_data_position(end)?;
}
@@ -260,11 +260,11 @@
new_parcel.append_from(parcel, data_start, data_size)?;
*self.data.get_mut().unwrap() = ParcelableHolderData::Parcel(new_parcel);
+ // Safety: `append_from` checks if `data_size` overflows
+ // `parcel` and returns `BAD_VALUE` if that happens. We also
+ // explicitly check for negative and zero `data_size` above,
+ // so `data_end` is guaranteed to be greater than `data_start`.
unsafe {
- // Safety: `append_from` checks if `data_size` overflows
- // `parcel` and returns `BAD_VALUE` if that happens. We also
- // explicitly check for negative and zero `data_size` above,
- // so `data_end` is guaranteed to be greater than `data_start`.
parcel.set_data_position(data_end)?;
}
diff --git a/libs/binder/rust/src/proxy.rs b/libs/binder/rust/src/proxy.rs
index 036f6b4..e55f48b 100644
--- a/libs/binder/rust/src/proxy.rs
+++ b/libs/binder/rust/src/proxy.rs
@@ -49,14 +49,12 @@
}
}
-/// # Safety
-///
-/// An `SpIBinder` is an immutable handle to a C++ IBinder, which is thread-safe
+/// Safety: An `SpIBinder` is an immutable handle to a C++ IBinder, which is
+/// thread-safe.
unsafe impl Send for SpIBinder {}
-/// # Safety
-///
-/// An `SpIBinder` is an immutable handle to a C++ IBinder, which is thread-safe
+/// Safety: An `SpIBinder` is an immutable handle to a C++ IBinder, which is
+/// thread-safe.
unsafe impl Sync for SpIBinder {}
impl SpIBinder {
@@ -97,11 +95,9 @@
/// Return true if this binder object is hosted in a different process than
/// the current one.
pub fn is_remote(&self) -> bool {
- unsafe {
- // Safety: `SpIBinder` guarantees that it always contains a valid
- // `AIBinder` pointer.
- sys::AIBinder_isRemote(self.as_native())
- }
+ // Safety: `SpIBinder` guarantees that it always contains a valid
+ // `AIBinder` pointer.
+ unsafe { sys::AIBinder_isRemote(self.as_native()) }
}
/// Try to convert this Binder object into a trait object for the given
@@ -116,12 +112,12 @@
/// Return the interface class of this binder object, if associated with
/// one.
pub fn get_class(&mut self) -> Option<InterfaceClass> {
+ // Safety: `SpIBinder` guarantees that it always contains a valid
+ // `AIBinder` pointer. `AIBinder_getClass` returns either a null
+ // pointer or a valid pointer to an `AIBinder_Class`. After mapping
+ // null to None, we can safely construct an `InterfaceClass` if the
+ // pointer was non-null.
unsafe {
- // Safety: `SpIBinder` guarantees that it always contains a valid
- // `AIBinder` pointer. `AIBinder_getClass` returns either a null
- // pointer or a valid pointer to an `AIBinder_Class`. After mapping
- // null to None, we can safely construct an `InterfaceClass` if the
- // pointer was non-null.
let class = sys::AIBinder_getClass(self.as_native_mut());
class.as_ref().map(|p| InterfaceClass::from_ptr(p))
}
@@ -152,7 +148,8 @@
///
/// See `SpIBinder::from_raw`.
pub unsafe fn new_spibinder(ptr: *mut sys::AIBinder) -> Option<SpIBinder> {
- SpIBinder::from_raw(ptr)
+ // Safety: The caller makes the same guarantees as this requires.
+ unsafe { SpIBinder::from_raw(ptr) }
}
}
@@ -171,30 +168,24 @@
impl AssociateClass for SpIBinder {
fn associate_class(&mut self, class: InterfaceClass) -> bool {
- unsafe {
- // Safety: `SpIBinder` guarantees that it always contains a valid
- // `AIBinder` pointer. An `InterfaceClass` can always be converted
- // into a valid `AIBinder_Class` pointer, so these parameters are
- // always safe.
- sys::AIBinder_associateClass(self.as_native_mut(), class.into())
- }
+ // Safety: `SpIBinder` guarantees that it always contains a valid
+ // `AIBinder` pointer. An `InterfaceClass` can always be converted
+ // into a valid `AIBinder_Class` pointer, so these parameters are
+ // always safe.
+ unsafe { sys::AIBinder_associateClass(self.as_native_mut(), class.into()) }
}
}
impl Ord for SpIBinder {
fn cmp(&self, other: &Self) -> Ordering {
- let less_than = unsafe {
- // Safety: SpIBinder always holds a valid `AIBinder` pointer, so
- // this pointer is always safe to pass to `AIBinder_lt` (null is
- // also safe to pass to this function, but we should never do that).
- sys::AIBinder_lt(self.0.as_ptr(), other.0.as_ptr())
- };
- let greater_than = unsafe {
- // Safety: SpIBinder always holds a valid `AIBinder` pointer, so
- // this pointer is always safe to pass to `AIBinder_lt` (null is
- // also safe to pass to this function, but we should never do that).
- sys::AIBinder_lt(other.0.as_ptr(), self.0.as_ptr())
- };
+ // Safety: SpIBinder always holds a valid `AIBinder` pointer, so this
+ // pointer is always safe to pass to `AIBinder_lt` (null is also safe to
+ // pass to this function, but we should never do that).
+ let less_than = unsafe { sys::AIBinder_lt(self.0.as_ptr(), other.0.as_ptr()) };
+ // Safety: SpIBinder always holds a valid `AIBinder` pointer, so this
+ // pointer is always safe to pass to `AIBinder_lt` (null is also safe to
+ // pass to this function, but we should never do that).
+ let greater_than = unsafe { sys::AIBinder_lt(other.0.as_ptr(), self.0.as_ptr()) };
if !less_than && !greater_than {
Ordering::Equal
} else if less_than {
@@ -221,10 +212,10 @@
impl Clone for SpIBinder {
fn clone(&self) -> Self {
+ // Safety: Cloning a strong reference must increment the reference
+ // count. We are guaranteed by the `SpIBinder` constructor
+ // invariants that `self.0` is always a valid `AIBinder` pointer.
unsafe {
- // Safety: Cloning a strong reference must increment the reference
- // count. We are guaranteed by the `SpIBinder` constructor
- // invariants that `self.0` is always a valid `AIBinder` pointer.
sys::AIBinder_incStrong(self.0.as_ptr());
}
Self(self.0)
@@ -235,9 +226,9 @@
// We hold a strong reference to the IBinder in SpIBinder and need to give up
// this reference on drop.
fn drop(&mut self) {
+ // Safety: SpIBinder always holds a valid `AIBinder` pointer, so we
+ // know this pointer is safe to pass to `AIBinder_decStrong` here.
unsafe {
- // Safety: SpIBinder always holds a valid `AIBinder` pointer, so we
- // know this pointer is safe to pass to `AIBinder_decStrong` here.
sys::AIBinder_decStrong(self.as_native_mut());
}
}
@@ -246,26 +237,24 @@
impl<T: AsNative<sys::AIBinder>> IBinderInternal for T {
fn prepare_transact(&self) -> Result<Parcel> {
let mut input = ptr::null_mut();
+ // Safety: `SpIBinder` guarantees that `self` always contains a
+ // valid pointer to an `AIBinder`. It is safe to cast from an
+ // immutable pointer to a mutable pointer here, because
+ // `AIBinder_prepareTransaction` only calls immutable `AIBinder`
+ // methods but the parameter is unfortunately not marked as const.
+ //
+ // After the call, input will be either a valid, owned `AParcel`
+ // pointer, or null.
let status = unsafe {
- // Safety: `SpIBinder` guarantees that `self` always contains a
- // valid pointer to an `AIBinder`. It is safe to cast from an
- // immutable pointer to a mutable pointer here, because
- // `AIBinder_prepareTransaction` only calls immutable `AIBinder`
- // methods but the parameter is unfortunately not marked as const.
- //
- // After the call, input will be either a valid, owned `AParcel`
- // pointer, or null.
sys::AIBinder_prepareTransaction(self.as_native() as *mut sys::AIBinder, &mut input)
};
status_result(status)?;
- unsafe {
- // Safety: At this point, `input` is either a valid, owned `AParcel`
- // pointer, or null. `OwnedParcel::from_raw` safely handles both cases,
- // taking ownership of the parcel.
- Parcel::from_raw(input).ok_or(StatusCode::UNEXPECTED_NULL)
- }
+ // Safety: At this point, `input` is either a valid, owned `AParcel`
+ // pointer, or null. `OwnedParcel::from_raw` safely handles both cases,
+ // taking ownership of the parcel.
+ unsafe { Parcel::from_raw(input).ok_or(StatusCode::UNEXPECTED_NULL) }
}
fn submit_transact(
@@ -275,23 +264,23 @@
flags: TransactionFlags,
) -> Result<Parcel> {
let mut reply = ptr::null_mut();
+ // Safety: `SpIBinder` guarantees that `self` always contains a
+ // valid pointer to an `AIBinder`. Although `IBinder::transact` is
+ // not a const method, it is still safe to cast our immutable
+ // pointer to mutable for the call. First, `IBinder::transact` is
+ // thread-safe, so concurrency is not an issue. The only way that
+ // `transact` can affect any visible, mutable state in the current
+ // process is by calling `onTransact` for a local service. However,
+ // in order for transactions to be thread-safe, this method must
+ // dynamically lock its data before modifying it. We enforce this
+ // property in Rust by requiring `Sync` for remotable objects and
+ // only providing `on_transact` with an immutable reference to
+ // `self`.
+ //
+ // This call takes ownership of the `data` parcel pointer, and
+ // passes ownership of the `reply` out parameter to its caller. It
+ // does not affect ownership of the `binder` parameter.
let status = unsafe {
- // Safety: `SpIBinder` guarantees that `self` always contains a
- // valid pointer to an `AIBinder`. Although `IBinder::transact` is
- // not a const method, it is still safe to cast our immutable
- // pointer to mutable for the call. First, `IBinder::transact` is
- // thread-safe, so concurrency is not an issue. The only way that
- // `transact` can affect any visible, mutable state in the current
- // process is by calling `onTransact` for a local service. However,
- // in order for transactions to be thread-safe, this method must
- // dynamically lock its data before modifying it. We enforce this
- // property in Rust by requiring `Sync` for remotable objects and
- // only providing `on_transact` with an immutable reference to
- // `self`.
- //
- // This call takes ownership of the `data` parcel pointer, and
- // passes ownership of the `reply` out parameter to its caller. It
- // does not affect ownership of the `binder` parameter.
sys::AIBinder_transact(
self.as_native() as *mut sys::AIBinder,
code,
@@ -302,45 +291,45 @@
};
status_result(status)?;
- unsafe {
- // Safety: `reply` is either a valid `AParcel` pointer or null
- // after the call to `AIBinder_transact` above, so we can
- // construct a `Parcel` out of it. `AIBinder_transact` passes
- // ownership of the `reply` parcel to Rust, so we need to
- // construct an owned variant.
- Parcel::from_raw(reply).ok_or(StatusCode::UNEXPECTED_NULL)
- }
+ // Safety: `reply` is either a valid `AParcel` pointer or null
+ // after the call to `AIBinder_transact` above, so we can
+ // construct a `Parcel` out of it. `AIBinder_transact` passes
+ // ownership of the `reply` parcel to Rust, so we need to
+ // construct an owned variant.
+ unsafe { Parcel::from_raw(reply).ok_or(StatusCode::UNEXPECTED_NULL) }
}
fn is_binder_alive(&self) -> bool {
- unsafe {
- // Safety: `SpIBinder` guarantees that `self` always contains a
- // valid pointer to an `AIBinder`.
- //
- // This call does not affect ownership of its pointer parameter.
- sys::AIBinder_isAlive(self.as_native())
- }
+ // Safety: `SpIBinder` guarantees that `self` always contains a valid
+ // pointer to an `AIBinder`.
+ //
+ // This call does not affect ownership of its pointer parameter.
+ unsafe { sys::AIBinder_isAlive(self.as_native()) }
}
#[cfg(not(android_vndk))]
fn set_requesting_sid(&mut self, enable: bool) {
+ // Safety: `SpIBinder` guarantees that `self` always contains a valid
+ // pointer to an `AIBinder`.
+ //
+ // This call does not affect ownership of its pointer parameter.
unsafe { sys::AIBinder_setRequestingSid(self.as_native_mut(), enable) };
}
fn dump<F: AsRawFd>(&mut self, fp: &F, args: &[&str]) -> Result<()> {
let args: Vec<_> = args.iter().map(|a| CString::new(*a).unwrap()).collect();
let mut arg_ptrs: Vec<_> = args.iter().map(|a| a.as_ptr()).collect();
+ // Safety: `SpIBinder` guarantees that `self` always contains a
+ // valid pointer to an `AIBinder`. `AsRawFd` guarantees that the
+ // file descriptor parameter is always be a valid open file. The
+ // `args` pointer parameter is a valid pointer to an array of C
+ // strings that will outlive the call since `args` lives for the
+ // whole function scope.
+ //
+ // This call does not affect ownership of its binder pointer
+ // parameter and does not take ownership of the file or args array
+ // parameters.
let status = unsafe {
- // Safety: `SpIBinder` guarantees that `self` always contains a
- // valid pointer to an `AIBinder`. `AsRawFd` guarantees that the
- // file descriptor parameter is always be a valid open file. The
- // `args` pointer parameter is a valid pointer to an array of C
- // strings that will outlive the call since `args` lives for the
- // whole function scope.
- //
- // This call does not affect ownership of its binder pointer
- // parameter and does not take ownership of the file or args array
- // parameters.
sys::AIBinder_dump(
self.as_native_mut(),
fp.as_raw_fd(),
@@ -353,22 +342,18 @@
fn get_extension(&mut self) -> Result<Option<SpIBinder>> {
let mut out = ptr::null_mut();
- let status = unsafe {
- // Safety: `SpIBinder` guarantees that `self` always contains a
- // valid pointer to an `AIBinder`. After this call, the `out`
- // parameter will be either null, or a valid pointer to an
- // `AIBinder`.
- //
- // This call passes ownership of the out pointer to its caller
- // (assuming it is set to a non-null value).
- sys::AIBinder_getExtension(self.as_native_mut(), &mut out)
- };
- let ibinder = unsafe {
- // Safety: The call above guarantees that `out` is either null or a
- // valid, owned pointer to an `AIBinder`, both of which are safe to
- // pass to `SpIBinder::from_raw`.
- SpIBinder::from_raw(out)
- };
+ // Safety: `SpIBinder` guarantees that `self` always contains a
+ // valid pointer to an `AIBinder`. After this call, the `out`
+ // parameter will be either null, or a valid pointer to an
+ // `AIBinder`.
+ //
+ // This call passes ownership of the out pointer to its caller
+ // (assuming it is set to a non-null value).
+ let status = unsafe { sys::AIBinder_getExtension(self.as_native_mut(), &mut out) };
+ // Safety: The call above guarantees that `out` is either null or a
+ // valid, owned pointer to an `AIBinder`, both of which are safe to
+ // pass to `SpIBinder::from_raw`.
+ let ibinder = unsafe { SpIBinder::from_raw(out) };
status_result(status)?;
Ok(ibinder)
@@ -377,17 +362,17 @@
impl<T: AsNative<sys::AIBinder>> IBinder for T {
fn link_to_death(&mut self, recipient: &mut DeathRecipient) -> Result<()> {
+ // Safety: `SpIBinder` guarantees that `self` always contains a
+ // valid pointer to an `AIBinder`. `recipient` can always be
+ // converted into a valid pointer to an
+ // `AIBinder_DeathRecipient`.
+ //
+ // The cookie is also the correct pointer, and by calling new_cookie,
+ // we have created a new ref-count to the cookie, which linkToDeath
+ // takes ownership of. Once the DeathRecipient is unlinked for any
+ // reason (including if this call fails), the onUnlinked callback
+ // will consume that ref-count.
status_result(unsafe {
- // Safety: `SpIBinder` guarantees that `self` always contains a
- // valid pointer to an `AIBinder`. `recipient` can always be
- // converted into a valid pointer to an
- // `AIBinder_DeathRecipient`.
- //
- // The cookie is also the correct pointer, and by calling new_cookie,
- // we have created a new ref-count to the cookie, which linkToDeath
- // takes ownership of. Once the DeathRecipient is unlinked for any
- // reason (including if this call fails), the onUnlinked callback
- // will consume that ref-count.
sys::AIBinder_linkToDeath(
self.as_native_mut(),
recipient.as_native_mut(),
@@ -397,13 +382,13 @@
}
fn unlink_to_death(&mut self, recipient: &mut DeathRecipient) -> Result<()> {
+ // Safety: `SpIBinder` guarantees that `self` always contains a
+ // valid pointer to an `AIBinder`. `recipient` can always be
+ // converted into a valid pointer to an
+ // `AIBinder_DeathRecipient`. Any value is safe to pass as the
+ // cookie, although we depend on this value being set by
+ // `get_cookie` when the death recipient callback is called.
status_result(unsafe {
- // Safety: `SpIBinder` guarantees that `self` always contains a
- // valid pointer to an `AIBinder`. `recipient` can always be
- // converted into a valid pointer to an
- // `AIBinder_DeathRecipient`. Any value is safe to pass as the
- // cookie, although we depend on this value being set by
- // `get_cookie` when the death recipient callback is called.
sys::AIBinder_unlinkToDeath(
self.as_native_mut(),
recipient.as_native_mut(),
@@ -413,13 +398,11 @@
}
fn ping_binder(&mut self) -> Result<()> {
- let status = unsafe {
- // Safety: `SpIBinder` guarantees that `self` always contains a
- // valid pointer to an `AIBinder`.
- //
- // This call does not affect ownership of its pointer parameter.
- sys::AIBinder_ping(self.as_native_mut())
- };
+ // Safety: `SpIBinder` guarantees that `self` always contains a
+ // valid pointer to an `AIBinder`.
+ //
+ // This call does not affect ownership of its pointer parameter.
+ let status = unsafe { sys::AIBinder_ping(self.as_native_mut()) };
status_result(status)
}
}
@@ -472,35 +455,31 @@
}
}
-/// # Safety
-///
-/// A `WpIBinder` is an immutable handle to a C++ IBinder, which is thread-safe.
+/// Safety: A `WpIBinder` is an immutable handle to a C++ IBinder, which is
+/// thread-safe.
unsafe impl Send for WpIBinder {}
-/// # Safety
-///
-/// A `WpIBinder` is an immutable handle to a C++ IBinder, which is thread-safe.
+/// Safety: A `WpIBinder` is an immutable handle to a C++ IBinder, which is
+/// thread-safe.
unsafe impl Sync for WpIBinder {}
impl WpIBinder {
/// Create a new weak reference from an object that can be converted into a
/// raw `AIBinder` pointer.
fn new<B: AsNative<sys::AIBinder>>(binder: &mut B) -> WpIBinder {
- let ptr = unsafe {
- // Safety: `SpIBinder` guarantees that `binder` always contains a
- // valid pointer to an `AIBinder`.
- sys::AIBinder_Weak_new(binder.as_native_mut())
- };
+ // Safety: `SpIBinder` guarantees that `binder` always contains a valid
+ // pointer to an `AIBinder`.
+ let ptr = unsafe { sys::AIBinder_Weak_new(binder.as_native_mut()) };
Self(ptr::NonNull::new(ptr).expect("Unexpected null pointer from AIBinder_Weak_new"))
}
/// Promote this weak reference to a strong reference to the binder object.
pub fn promote(&self) -> Option<SpIBinder> {
+ // Safety: `WpIBinder` always contains a valid weak reference, so we can
+ // pass this pointer to `AIBinder_Weak_promote`. Returns either null or
+ // an AIBinder owned by the caller, both of which are valid to pass to
+ // `SpIBinder::from_raw`.
unsafe {
- // Safety: `WpIBinder` always contains a valid weak reference, so we
- // can pass this pointer to `AIBinder_Weak_promote`. Returns either
- // null or an AIBinder owned by the caller, both of which are valid
- // to pass to `SpIBinder::from_raw`.
let ptr = sys::AIBinder_Weak_promote(self.0.as_ptr());
SpIBinder::from_raw(ptr)
}
@@ -509,35 +488,27 @@
impl Clone for WpIBinder {
fn clone(&self) -> Self {
- let ptr = unsafe {
- // Safety: WpIBinder always holds a valid `AIBinder_Weak` pointer,
- // so this pointer is always safe to pass to `AIBinder_Weak_clone`
- // (although null is also a safe value to pass to this API).
- //
- // We get ownership of the returned pointer, so can construct a new
- // WpIBinder object from it.
- sys::AIBinder_Weak_clone(self.0.as_ptr())
- };
+ // Safety: WpIBinder always holds a valid `AIBinder_Weak` pointer, so
+ // this pointer is always safe to pass to `AIBinder_Weak_clone`
+ // (although null is also a safe value to pass to this API).
+ //
+ // We get ownership of the returned pointer, so can construct a new
+ // WpIBinder object from it.
+ let ptr = unsafe { sys::AIBinder_Weak_clone(self.0.as_ptr()) };
Self(ptr::NonNull::new(ptr).expect("Unexpected null pointer from AIBinder_Weak_clone"))
}
}
impl Ord for WpIBinder {
fn cmp(&self, other: &Self) -> Ordering {
- let less_than = unsafe {
- // Safety: WpIBinder always holds a valid `AIBinder_Weak` pointer,
- // so this pointer is always safe to pass to `AIBinder_Weak_lt`
- // (null is also safe to pass to this function, but we should never
- // do that).
- sys::AIBinder_Weak_lt(self.0.as_ptr(), other.0.as_ptr())
- };
- let greater_than = unsafe {
- // Safety: WpIBinder always holds a valid `AIBinder_Weak` pointer,
- // so this pointer is always safe to pass to `AIBinder_Weak_lt`
- // (null is also safe to pass to this function, but we should never
- // do that).
- sys::AIBinder_Weak_lt(other.0.as_ptr(), self.0.as_ptr())
- };
+ // Safety: WpIBinder always holds a valid `AIBinder_Weak` pointer, so
+ // this pointer is always safe to pass to `AIBinder_Weak_lt` (null is
+ // also safe to pass to this function, but we should never do that).
+ let less_than = unsafe { sys::AIBinder_Weak_lt(self.0.as_ptr(), other.0.as_ptr()) };
+ // Safety: WpIBinder always holds a valid `AIBinder_Weak` pointer, so
+ // this pointer is always safe to pass to `AIBinder_Weak_lt` (null is
+ // also safe to pass to this function, but we should never do that).
+ let greater_than = unsafe { sys::AIBinder_Weak_lt(other.0.as_ptr(), self.0.as_ptr()) };
if !less_than && !greater_than {
Ordering::Equal
} else if less_than {
@@ -564,9 +535,9 @@
impl Drop for WpIBinder {
fn drop(&mut self) {
+ // Safety: WpIBinder always holds a valid `AIBinder_Weak` pointer, so we
+ // know this pointer is safe to pass to `AIBinder_Weak_delete` here.
unsafe {
- // Safety: WpIBinder always holds a valid `AIBinder_Weak` pointer, so we
- // know this pointer is safe to pass to `AIBinder_Weak_delete` here.
sys::AIBinder_Weak_delete(self.0.as_ptr());
}
}
@@ -592,17 +563,13 @@
cookie_decr_refcount: unsafe extern "C" fn(*mut c_void),
}
-/// # Safety
-///
-/// A `DeathRecipient` is a wrapper around `AIBinder_DeathRecipient` and a pointer
-/// to a `Fn` which is `Sync` and `Send` (the cookie field). As
+/// Safety: A `DeathRecipient` is a wrapper around `AIBinder_DeathRecipient` and
+/// a pointer to a `Fn` which is `Sync` and `Send` (the cookie field). As
/// `AIBinder_DeathRecipient` is threadsafe, this structure is too.
unsafe impl Send for DeathRecipient {}
-/// # Safety
-///
-/// A `DeathRecipient` is a wrapper around `AIBinder_DeathRecipient` and a pointer
-/// to a `Fn` which is `Sync` and `Send` (the cookie field). As
+/// Safety: A `DeathRecipient` is a wrapper around `AIBinder_DeathRecipient` and
+/// a pointer to a `Fn` which is `Sync` and `Send` (the cookie field). As
/// `AIBinder_DeathRecipient` is threadsafe, this structure is too.
unsafe impl Sync for DeathRecipient {}
@@ -614,19 +581,17 @@
F: Fn() + Send + Sync + 'static,
{
let callback: *const F = Arc::into_raw(Arc::new(callback));
- let recipient = unsafe {
- // Safety: The function pointer is a valid death recipient callback.
- //
- // This call returns an owned `AIBinder_DeathRecipient` pointer
- // which must be destroyed via `AIBinder_DeathRecipient_delete` when
- // no longer needed.
- sys::AIBinder_DeathRecipient_new(Some(Self::binder_died::<F>))
- };
+ // Safety: The function pointer is a valid death recipient callback.
+ //
+ // This call returns an owned `AIBinder_DeathRecipient` pointer which
+ // must be destroyed via `AIBinder_DeathRecipient_delete` when no longer
+ // needed.
+ let recipient = unsafe { sys::AIBinder_DeathRecipient_new(Some(Self::binder_died::<F>)) };
+ // Safety: The function pointer is a valid onUnlinked callback.
+ //
+ // All uses of linkToDeath in this file correctly increment the
+ // ref-count that this onUnlinked callback will decrement.
unsafe {
- // Safety: The function pointer is a valid onUnlinked callback.
- //
- // All uses of linkToDeath in this file correctly increment the
- // ref-count that this onUnlinked callback will decrement.
sys::AIBinder_DeathRecipient_setOnUnlinked(
recipient,
Some(Self::cookie_decr_refcount::<F>),
@@ -648,7 +613,12 @@
///
/// The caller must handle the returned ref-count correctly.
unsafe fn new_cookie(&self) -> *mut c_void {
- (self.vtable.cookie_incr_refcount)(self.cookie);
+ // Safety: `cookie_incr_refcount` points to
+ // `Self::cookie_incr_refcount`, and `self.cookie` is the cookie for an
+ // Arc<F>.
+ unsafe {
+ (self.vtable.cookie_incr_refcount)(self.cookie);
+ }
// Return a raw pointer with ownership of a ref-count
self.cookie
@@ -673,7 +643,8 @@
where
F: Fn() + Send + Sync + 'static,
{
- let callback = (cookie as *const F).as_ref().unwrap();
+ // Safety: The caller promises that `cookie` is for an Arc<F>.
+ let callback = unsafe { (cookie as *const F).as_ref().unwrap() };
callback();
}
@@ -688,7 +659,8 @@
where
F: Fn() + Send + Sync + 'static,
{
- drop(Arc::from_raw(cookie as *const F));
+ // Safety: The caller promises that `cookie` is for an Arc<F>.
+ drop(unsafe { Arc::from_raw(cookie as *const F) });
}
/// Callback that increments the ref-count.
@@ -701,15 +673,14 @@
where
F: Fn() + Send + Sync + 'static,
{
- let arc = mem::ManuallyDrop::new(Arc::from_raw(cookie as *const F));
+ // Safety: The caller promises that `cookie` is for an Arc<F>.
+ let arc = mem::ManuallyDrop::new(unsafe { Arc::from_raw(cookie as *const F) });
mem::forget(Arc::clone(&arc));
}
}
-/// # Safety
-///
-/// A `DeathRecipient` is always constructed with a valid raw pointer to an
-/// `AIBinder_DeathRecipient`, so it is always type-safe to extract this
+/// Safety: A `DeathRecipient` is always constructed with a valid raw pointer to
+/// an `AIBinder_DeathRecipient`, so it is always type-safe to extract this
/// pointer.
unsafe impl AsNative<sys::AIBinder_DeathRecipient> for DeathRecipient {
fn as_native(&self) -> *const sys::AIBinder_DeathRecipient {
@@ -723,18 +694,19 @@
impl Drop for DeathRecipient {
fn drop(&mut self) {
+ // Safety: `self.recipient` is always a valid, owned
+ // `AIBinder_DeathRecipient` pointer returned by
+ // `AIBinder_DeathRecipient_new` when `self` was created. This delete
+ // method can only be called once when `self` is dropped.
unsafe {
- // Safety: `self.recipient` is always a valid, owned
- // `AIBinder_DeathRecipient` pointer returned by
- // `AIBinder_DeathRecipient_new` when `self` was created. This
- // delete method can only be called once when `self` is dropped.
sys::AIBinder_DeathRecipient_delete(self.recipient);
+ }
- // Safety: We own a ref-count to the cookie, and so does every
- // linked binder. This call gives up our ref-count. The linked
- // binders should already have given up their ref-count, or should
- // do so shortly.
- (self.vtable.cookie_decr_refcount)(self.cookie)
+ // Safety: We own a ref-count to the cookie, and so does every linked
+ // binder. This call gives up our ref-count. The linked binders should
+ // already have given up their ref-count, or should do so shortly.
+ unsafe {
+ (self.vtable.cookie_decr_refcount)(self.cookie);
}
}
}
@@ -754,11 +726,9 @@
fn from_binder(binder: SpIBinder) -> Result<Self>;
}
-/// # Safety
-///
-/// This is a convenience method that wraps `AsNative` for `SpIBinder` to allow
-/// invocation of `IBinder` methods directly from `Interface` objects. It shares
-/// the same safety as the implementation for `SpIBinder`.
+/// Safety: This is a convenience method that wraps `AsNative` for `SpIBinder`
+/// to allow invocation of `IBinder` methods directly from `Interface` objects.
+/// It shares the same safety as the implementation for `SpIBinder`.
unsafe impl<T: Proxy> AsNative<sys::AIBinder> for T {
fn as_native(&self) -> *const sys::AIBinder {
self.as_binder().as_native()
@@ -773,24 +743,20 @@
/// exist.
pub fn get_service(name: &str) -> Option<SpIBinder> {
let name = CString::new(name).ok()?;
- unsafe {
- // Safety: `AServiceManager_getService` returns either a null pointer or
- // a valid pointer to an owned `AIBinder`. Either of these values is
- // safe to pass to `SpIBinder::from_raw`.
- SpIBinder::from_raw(sys::AServiceManager_getService(name.as_ptr()))
- }
+ // Safety: `AServiceManager_getService` returns either a null pointer or a
+ // valid pointer to an owned `AIBinder`. Either of these values is safe to
+ // pass to `SpIBinder::from_raw`.
+ unsafe { SpIBinder::from_raw(sys::AServiceManager_getService(name.as_ptr())) }
}
/// Retrieve an existing service, or start it if it is configured as a dynamic
/// service and isn't yet started.
pub fn wait_for_service(name: &str) -> Option<SpIBinder> {
let name = CString::new(name).ok()?;
- unsafe {
- // Safety: `AServiceManager_waitforService` returns either a null
- // pointer or a valid pointer to an owned `AIBinder`. Either of these
- // values is safe to pass to `SpIBinder::from_raw`.
- SpIBinder::from_raw(sys::AServiceManager_waitForService(name.as_ptr()))
- }
+ // Safety: `AServiceManager_waitforService` returns either a null pointer or
+ // a valid pointer to an owned `AIBinder`. Either of these values is safe to
+ // pass to `SpIBinder::from_raw`.
+ unsafe { SpIBinder::from_raw(sys::AServiceManager_waitForService(name.as_ptr())) }
}
/// Retrieve an existing service for a particular interface, blocking for a few
@@ -809,12 +775,10 @@
pub fn is_declared(interface: &str) -> Result<bool> {
let interface = CString::new(interface).or(Err(StatusCode::UNEXPECTED_NULL))?;
- unsafe {
- // Safety: `interface` is a valid null-terminated C-style string and is
- // only borrowed for the lifetime of the call. The `interface` local
- // outlives this call as it lives for the function scope.
- Ok(sys::AServiceManager_isDeclared(interface.as_ptr()))
- }
+ // Safety: `interface` is a valid null-terminated C-style string and is only
+ // borrowed for the lifetime of the call. The `interface` local outlives
+ // this call as it lives for the function scope.
+ unsafe { Ok(sys::AServiceManager_isDeclared(interface.as_ptr())) }
}
/// Retrieve all declared instances for a particular interface
@@ -827,11 +791,13 @@
// CString, and outlives this callback. The null handling here is just
// to avoid the possibility of unwinding across C code if this crate is
// ever compiled with panic=unwind.
- if let Some(instances) = opaque.cast::<Vec<CString>>().as_mut() {
+ if let Some(instances) = unsafe { opaque.cast::<Vec<CString>>().as_mut() } {
// Safety: instance is a valid null-terminated C string with a
// lifetime at least as long as this function, and we immediately
// copy it into an owned CString.
- instances.push(CStr::from_ptr(instance).to_owned());
+ unsafe {
+ instances.push(CStr::from_ptr(instance).to_owned());
+ }
} else {
eprintln!("Opaque pointer was null in get_declared_instances callback!");
}
@@ -839,10 +805,10 @@
let interface = CString::new(interface).or(Err(StatusCode::UNEXPECTED_NULL))?;
let mut instances: Vec<CString> = vec![];
+ // Safety: `interface` and `instances` are borrowed for the length of this
+ // call and both outlive the call. `interface` is guaranteed to be a valid
+ // null-terminated C-style string.
unsafe {
- // Safety: `interface` and `instances` are borrowed for the length of
- // this call and both outlive the call. `interface` is guaranteed to be
- // a valid null-terminated C-style string.
sys::AServiceManager_forEachDeclaredInstance(
interface.as_ptr(),
&mut instances as *mut _ as *mut c_void,
@@ -860,10 +826,8 @@
})
}
-/// # Safety
-///
-/// `SpIBinder` guarantees that `binder` always contains a valid pointer to an
-/// `AIBinder`, so we can trivially extract this pointer here.
+/// Safety: `SpIBinder` guarantees that `binder` always contains a valid pointer
+/// to an `AIBinder`, so we can trivially extract this pointer here.
unsafe impl AsNative<sys::AIBinder> for SpIBinder {
fn as_native(&self) -> *const sys::AIBinder {
self.0.as_ptr()
diff --git a/libs/binder/rust/src/state.rs b/libs/binder/rust/src/state.rs
index 4886c5f..a3a2562 100644
--- a/libs/binder/rust/src/state.rs
+++ b/libs/binder/rust/src/state.rs
@@ -35,8 +35,8 @@
/// not work: the callbacks will be queued but never called as there is no
/// thread to call them on.
pub fn start_thread_pool() {
+ // Safety: Safe FFI
unsafe {
- // Safety: Safe FFI
sys::ABinderProcess_startThreadPool();
}
}
@@ -48,8 +48,8 @@
/// called, this is 15. If it is called additional times, the thread pool
/// size can only be increased.
pub fn set_thread_pool_max_thread_count(num_threads: u32) {
+ // Safety: Safe FFI
unsafe {
- // Safety: Safe FFI
sys::ABinderProcess_setThreadPoolMaxThreadCount(num_threads);
}
}
@@ -62,8 +62,8 @@
/// [`set_thread_pool_max_thread_count`](Self::set_thread_pool_max_thread_count)
/// and [`start_thread_pool`](Self::start_thread_pool).
pub fn join_thread_pool() {
+ // Safety: Safe FFI
unsafe {
- // Safety: Safe FFI
sys::ABinderProcess_joinThreadPool();
}
}
@@ -86,10 +86,8 @@
/// \return calling uid or the current process's UID if this thread isn't
/// processing a transaction.
pub fn get_calling_uid() -> uid_t {
- unsafe {
- // Safety: Safe FFI
- sys::AIBinder_getCallingUid()
- }
+ // Safety: Safe FFI
+ unsafe { sys::AIBinder_getCallingUid() }
}
/// This returns the calling PID assuming that this thread is called from a
@@ -111,10 +109,8 @@
/// If the transaction being processed is a oneway transaction, then this
/// method will return 0.
pub fn get_calling_pid() -> pid_t {
- unsafe {
- // Safety: Safe FFI
- sys::AIBinder_getCallingPid()
- }
+ // Safety: Safe FFI
+ unsafe { sys::AIBinder_getCallingPid() }
}
/// Determine whether the current thread is currently executing an incoming transaction.
@@ -122,10 +118,8 @@
/// \return true if the current thread is currently executing an incoming transaction, and false
/// otherwise.
pub fn is_handling_transaction() -> bool {
- unsafe {
- // Safety: Safe FFI
- sys::AIBinder_isHandlingTransaction()
- }
+ // Safety: Safe FFI
+ unsafe { sys::AIBinder_isHandlingTransaction() }
}
/// This function makes the client's security context available to the