diff options
| -rw-r--r-- | cmds/servicemanager/Android.bp | 1 | ||||
| -rw-r--r-- | cmds/servicemanager/ServiceManager.cpp | 12 | ||||
| -rw-r--r-- | cmds/servicemanager/main.cpp | 7 | ||||
| -rw-r--r-- | cmds/servicemanager/test_sm.cpp | 38 | ||||
| -rw-r--r-- | libs/binder/include/binder/IInterface.h | 2 |
5 files changed, 33 insertions, 27 deletions
diff --git a/cmds/servicemanager/Android.bp b/cmds/servicemanager/Android.bp index 9de344a820..3ebdeee7aa 100644 --- a/cmds/servicemanager/Android.bp +++ b/cmds/servicemanager/Android.bp @@ -14,6 +14,7 @@ cc_defaults { "-Wall", "-Wextra", "-Werror", + "-DANDROID_UTILS_REF_BASE_DISABLE_IMPLICIT_CONSTRUCTION", ], srcs: [ diff --git a/cmds/servicemanager/ServiceManager.cpp b/cmds/servicemanager/ServiceManager.cpp index 0dbab4e055..2f5524940e 100644 --- a/cmds/servicemanager/ServiceManager.cpp +++ b/cmds/servicemanager/ServiceManager.cpp @@ -239,7 +239,8 @@ Status ServiceManager::addService(const std::string& name, const sp<IBinder>& bi #endif // !VENDORSERVICEMANAGER // implicitly unlinked when the binder is removed - if (binder->remoteBinder() != nullptr && binder->linkToDeath(this) != OK) { + if (binder->remoteBinder() != nullptr && + binder->linkToDeath(sp<ServiceManager>::fromExisting(this)) != OK) { LOG(ERROR) << "Could not linkToDeath when adding " << name; return Status::fromExceptionCode(Status::EX_ILLEGAL_STATE); } @@ -307,7 +308,9 @@ Status ServiceManager::registerForNotifications( return Status::fromExceptionCode(Status::EX_NULL_POINTER); } - if (OK != IInterface::asBinder(callback)->linkToDeath(this)) { + if (OK != + IInterface::asBinder(callback)->linkToDeath( + sp<ServiceManager>::fromExisting(this))) { LOG(ERROR) << "Could not linkToDeath when adding " << name; return Status::fromExceptionCode(Status::EX_ILLEGAL_STATE); } @@ -461,7 +464,8 @@ Status ServiceManager::registerClientCallback(const std::string& name, const sp< return Status::fromExceptionCode(Status::EX_ILLEGAL_ARGUMENT); } - if (OK != IInterface::asBinder(cb)->linkToDeath(this)) { + if (OK != + IInterface::asBinder(cb)->linkToDeath(sp<ServiceManager>::fromExisting(this))) { LOG(ERROR) << "Could not linkToDeath when adding client callback for " << name; return Status::fromExceptionCode(Status::EX_ILLEGAL_STATE); } @@ -491,7 +495,7 @@ void ServiceManager::removeClientCallback(const wp<IBinder>& who, } ssize_t ServiceManager::Service::getNodeStrongRefCount() { - sp<BpBinder> bpBinder = binder->remoteBinder(); + sp<BpBinder> bpBinder = sp<BpBinder>::fromExisting(binder->remoteBinder()); if (bpBinder == nullptr) return -1; return ProcessState::self()->getStrongRefCountForNode(bpBinder); diff --git a/cmds/servicemanager/main.cpp b/cmds/servicemanager/main.cpp index 627dfe6382..8c1beaca20 100644 --- a/cmds/servicemanager/main.cpp +++ b/cmds/servicemanager/main.cpp @@ -39,7 +39,7 @@ using ::android::sp; class BinderCallback : public LooperCallback { public: static sp<BinderCallback> setupTo(const sp<Looper>& looper) { - sp<BinderCallback> cb = new BinderCallback; + sp<BinderCallback> cb = sp<BinderCallback>::make(); int binder_fd = -1; IPCThreadState::self()->setupPolling(&binder_fd); @@ -65,7 +65,7 @@ public: class ClientCallbackCallback : public LooperCallback { public: static sp<ClientCallbackCallback> setupTo(const sp<Looper>& looper, const sp<ServiceManager>& manager) { - sp<ClientCallbackCallback> cb = new ClientCallbackCallback(manager); + sp<ClientCallbackCallback> cb = sp<ClientCallbackCallback>::make(manager); int fdTimer = timerfd_create(CLOCK_MONOTONIC, 0 /*flags*/); LOG_ALWAYS_FATAL_IF(fdTimer < 0, "Failed to timerfd_create: fd: %d err: %d", fdTimer, errno); @@ -105,6 +105,7 @@ public: return 1; // Continue receiving callbacks. } private: + friend sp<ClientCallbackCallback>; ClientCallbackCallback(const sp<ServiceManager>& manager) : mManager(manager) {} sp<ServiceManager> mManager; }; @@ -120,7 +121,7 @@ int main(int argc, char** argv) { ps->setThreadPoolMaxThreadCount(0); ps->setCallRestriction(ProcessState::CallRestriction::FATAL_IF_NOT_ONEWAY); - sp<ServiceManager> manager = new ServiceManager(std::make_unique<Access>()); + sp<ServiceManager> manager = sp<ServiceManager>::make(std::make_unique<Access>()); if (!manager->addService("manager", manager, false /*allowIsolated*/, IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT).isOk()) { LOG(ERROR) << "Could not self register servicemanager"; } diff --git a/cmds/servicemanager/test_sm.cpp b/cmds/servicemanager/test_sm.cpp index fb9f9df856..5d5a75e174 100644 --- a/cmds/servicemanager/test_sm.cpp +++ b/cmds/servicemanager/test_sm.cpp @@ -46,7 +46,7 @@ static sp<IBinder> getBinder() { } }; - return new LinkableBinder; + return sp<LinkableBinder>::make(); } class MockAccess : public Access { @@ -71,7 +71,7 @@ static sp<ServiceManager> getPermissiveServiceManager() { ON_CALL(*access, canFind(_, _)).WillByDefault(Return(true)); ON_CALL(*access, canList(_)).WillByDefault(Return(true)); - sp<ServiceManager> sm = new NiceMock<MockServiceManager>(std::move(access)); + sp<ServiceManager> sm = sp<NiceMock<MockServiceManager>>::make(std::move(access)); return sm; } @@ -119,7 +119,7 @@ TEST(AddService, AddDisallowedFromApp) { .uid = uid, })); EXPECT_CALL(*access, canAdd(_, _)).Times(0); - sp<ServiceManager> sm = new NiceMock<MockServiceManager>(std::move(access)); + sp<ServiceManager> sm = sp<NiceMock<MockServiceManager>>::make(std::move(access)); EXPECT_FALSE(sm->addService("foo", getBinder(), false /*allowIsolated*/, IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT).isOk()); @@ -161,7 +161,7 @@ TEST(AddService, NoPermissions) { EXPECT_CALL(*access, getCallingContext()).WillOnce(Return(Access::CallingContext{})); EXPECT_CALL(*access, canAdd(_, _)).WillOnce(Return(false)); - sp<ServiceManager> sm = new NiceMock<MockServiceManager>(std::move(access)); + sp<ServiceManager> sm = sp<NiceMock<MockServiceManager>>::make(std::move(access)); EXPECT_FALSE(sm->addService("foo", getBinder(), false /*allowIsolated*/, IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT).isOk()); @@ -194,7 +194,7 @@ TEST(GetService, NoPermissionsForGettingService) { EXPECT_CALL(*access, canAdd(_, _)).WillOnce(Return(true)); EXPECT_CALL(*access, canFind(_, _)).WillOnce(Return(false)); - sp<ServiceManager> sm = new NiceMock<MockServiceManager>(std::move(access)); + sp<ServiceManager> sm = sp<NiceMock<MockServiceManager>>::make(std::move(access)); EXPECT_TRUE(sm->addService("foo", getBinder(), false /*allowIsolated*/, IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT).isOk()); @@ -218,7 +218,7 @@ TEST(GetService, AllowedFromIsolated) { EXPECT_CALL(*access, canAdd(_, _)).WillOnce(Return(true)); EXPECT_CALL(*access, canFind(_, _)).WillOnce(Return(true)); - sp<ServiceManager> sm = new NiceMock<MockServiceManager>(std::move(access)); + sp<ServiceManager> sm = sp<NiceMock<MockServiceManager>>::make(std::move(access)); sp<IBinder> service = getBinder(); EXPECT_TRUE(sm->addService("foo", service, true /*allowIsolated*/, @@ -244,7 +244,7 @@ TEST(GetService, NotAllowedFromIsolated) { // TODO(b/136023468): when security check is first, this should be called first // EXPECT_CALL(*access, canFind(_, _)).WillOnce(Return(true)); - sp<ServiceManager> sm = new NiceMock<MockServiceManager>(std::move(access)); + sp<ServiceManager> sm = sp<NiceMock<MockServiceManager>>::make(std::move(access)); EXPECT_TRUE(sm->addService("foo", getBinder(), false /*allowIsolated*/, IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT).isOk()); @@ -261,7 +261,7 @@ TEST(ListServices, NoPermissions) { EXPECT_CALL(*access, getCallingContext()).WillOnce(Return(Access::CallingContext{})); EXPECT_CALL(*access, canList(_)).WillOnce(Return(false)); - sp<ServiceManager> sm = new NiceMock<MockServiceManager>(std::move(access)); + sp<ServiceManager> sm = sp<NiceMock<MockServiceManager>>::make(std::move(access)); std::vector<std::string> out; EXPECT_FALSE(sm->listServices(IServiceManager::DUMP_FLAG_PRIORITY_ALL, &out).isOk()); @@ -329,9 +329,9 @@ TEST(ServiceNotifications, NoPermissionsRegister) { EXPECT_CALL(*access, getCallingContext()).WillOnce(Return(Access::CallingContext{})); EXPECT_CALL(*access, canFind(_,_)).WillOnce(Return(false)); - sp<ServiceManager> sm = new ServiceManager(std::move(access)); + sp<ServiceManager> sm = sp<ServiceManager>::make(std::move(access)); - sp<CallbackHistorian> cb = new CallbackHistorian; + sp<CallbackHistorian> cb = sp<CallbackHistorian>::make(); EXPECT_EQ(sm->registerForNotifications("foofoo", cb).exceptionCode(), Status::EX_SECURITY); @@ -343,9 +343,9 @@ TEST(ServiceNotifications, NoPermissionsUnregister) { EXPECT_CALL(*access, getCallingContext()).WillOnce(Return(Access::CallingContext{})); EXPECT_CALL(*access, canFind(_,_)).WillOnce(Return(false)); - sp<ServiceManager> sm = new ServiceManager(std::move(access)); + sp<ServiceManager> sm = sp<ServiceManager>::make(std::move(access)); - sp<CallbackHistorian> cb = new CallbackHistorian; + sp<CallbackHistorian> cb = sp<CallbackHistorian>::make(); // should always hit security error first EXPECT_EQ(sm->unregisterForNotifications("foofoo", cb).exceptionCode(), @@ -355,7 +355,7 @@ TEST(ServiceNotifications, NoPermissionsUnregister) { TEST(ServiceNotifications, InvalidName) { auto sm = getPermissiveServiceManager(); - sp<CallbackHistorian> cb = new CallbackHistorian; + sp<CallbackHistorian> cb = sp<CallbackHistorian>::make(); EXPECT_EQ(sm->registerForNotifications("foo@foo", cb).exceptionCode(), Status::EX_ILLEGAL_ARGUMENT); @@ -371,7 +371,7 @@ TEST(ServiceNotifications, NullCallback) { TEST(ServiceNotifications, Unregister) { auto sm = getPermissiveServiceManager(); - sp<CallbackHistorian> cb = new CallbackHistorian; + sp<CallbackHistorian> cb = sp<CallbackHistorian>::make(); EXPECT_TRUE(sm->registerForNotifications("foofoo", cb).isOk()); EXPECT_EQ(sm->unregisterForNotifications("foofoo", cb).exceptionCode(), 0); @@ -380,7 +380,7 @@ TEST(ServiceNotifications, Unregister) { TEST(ServiceNotifications, UnregisterWhenNoRegistrationExists) { auto sm = getPermissiveServiceManager(); - sp<CallbackHistorian> cb = new CallbackHistorian; + sp<CallbackHistorian> cb = sp<CallbackHistorian>::make(); EXPECT_EQ(sm->unregisterForNotifications("foofoo", cb).exceptionCode(), Status::EX_ILLEGAL_STATE); @@ -389,7 +389,7 @@ TEST(ServiceNotifications, UnregisterWhenNoRegistrationExists) { TEST(ServiceNotifications, NoNotification) { auto sm = getPermissiveServiceManager(); - sp<CallbackHistorian> cb = new CallbackHistorian; + sp<CallbackHistorian> cb = sp<CallbackHistorian>::make(); EXPECT_TRUE(sm->registerForNotifications("foofoo", cb).isOk()); EXPECT_TRUE(sm->addService("otherservice", getBinder(), @@ -402,7 +402,7 @@ TEST(ServiceNotifications, NoNotification) { TEST(ServiceNotifications, GetNotification) { auto sm = getPermissiveServiceManager(); - sp<CallbackHistorian> cb = new CallbackHistorian; + sp<CallbackHistorian> cb = sp<CallbackHistorian>::make(); sp<IBinder> service = getBinder(); @@ -417,7 +417,7 @@ TEST(ServiceNotifications, GetNotification) { TEST(ServiceNotifications, GetNotificationForAlreadyRegisteredService) { auto sm = getPermissiveServiceManager(); - sp<CallbackHistorian> cb = new CallbackHistorian; + sp<CallbackHistorian> cb = sp<CallbackHistorian>::make(); sp<IBinder> service = getBinder(); @@ -433,7 +433,7 @@ TEST(ServiceNotifications, GetNotificationForAlreadyRegisteredService) { TEST(ServiceNotifications, GetMultipleNotification) { auto sm = getPermissiveServiceManager(); - sp<CallbackHistorian> cb = new CallbackHistorian; + sp<CallbackHistorian> cb = sp<CallbackHistorian>::make(); sp<IBinder> binder1 = getBinder(); sp<IBinder> binder2 = getBinder(); diff --git a/libs/binder/include/binder/IInterface.h b/libs/binder/include/binder/IInterface.h index f4a21ddd86..ea917539b7 100644 --- a/libs/binder/include/binder/IInterface.h +++ b/libs/binder/include/binder/IInterface.h @@ -186,7 +186,7 @@ template<typename INTERFACE> inline sp<IInterface> BnInterface<INTERFACE>::queryLocalInterface( const String16& _descriptor) { - if (_descriptor == INTERFACE::descriptor) return this; + if (_descriptor == INTERFACE::descriptor) return sp<IInterface>::fromExisting(this); return nullptr; } |