libbinder: attachObject* APIs [[nodiscard]]
Make sure we're catching the recently exposed error conditions for these
(turns out these APIs are almost used nowhere!).
Fixes: 192023359
Test: boot, CtsNdkBinderTestCases
Change-Id: If6cdcf18520e874b28b33b80c5b4294ed8d4a302
diff --git a/libs/binder/ServiceManagerHost.cpp b/libs/binder/ServiceManagerHost.cpp
index 07f5778..1c2f9b4 100644
--- a/libs/binder/ServiceManagerHost.cpp
+++ b/libs/binder/ServiceManagerHost.cpp
@@ -167,9 +167,12 @@
ALOGE("RpcSession::getRootObject returns nullptr");
return nullptr;
}
- binder->attachObject(kDeviceServiceExtraId,
- static_cast<void*>(new CommandResult(std::move(*result))), nullptr,
- &cleanupCommandResult);
+
+ LOG_ALWAYS_FATAL_IF(
+ nullptr !=
+ binder->attachObject(kDeviceServiceExtraId,
+ static_cast<void*>(new CommandResult(std::move(*result))), nullptr,
+ &cleanupCommandResult));
return binder;
}
diff --git a/libs/binder/include/binder/IBinder.h b/libs/binder/include/binder/IBinder.h
index c484d83..43fc5ff 100644
--- a/libs/binder/include/binder/IBinder.h
+++ b/libs/binder/include/binder/IBinder.h
@@ -259,21 +259,20 @@
* but calls from different threads are allowed to be interleaved.
*
* This returns the object which is already attached. If this returns a
- * non-null value, it means that attachObject failed. TODO(b/192023359):
- * remove logs and add [[nodiscard]]
+ * non-null value, it means that attachObject failed (a given objectID can
+ * only be used once).
*/
- virtual void* attachObject(const void* objectID, void* object, void* cleanupCookie,
- object_cleanup_func func) = 0;
+ [[nodiscard]] virtual void* attachObject(const void* objectID, void* object,
+ void* cleanupCookie, object_cleanup_func func) = 0;
/**
* Returns object attached with attachObject.
*/
- virtual void* findObject(const void* objectID) const = 0;
+ [[nodiscard]] virtual void* findObject(const void* objectID) const = 0;
/**
* Returns object attached with attachObject, and detaches it. This does not
- * delete the object. This is equivalent to using attachObject to attach a null
- * object.
+ * delete the object.
*/
- virtual void* detachObject(const void* objectID) = 0;
+ [[nodiscard]] virtual void* detachObject(const void* objectID) = 0;
/**
* Use the lock that this binder contains internally. For instance, this can
diff --git a/libs/binder/ndk/ibinder.cpp b/libs/binder/ndk/ibinder.cpp
index 266ef37..b89714a 100644
--- a/libs/binder/ndk/ibinder.cpp
+++ b/libs/binder/ndk/ibinder.cpp
@@ -47,7 +47,8 @@
void clean(const void* /*id*/, void* /*obj*/, void* /*cookie*/){/* do nothing */};
static void attach(const sp<IBinder>& binder) {
- binder->attachObject(kId, kValue, nullptr /*cookie*/, clean);
+ // can only attach once
+ CHECK_EQ(nullptr, binder->attachObject(kId, kValue, nullptr /*cookie*/, clean));
}
static bool has(const sp<IBinder>& binder) {
return binder != nullptr && binder->findObject(kId) == kValue;
diff --git a/libs/binder/tests/unit_fuzzers/IBinderFuzzFunctions.h b/libs/binder/tests/unit_fuzzers/IBinderFuzzFunctions.h
index 626b758..4a0aeba 100644
--- a/libs/binder/tests/unit_fuzzers/IBinderFuzzFunctions.h
+++ b/libs/binder/tests/unit_fuzzers/IBinderFuzzFunctions.h
@@ -62,20 +62,22 @@
object = fdp->ConsumeIntegral<uint32_t>();
cleanup_cookie = fdp->ConsumeIntegral<uint32_t>();
IBinder::object_cleanup_func func = IBinder::object_cleanup_func();
- ibinder->attachObject(fdp->ConsumeBool() ? reinterpret_cast<void*>(&objectID)
- : nullptr,
- fdp->ConsumeBool() ? reinterpret_cast<void*>(&object) : nullptr,
- fdp->ConsumeBool() ? reinterpret_cast<void*>(&cleanup_cookie)
- : nullptr,
- func);
+ (void)ibinder->attachObject(fdp->ConsumeBool() ? reinterpret_cast<void*>(&objectID)
+ : nullptr,
+ fdp->ConsumeBool() ? reinterpret_cast<void*>(&object)
+ : nullptr,
+ fdp->ConsumeBool()
+ ? reinterpret_cast<void*>(&cleanup_cookie)
+ : nullptr,
+ func);
},
[](FuzzedDataProvider* fdp, IBinder* ibinder) -> void {
uint32_t id = fdp->ConsumeIntegral<uint32_t>();
- ibinder->findObject(reinterpret_cast<void*>(&id));
+ (void)ibinder->findObject(reinterpret_cast<void*>(&id));
},
[](FuzzedDataProvider* fdp, IBinder* ibinder) -> void {
uint32_t id = fdp->ConsumeIntegral<uint32_t>();
- ibinder->detachObject(reinterpret_cast<void*>(&id));
+ (void)ibinder->detachObject(reinterpret_cast<void*>(&id));
},
[](FuzzedDataProvider* fdp, IBinder* ibinder) -> void {
uint32_t code = fdp->ConsumeIntegral<uint32_t>();