Merge "libbinder_ndk: finalizeTransaction->AParcel_delete" am: d56b315086 am: 7f88835e58
am: 4e57182d0d
Change-Id: Ie609133b3941366423cc6ba88066ab0c74d4003b
diff --git a/libs/binder/ndk/AIBinder.cpp b/libs/binder/ndk/AIBinder.cpp
index 58cbf56..40d3a00 100644
--- a/libs/binder/ndk/AIBinder.cpp
+++ b/libs/binder/ndk/AIBinder.cpp
@@ -279,12 +279,6 @@
return status;
}
-using AutoParcelDestroyer = std::unique_ptr<AParcel*, void (*)(AParcel**)>;
-static void destroy_parcel(AParcel** parcel) {
- delete *parcel;
- *parcel = nullptr;
-}
-
binder_status_t AIBinder_transact(AIBinder* binder, transaction_code_t code, AParcel** in,
AParcel** out, binder_flags_t flags) {
if (in == nullptr) {
@@ -292,9 +286,10 @@
return EX_NULL_POINTER;
}
+ using AutoParcelDestroyer = std::unique_ptr<AParcel*, void (*)(AParcel**)>;
// This object is the input to the transaction. This function takes ownership of it and deletes
// it.
- AutoParcelDestroyer forIn(in, destroy_parcel);
+ AutoParcelDestroyer forIn(in, AParcel_delete);
if (!isUserCommand(code)) {
LOG(ERROR) << __func__ << ": Only user-defined transactions can be made from the NDK.";
@@ -329,33 +324,3 @@
return parcelStatus;
}
-
-binder_status_t AIBinder_finalizeTransaction(AIBinder* binder, AParcel** out) {
- if (out == nullptr) {
- LOG(ERROR) << __func__ << ": requires non-null out parameter";
- return EX_NULL_POINTER;
- }
-
- // This object is the input to the transaction. This function takes ownership of it and deletes
- // it.
- AutoParcelDestroyer forOut(out, destroy_parcel);
-
- if (binder == nullptr || *out == nullptr) {
- LOG(ERROR) << __func__ << ": requires non-null parameters.";
- return EX_NULL_POINTER;
- }
-
- if ((*out)->getBinder() != binder) {
- LOG(ERROR) << __func__ << ": parcel is associated with binder object " << binder
- << " but called with " << (*out)->getBinder();
- return EX_ILLEGAL_STATE;
- }
-
- if ((**out)->dataAvail() != 0) {
- LOG(ERROR) << __func__
- << ": Only part of this transaction was read. There is remaining data left.";
- return EX_ILLEGAL_STATE;
- }
-
- return EX_NONE;
-}
diff --git a/libs/binder/ndk/AParcel.cpp b/libs/binder/ndk/AParcel.cpp
index f39e732..93384d6 100644
--- a/libs/binder/ndk/AParcel.cpp
+++ b/libs/binder/ndk/AParcel.cpp
@@ -25,6 +25,15 @@
using ::android::Parcel;
using ::android::sp;
+void AParcel_delete(AParcel** parcel) {
+ if (parcel == nullptr) {
+ return;
+ }
+
+ delete *parcel;
+ *parcel = nullptr;
+}
+
binder_status_t AParcel_writeStrongBinder(AParcel* parcel, AIBinder* binder) {
return (*parcel)->writeStrongBinder(binder->getBinder());
}
diff --git a/libs/binder/ndk/include_ndk/android/binder_ibinder.h b/libs/binder/ndk/include_ndk/android/binder_ibinder.h
index f752008..91a87fd 100644
--- a/libs/binder/ndk/include_ndk/android/binder_ibinder.h
+++ b/libs/binder/ndk/include_ndk/android/binder_ibinder.h
@@ -186,10 +186,9 @@
/**
* A transaction is a series of calls to these functions which looks this
* - call AIBinder_prepareTransaction
- * - fill out parcel with in parameters (lifetime of the 'in' variable)
+ * - fill out the in parcel with parameters (lifetime of the 'in' variable)
* - call AIBinder_transact
- * - fill out parcel with out parameters (lifetime of the 'out' variable)
- * - call AIBinder_finalizeTransaction
+ * - read results from the out parcel (lifetime of the 'out' variable)
*/
/**
@@ -200,7 +199,10 @@
* can be avoided. This AIBinder must be either built with a class or associated with a class before
* using this API.
*
- * This does not affect the ownership of binder.
+ * This does not affect the ownership of binder. When this function succeeds, the in parcel's
+ * ownership is passed to the caller. At this point, the parcel can be filled out and passed to
+ * AIBinder_transact. Alternatively, if there is an error while filling out the parcel, it can be
+ * deleted with AParcel_delete.
*/
binder_status_t AIBinder_prepareTransaction(AIBinder* binder, AParcel** in);
@@ -213,19 +215,12 @@
* remote process has processed the transaction, and the out parcel will contain the output data
* from transaction.
*
- * This does not affect the ownership of binder.
+ * This does not affect the ownership of binder. The out parcel's ownership is passed to the caller
+ * and must be released with AParcel_delete when finished reading.
*/
binder_status_t AIBinder_transact(AIBinder* binder, transaction_code_t code, AParcel** in,
AParcel** out, binder_flags_t flags);
-/**
- * This takes ownership of the out parcel and automatically deletes it. Additional checks for
- * security or debugging maybe performed internally.
- *
- * This does not affect the ownership of binder.
- */
-binder_status_t AIBinder_finalizeTransaction(AIBinder* binder, AParcel** out);
-
/*
* This does not take any ownership of the input binder, but it can be used to retrieve it if
* something else in some process still holds a reference to it.
diff --git a/libs/binder/ndk/include_ndk/android/binder_parcel.h b/libs/binder/ndk/include_ndk/android/binder_parcel.h
index 091ae8e..19925f7 100644
--- a/libs/binder/ndk/include_ndk/android/binder_parcel.h
+++ b/libs/binder/ndk/include_ndk/android/binder_parcel.h
@@ -35,6 +35,11 @@
typedef struct AParcel AParcel;
/**
+ * Cleans up a parcel and sets it to nullptr.
+ */
+void AParcel_delete(AParcel** parcel);
+
+/**
* Writes an AIBinder to the next location in a non-null parcel. Can be null.
*/
binder_status_t AParcel_writeStrongBinder(AParcel* parcel, AIBinder* binder);
diff --git a/libs/binder/ndk/test/iface.cpp b/libs/binder/ndk/test/iface.cpp
index 27553c8..a1aa0fa 100644
--- a/libs/binder/ndk/test/iface.cpp
+++ b/libs/binder/ndk/test/iface.cpp
@@ -81,7 +81,8 @@
int32_t out;
CHECK(EX_NONE == AParcel_readInt32(parcelOut, &out));
- CHECK(EX_NONE == AIBinder_finalizeTransaction(mBinder, &parcelOut));
+ AParcel_delete(&parcelOut);
+
return out;
}