libbinder_ndk: _delete APIs take * not **.

Since having ** requires having a pointer allocated on the stack. This
is pretty common if you are using an object within a scope, but causes
problems when it is not.

Bug: 111445392
Test: ./ndk/runtests.sh
Change-Id: If7586548af0b47d8e09178fc3b71af50d449e290

8 files changed, 24 insertions, 39 deletions

diff --git a/libs/binder/ndk/ibinder.cpp b/libs/binder/ndk/ibinder.cpp
index b1b419408f..71310ff745 100644
--- a/libs/binder/ndk/ibinder.cpp
+++ b/libs/binder/ndk/ibinder.cpp
@@ -198,13 +198,8 @@ AIBinder_Weak* AIBinder_Weak_new(AIBinder* binder) {
 
     return new AIBinder_Weak{wp<AIBinder>(binder)};
 }
-void AIBinder_Weak_delete(AIBinder_Weak** weakBinder) {
-    if (weakBinder == nullptr) {
-        return;
-    }
-
-    delete *weakBinder;
-    *weakBinder = nullptr;
+void AIBinder_Weak_delete(AIBinder_Weak* weakBinder) {
+    delete weakBinder;
 }
 AIBinder* AIBinder_Weak_promote(AIBinder_Weak* weakBinder) {
     if (weakBinder == nullptr) {
@@ -437,6 +432,11 @@ binder_status_t AIBinder_prepareTransaction(AIBinder* binder, AParcel** in) {
     return ret;
 }
 
+static void DestroyParcel(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) {
@@ -447,7 +447,7 @@ binder_status_t AIBinder_transact(AIBinder* binder, transaction_code_t code, APa
     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, AParcel_delete);
+    AutoParcelDestroyer forIn(in, DestroyParcel);
 
     if (!isUserCommand(code)) {
         LOG(ERROR) << __func__ << ": Only user-defined transactions can be made from the NDK.";
@@ -492,11 +492,6 @@ AIBinder_DeathRecipient* AIBinder_DeathRecipient_new(
     return new AIBinder_DeathRecipient(onBinderDied);
 }
 
-void AIBinder_DeathRecipient_delete(AIBinder_DeathRecipient** recipient) {
-    if (recipient == nullptr) {
-        return;
-    }
-
-    delete *recipient;
-    *recipient = nullptr;
+void AIBinder_DeathRecipient_delete(AIBinder_DeathRecipient* recipient) {
+    delete recipient;
 }
diff --git a/libs/binder/ndk/include_ndk/android/binder_ibinder.h b/libs/binder/ndk/include_ndk/android/binder_ibinder.h
index 50812489c7..871335d79d 100644
--- a/libs/binder/ndk/include_ndk/android/binder_ibinder.h
+++ b/libs/binder/ndk/include_ndk/android/binder_ibinder.h
@@ -300,7 +300,7 @@ __attribute__((warn_unused_result)) AIBinder_Weak* AIBinder_Weak_new(AIBinder* b
 /**
  * Deletes the weak reference. This will have no impact on the lifetime of the binder.
  */
-void AIBinder_Weak_delete(AIBinder_Weak** weakBinder);
+void AIBinder_Weak_delete(AIBinder_Weak* weakBinder);
 
 /**
  * If promotion succeeds, result will have one strong refcount added to it. Otherwise, this returns
@@ -323,7 +323,7 @@ __attribute__((warn_unused_result)) AIBinder_DeathRecipient* AIBinder_DeathRecip
  * Deletes a binder death recipient. It is not necessary to call AIBinder_unlinkToDeath before
  * calling this as these will all be automatically unlinked.
  */
-void AIBinder_DeathRecipient_delete(AIBinder_DeathRecipient** recipient);
+void AIBinder_DeathRecipient_delete(AIBinder_DeathRecipient* recipient);
 
 __END_DECLS
 
diff --git a/libs/binder/ndk/include_ndk/android/binder_parcel.h b/libs/binder/ndk/include_ndk/android/binder_parcel.h
index 862802523e..271810af6d 100644
--- a/libs/binder/ndk/include_ndk/android/binder_parcel.h
+++ b/libs/binder/ndk/include_ndk/android/binder_parcel.h
@@ -45,9 +45,9 @@ struct AParcel;
 typedef struct AParcel AParcel;
 
 /**
- * Cleans up a parcel and sets it to nullptr.
+ * Cleans up a parcel.
  */
-void AParcel_delete(AParcel** parcel);
+void AParcel_delete(AParcel* parcel);
 
 /**
  * Writes an AIBinder to the next location in a non-null parcel. Can be null.
diff --git a/libs/binder/ndk/include_ndk/android/binder_status.h b/libs/binder/ndk/include_ndk/android/binder_status.h
index d9c36d219b..206bf40113 100644
--- a/libs/binder/ndk/include_ndk/android/binder_status.h
+++ b/libs/binder/ndk/include_ndk/android/binder_status.h
@@ -174,7 +174,7 @@ const char* AStatus_getMessage(const AStatus* status);
 /**
  * Deletes memory associated with the status instance.
  */
-void AStatus_delete(AStatus** status);
+void AStatus_delete(AStatus* status);
 
 __END_DECLS
 
diff --git a/libs/binder/ndk/parcel.cpp b/libs/binder/ndk/parcel.cpp
index a063657dd6..385e898aeb 100644
--- a/libs/binder/ndk/parcel.cpp
+++ b/libs/binder/ndk/parcel.cpp
@@ -27,13 +27,8 @@ using ::android::Parcel;
 using ::android::sp;
 using ::android::status_t;
 
-void AParcel_delete(AParcel** parcel) {
-    if (parcel == nullptr) {
-        return;
-    }
-
-    delete *parcel;
-    *parcel = nullptr;
+void AParcel_delete(AParcel* parcel) {
+    delete parcel;
 }
 
 binder_status_t AParcel_writeStrongBinder(AParcel* parcel, AIBinder* binder) {
diff --git a/libs/binder/ndk/status.cpp b/libs/binder/ndk/status.cpp
index deb0392cc7..549e4b3b0a 100644
--- a/libs/binder/ndk/status.cpp
+++ b/libs/binder/ndk/status.cpp
@@ -66,13 +66,8 @@ const char* AStatus_getMessage(const AStatus* status) {
     return status->get()->exceptionMessage().c_str();
 }
 
-void AStatus_delete(AStatus** status) {
-    if (status == nullptr) {
-        return;
-    }
-
-    delete *status;
-    *status = nullptr;
+void AStatus_delete(AStatus* status) {
+    delete status;
 }
 
 binder_status_t PruneStatusT(status_t status) {
diff --git a/libs/binder/ndk/test/iface.cpp b/libs/binder/ndk/test/iface.cpp
index 80700dfe3e..0dc3cc4f95 100644
--- a/libs/binder/ndk/test/iface.cpp
+++ b/libs/binder/ndk/test/iface.cpp
@@ -81,7 +81,7 @@ public:
         int32_t out;
         CHECK(STATUS_OK == AParcel_readInt32(parcelOut, &out));
 
-        AParcel_delete(&parcelOut);
+        AParcel_delete(parcelOut);
 
         return out;
     }
@@ -92,7 +92,7 @@ private:
 };
 
 IFoo::~IFoo() {
-    AIBinder_Weak_delete(&mWeakBinder);
+    AIBinder_Weak_delete(mWeakBinder);
 }
 
 binder_status_t IFoo::addService(const char* instance) {
@@ -106,7 +106,7 @@ binder_status_t IFoo::addService(const char* instance) {
         // or one strong refcount here
         binder = AIBinder_new(IFoo::kClass, static_cast<void*>(new IFoo_Class_Data{this}));
         if (mWeakBinder != nullptr) {
-            AIBinder_Weak_delete(&mWeakBinder);
+            AIBinder_Weak_delete(mWeakBinder);
         }
         mWeakBinder = AIBinder_Weak_new(binder);
     }
diff --git a/libs/binder/ndk/test/main_client.cpp b/libs/binder/ndk/test/main_client.cpp
index 2dcccfefdd..c07462e84d 100644
--- a/libs/binder/ndk/test/main_client.cpp
+++ b/libs/binder/ndk/test/main_client.cpp
@@ -66,7 +66,7 @@ TEST(NdkBinder, LinkToDeath) {
     EXPECT_EQ(STATUS_OK, AIBinder_unlinkToDeath(binder, recipient, nullptr));
     EXPECT_EQ(STATUS_NAME_NOT_FOUND, AIBinder_unlinkToDeath(binder, recipient, nullptr));
 
-    AIBinder_DeathRecipient_delete(&recipient);
+    AIBinder_DeathRecipient_delete(recipient);
     AIBinder_decStrong(binder);
 }
 
@@ -114,7 +114,7 @@ TEST(NdkBinder, ABpBinderRefCount) {
     // assert because would need to decStrong if non-null and we shouldn't need to add a no-op here
     ASSERT_NE(nullptr, AIBinder_Weak_promote(wBinder));
 
-    AIBinder_Weak_delete(&wBinder);
+    AIBinder_Weak_delete(wBinder);
 }
 
 TEST(NdkBinder, AddServiceMultipleTimes) {