diff options
-rw-r--r-- | libs/binder/Binder.cpp | 4 | ||||
-rw-r--r-- | libs/binder/BpBinder.cpp | 26 | ||||
-rw-r--r-- | libs/binder/IMemory.cpp | 43 | ||||
-rw-r--r-- | libs/binder/IPCThreadState.cpp | 5 | ||||
-rw-r--r-- | libs/binder/RpcSession.cpp | 9 | ||||
-rw-r--r-- | libs/binder/include/binder/BpBinder.h | 4 |
6 files changed, 51 insertions, 40 deletions
diff --git a/libs/binder/Binder.cpp b/libs/binder/Binder.cpp index 0970ca5aa5..01b25d390d 100644 --- a/libs/binder/Binder.cpp +++ b/libs/binder/Binder.cpp @@ -19,6 +19,7 @@ #include <atomic> #include <set> +#include <android-base/logging.h> #include <android-base/unique_fd.h> #include <binder/BpBinder.h> #include <binder/IInterface.h> @@ -281,9 +282,11 @@ status_t BBinder::transact( err = pingBinder(); break; case EXTENSION_TRANSACTION: + CHECK(reply != nullptr); err = reply->writeStrongBinder(getExtension()); break; case DEBUG_PID_TRANSACTION: + CHECK(reply != nullptr); err = reply->writeInt32(getDebugPid()); break; case SET_RPC_CLIENT_TRANSACTION: { @@ -590,6 +593,7 @@ status_t BBinder::onTransact( { switch (code) { case INTERFACE_TRANSACTION: + CHECK(reply != nullptr); reply->writeString16(getInterfaceDescriptor()); return NO_ERROR; diff --git a/libs/binder/BpBinder.cpp b/libs/binder/BpBinder.cpp index 056ef0ab74..921e57c7bf 100644 --- a/libs/binder/BpBinder.cpp +++ b/libs/binder/BpBinder.cpp @@ -72,29 +72,29 @@ void* BpBinder::ObjectManager::attach(const void* objectID, void* object, void* e.cleanupCookie = cleanupCookie; e.func = func; - if (ssize_t idx = mObjects.indexOfKey(objectID); idx >= 0) { + if (mObjects.find(objectID) != mObjects.end()) { ALOGI("Trying to attach object ID %p to binder ObjectManager %p with object %p, but object " "ID already in use", objectID, this, object); - return mObjects[idx].object; + return mObjects[objectID].object; } - mObjects.add(objectID, e); + mObjects.insert({objectID, e}); return nullptr; } void* BpBinder::ObjectManager::find(const void* objectID) const { - const ssize_t i = mObjects.indexOfKey(objectID); - if (i < 0) return nullptr; - return mObjects.valueAt(i).object; + auto i = mObjects.find(objectID); + if (i == mObjects.end()) return nullptr; + return i->second.object; } void* BpBinder::ObjectManager::detach(const void* objectID) { - ssize_t idx = mObjects.indexOfKey(objectID); - if (idx < 0) return nullptr; - void* value = mObjects[idx].object; - mObjects.removeItemsAt(idx, 1); + auto i = mObjects.find(objectID); + if (i == mObjects.end()) return nullptr; + void* value = i->second.object; + mObjects.erase(i); return value; } @@ -102,10 +102,10 @@ void BpBinder::ObjectManager::kill() { const size_t N = mObjects.size(); ALOGV("Killing %zu objects in manager %p", N, this); - for (size_t i=0; i<N; i++) { - const entry_t& e = mObjects.valueAt(i); + for (auto i : mObjects) { + const entry_t& e = i.second; if (e.func != nullptr) { - e.func(mObjects.keyAt(i), e.object, e.cleanupCookie); + e.func(i.first, e.object, e.cleanupCookie); } } diff --git a/libs/binder/IMemory.cpp b/libs/binder/IMemory.cpp index bd974b02b1..9c7ff97a48 100644 --- a/libs/binder/IMemory.cpp +++ b/libs/binder/IMemory.cpp @@ -31,9 +31,10 @@ #include <binder/Parcel.h> #include <log/log.h> -#include <utils/KeyedVector.h> #include <utils/threads.h> +#include <map> + #define VERBOSE 0 namespace android { @@ -63,7 +64,7 @@ private: void free_heap(const wp<IBinder>& binder); Mutex mHeapCacheLock; // Protects entire vector below. - KeyedVector< wp<IBinder>, heap_info_t > mHeapCache; + std::map<wp<IBinder>, heap_info_t> mHeapCache; // We do not use the copy-on-write capabilities of KeyedVector. // TODO: Reimplemement based on standard C++ container? }; @@ -434,9 +435,9 @@ void HeapCache::binderDied(const wp<IBinder>& binder) sp<IMemoryHeap> HeapCache::find_heap(const sp<IBinder>& binder) { Mutex::Autolock _l(mHeapCacheLock); - ssize_t i = mHeapCache.indexOfKey(binder); - if (i>=0) { - heap_info_t& info = mHeapCache.editValueAt(i); + auto i = mHeapCache.find(binder); + if (i != mHeapCache.end()) { + heap_info_t& info = i->second; ALOGD_IF(VERBOSE, "found binder=%p, heap=%p, size=%zu, fd=%d, count=%d", binder.get(), info.heap.get(), @@ -452,7 +453,7 @@ sp<IMemoryHeap> HeapCache::find_heap(const sp<IBinder>& binder) info.count = 1; //ALOGD("adding binder=%p, heap=%p, count=%d", // binder.get(), info.heap.get(), info.count); - mHeapCache.add(binder, info); + mHeapCache.insert({binder, info}); return info.heap; } } @@ -466,9 +467,9 @@ void HeapCache::free_heap(const wp<IBinder>& binder) sp<IMemoryHeap> rel; { Mutex::Autolock _l(mHeapCacheLock); - ssize_t i = mHeapCache.indexOfKey(binder); - if (i>=0) { - heap_info_t& info(mHeapCache.editValueAt(i)); + auto i = mHeapCache.find(binder); + if (i != mHeapCache.end()) { + heap_info_t& info = i->second; if (--info.count == 0) { ALOGD_IF(VERBOSE, "removing binder=%p, heap=%p, size=%zu, fd=%d, count=%d", @@ -477,8 +478,8 @@ void HeapCache::free_heap(const wp<IBinder>& binder) static_cast<BpMemoryHeap*>(info.heap.get()) ->mHeapId.load(memory_order_relaxed), info.count); - rel = mHeapCache.valueAt(i).heap; - mHeapCache.removeItemsAt(i); + rel = i->second.heap; + mHeapCache.erase(i); } } else { ALOGE("free_heap binder=%p not found!!!", binder.unsafe_get()); @@ -490,23 +491,23 @@ sp<IMemoryHeap> HeapCache::get_heap(const sp<IBinder>& binder) { sp<IMemoryHeap> realHeap; Mutex::Autolock _l(mHeapCacheLock); - ssize_t i = mHeapCache.indexOfKey(binder); - if (i>=0) realHeap = mHeapCache.valueAt(i).heap; - else realHeap = interface_cast<IMemoryHeap>(binder); + auto i = mHeapCache.find(binder); + if (i != mHeapCache.end()) + realHeap = i->second.heap; + else + realHeap = interface_cast<IMemoryHeap>(binder); return realHeap; } void HeapCache::dump_heaps() { Mutex::Autolock _l(mHeapCacheLock); - int c = mHeapCache.size(); - for (int i=0 ; i<c ; i++) { - const heap_info_t& info = mHeapCache.valueAt(i); + for (const auto& i : mHeapCache) { + const heap_info_t& info = i.second; BpMemoryHeap const* h(static_cast<BpMemoryHeap const *>(info.heap.get())); - ALOGD("hey=%p, heap=%p, count=%d, (fd=%d, base=%p, size=%zu)", - mHeapCache.keyAt(i).unsafe_get(), - info.heap.get(), info.count, - h->mHeapId.load(memory_order_relaxed), h->mBase, h->mSize); + ALOGD("hey=%p, heap=%p, count=%d, (fd=%d, base=%p, size=%zu)", i.first.unsafe_get(), + info.heap.get(), info.count, h->mHeapId.load(memory_order_relaxed), h->mBase, + h->mSize); } } diff --git a/libs/binder/IPCThreadState.cpp b/libs/binder/IPCThreadState.cpp index 13f0a4c4a5..f79075d260 100644 --- a/libs/binder/IPCThreadState.cpp +++ b/libs/binder/IPCThreadState.cpp @@ -1199,7 +1199,8 @@ status_t IPCThreadState::executeCommand(int32_t cmd) case BR_DECREFS: refs = (RefBase::weakref_type*)mIn.readPointer(); - obj = (BBinder*)mIn.readPointer(); + // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores) + obj = (BBinder*)mIn.readPointer(); // consume // NOTE: This assertion is not valid, because the object may no // longer exist (thus the (BBinder*)cast above resulting in a different // memory address). @@ -1409,7 +1410,7 @@ status_t IPCThreadState::getProcessFreezeInfo(pid_t pid, uint32_t *sync_received uint32_t *async_received) { int ret = 0; - binder_frozen_status_info info; + binder_frozen_status_info info = {}; info.pid = pid; #if defined(__ANDROID__) diff --git a/libs/binder/RpcSession.cpp b/libs/binder/RpcSession.cpp index b84395e7cb..e79cb86ffa 100644 --- a/libs/binder/RpcSession.cpp +++ b/libs/binder/RpcSession.cpp @@ -152,8 +152,13 @@ status_t RpcSession::setupInetClient(const char* addr, unsigned int port) { } status_t RpcSession::setupPreconnectedClient(unique_fd fd, std::function<unique_fd()>&& request) { - return setupClient([&](const std::vector<uint8_t>& sessionId, bool incoming) -> status_t { - // std::move'd from fd becomes -1 (!ok()) + // Why passing raw fd? When fd is passed as reference, Clang analyzer sees that the variable + // `fd` is a moved-from object. To work-around the issue, unwrap the raw fd from the outer `fd`, + // pass the raw fd by value to the lambda, and then finally wrap it in unique_fd inside the + // lambda. + return setupClient([&, raw = fd.release()](const std::vector<uint8_t>& sessionId, + bool incoming) -> status_t { + unique_fd fd(raw); if (!fd.ok()) { fd = request(); if (!fd.ok()) return BAD_VALUE; diff --git a/libs/binder/include/binder/BpBinder.h b/libs/binder/include/binder/BpBinder.h index c0454b6fbd..8deb2fe4a8 100644 --- a/libs/binder/include/binder/BpBinder.h +++ b/libs/binder/include/binder/BpBinder.h @@ -17,10 +17,10 @@ #pragma once #include <binder/IBinder.h> -#include <utils/KeyedVector.h> #include <utils/Mutex.h> #include <utils/threads.h> +#include <map> #include <unordered_map> #include <variant> @@ -110,7 +110,7 @@ public: IBinder::object_cleanup_func func; }; - KeyedVector<const void*, entry_t> mObjects; + std::map<const void*, entry_t> mObjects; }; class PrivateAccessor { |