Merge "rust: split transact into prepare and submit" (78793f22) · Commits · e / os / android_frameworks_native

libs/binder/rust/src/binder.rs

+29 −3

Original line number	Diff line number	Diff line
		@@ -152,20 +152,46 @@ pub trait IBinderInternal: IBinder {
		/// available.
		fn get_extension(&mut self) -> Result<Option<SpIBinder>>;

		/// Create a Parcel that can be used with `submit_transact`.
		fn prepare_transact(&self) -> Result<Parcel>;

		/// Perform a generic operation with the object.
		///
		/// The provided [`Parcel`] must have been created by a call to
		/// `prepare_transact` on the same binder.
		///
		/// # Arguments
		///
		/// * `code` - Transaction code for the operation.
		/// * `data` - [`Parcel`] with input data.
		/// * `flags` - Transaction flags, e.g. marking the transaction as
		/// asynchronous ([`FLAG_ONEWAY`](FLAG_ONEWAY)).
		fn submit_transact(
		&self,
		code: TransactionCode,
		data: Parcel,
		flags: TransactionFlags,
		) -> Result<Parcel>;

		/// Perform a generic operation with the object. This is a convenience
		/// method that internally calls `prepare_transact` followed by
		/// `submit_transact.
		///
		/// # Arguments
		/// * `code` - Transaction code for the operation
		/// * `data` - [`Parcel`] with input data
		/// * `reply` - Optional [`Parcel`] for reply data
		/// * `flags` - Transaction flags, e.g. marking the transaction as
		/// asynchronous ([`FLAG_ONEWAY`](FLAG_ONEWAY))
		/// * `input_callback` A callback for building the `Parcel`.
		fn transact<F: FnOnce(&mut Parcel) -> Result<()>>(
		&self,
		code: TransactionCode,
		flags: TransactionFlags,
		input_callback: F,
		) -> Result<Parcel>;
		) -> Result<Parcel> {
		let mut parcel = self.prepare_transact()?;
		input_callback(&mut parcel)?;
		self.submit_transact(code, parcel, flags)
		}
		}

		/// Interface of binder local or remote objects.

libs/binder/rust/src/parcel.rs

+15 −0

Original line number	Diff line number	Diff line
		@@ -25,6 +25,7 @@ use std::cell::RefCell;
		use std::convert::TryInto;
		use std::mem::ManuallyDrop;
		use std::ptr;
		use std::fmt;

		mod file_descriptor;
		mod parcelable;
		@@ -96,6 +97,13 @@ impl Parcel {
		let _ = ManuallyDrop::new(self);
		ptr
		}

		pub(crate) fn is_owned(&self) -> bool {
		match *self {
		Self::Owned(_) => true,
		Self::Borrowed(_) => false,
		}
		}
		}

		// Data serialization methods
		@@ -412,6 +420,13 @@ impl Drop for Parcel {
		}
		}

		impl fmt::Debug for Parcel {
		fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
		f.debug_struct("Parcel")
		.finish()
		}
		}

		#[cfg(test)]
		impl Parcel {
		/// Create a new parcel tied to a bogus binder. TESTING ONLY!

libs/binder/rust/src/proxy.rs

+17 −13

Original line number	Diff line number	Diff line
		@@ -235,13 +235,7 @@ impl Drop for SpIBinder {
		}

		impl<T: AsNative<sys::AIBinder>> IBinderInternal for T {
		/// Perform a binder transaction
		fn transact<F: FnOnce(&mut Parcel) -> Result<()>>(
		&self,
		code: TransactionCode,
		flags: TransactionFlags,
		input_callback: F,
		) -> Result<Parcel> {
		fn prepare_transact(&self) -> Result<Parcel> {
		let mut input = ptr::null_mut();
		let status = unsafe {
		// Safety: `SpIBinder` guarantees that `self` always contains a
		@@ -254,15 +248,25 @@ impl<T: AsNative<sys::AIBinder>> IBinderInternal for T {
		// pointer, or null.
		sys::AIBinder_prepareTransaction(self.as_native() as *mut sys::AIBinder, &mut input)
		};

		status_result(status)?;
		let mut input = unsafe {

		unsafe {
		// Safety: At this point, `input` is either a valid, owned `AParcel`
		// pointer, or null. `Parcel::owned` safely handles both cases,
		// taking ownership of the parcel.
		Parcel::owned(input).ok_or(StatusCode::UNEXPECTED_NULL)?
		};
		input_callback(&mut input)?;
		Parcel::owned(input).ok_or(StatusCode::UNEXPECTED_NULL)
		}
		}

		fn submit_transact(
		&self,
		code: TransactionCode,
		data: Parcel,
		flags: TransactionFlags,
		) -> Result<Parcel> {
		let mut reply = ptr::null_mut();
		assert!(data.is_owned());
		let status = unsafe {
		// Safety: `SpIBinder` guarantees that `self` always contains a
		// valid pointer to an `AIBinder`. Although `IBinder::transact` is
		@@ -277,13 +281,13 @@ impl<T: AsNative<sys::AIBinder>> IBinderInternal for T {
		// only providing `on_transact` with an immutable reference to
		// `self`.
		//
		// This call takes ownership of the `input` parcel pointer, and
		// 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,
		&mut input.into_raw(),
		&mut data.into_raw(),
		&mut reply,
		flags,
		)

libs/binder/rust/tests/integration.rs

+23 −0

Original line number	Diff line number	Diff line
		@@ -697,4 +697,27 @@ mod tests {
		assert!(!(service1 > service1));
		assert_eq!(service1 < service2, !(service2 < service1));
		}

		#[test]
		fn binder_parcel_mixup() {
		let service1 = BnTest::new_binder(
		TestService::new("testing_service1"),
		BinderFeatures::default(),
		);
		let service2 = BnTest::new_binder(
		TestService::new("testing_service2"),
		BinderFeatures::default(),
		);

		let service1 = service1.as_binder();
		let service2 = service2.as_binder();

		let parcel = service1.prepare_transact().unwrap();
		let res = service2.submit_transact(super::TestTransactionCode::Test as binder::TransactionCode, parcel, 0);

		match res {
		Ok(_) => panic!("submit_transact should fail"),
		Err(err) => assert_eq!(err, binder::StatusCode::BAD_VALUE),
		}
		}
		}