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
diff --git a/libs/binder/ndk/ibinder.cpp b/libs/binder/ndk/ibinder.cpp
index b1b4194..71310ff 100644
--- a/libs/binder/ndk/ibinder.cpp
+++ b/libs/binder/ndk/ibinder.cpp
@@ -198,13 +198,8 @@
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 @@
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 @@
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 @@
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 5081248..871335d 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 @@
/**
* 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 @@
* 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 8628025..271810a 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 @@
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 d9c36d2..206bf40 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 @@
/**
* 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 a063657..385e898 100644
--- a/libs/binder/ndk/parcel.cpp
+++ b/libs/binder/ndk/parcel.cpp
@@ -27,13 +27,8 @@
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 deb0392..549e4b3 100644
--- a/libs/binder/ndk/status.cpp
+++ b/libs/binder/ndk/status.cpp
@@ -66,13 +66,8 @@
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 80700df..0dc3cc4 100644
--- a/libs/binder/ndk/test/iface.cpp
+++ b/libs/binder/ndk/test/iface.cpp
@@ -81,7 +81,7 @@
int32_t out;
CHECK(STATUS_OK == AParcel_readInt32(parcelOut, &out));
- AParcel_delete(&parcelOut);
+ AParcel_delete(parcelOut);
return out;
}
@@ -92,7 +92,7 @@
};
IFoo::~IFoo() {
- AIBinder_Weak_delete(&mWeakBinder);
+ AIBinder_Weak_delete(mWeakBinder);
}
binder_status_t IFoo::addService(const char* instance) {
@@ -106,7 +106,7 @@
// 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 2dcccfe..c07462e 100644
--- a/libs/binder/ndk/test/main_client.cpp
+++ b/libs/binder/ndk/test/main_client.cpp
@@ -66,7 +66,7 @@
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 @@
// 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) {