diff options
-rw-r--r-- | libs/binder/IServiceManager.cpp | 3 | ||||
-rw-r--r-- | libs/binder/Parcel.cpp | 2 | ||||
-rw-r--r-- | libs/binder/RecordedTransaction.cpp | 6 | ||||
-rw-r--r-- | libs/binder/Stability.cpp | 2 | ||||
-rw-r--r-- | libs/binder/include/binder/Parcel.h | 2 | ||||
-rw-r--r-- | libs/binder/ndk/include_cpp/android/binder_auto_utils.h | 2 | ||||
-rw-r--r-- | libs/binder/tests/Android.bp | 5 | ||||
-rw-r--r-- | libs/binder/tests/RpcTlsUtilsTest.cpp | 22 | ||||
-rw-r--r-- | libs/binder/tests/binderAllocationLimits.cpp | 18 | ||||
-rw-r--r-- | libs/binder/tests/binderClearBufTest.cpp | 2 | ||||
-rw-r--r-- | libs/binder/tests/binderLibTest.cpp | 16 | ||||
-rw-r--r-- | libs/binder/tests/binderRpcBenchmark.cpp | 2 | ||||
-rw-r--r-- | libs/binder/tests/binderRpcTest.cpp | 38 | ||||
-rw-r--r-- | libs/binder/tests/binderRpcTestCommon.h | 4 | ||||
-rw-r--r-- | libs/binder/tests/binderRpcTestTrusty.cpp | 6 |
15 files changed, 68 insertions, 62 deletions
diff --git a/libs/binder/IServiceManager.cpp b/libs/binder/IServiceManager.cpp index 39573ec54d..fbcf823045 100644 --- a/libs/binder/IServiceManager.cpp +++ b/libs/binder/IServiceManager.cpp @@ -20,6 +20,7 @@ #include <inttypes.h> #include <unistd.h> +#include <condition_variable> #include <android-base/properties.h> #include <android/os/BnServiceCallback.h> @@ -642,7 +643,7 @@ public: protected: // Override realGetService for ServiceManagerShim::waitForService. - Status realGetService(const std::string& name, sp<IBinder>* _aidl_return) { + Status realGetService(const std::string& name, sp<IBinder>* _aidl_return) override { *_aidl_return = getDeviceService({"-g", name}, mOptions); return Status::ok(); } diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp index 84ef489a6c..77b32753de 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -2768,7 +2768,7 @@ void Parcel::ipcSetDataReference(const uint8_t* data, size_t dataSize, const bin } if (type == BINDER_TYPE_FD) { // FDs from the kernel are always owned - FdTag(flat->handle, 0, this); + FdTag(flat->handle, nullptr, this); } minOffset = offset + sizeof(flat_binder_object); } diff --git a/libs/binder/RecordedTransaction.cpp b/libs/binder/RecordedTransaction.cpp index de2a69f7cf..924537e63a 100644 --- a/libs/binder/RecordedTransaction.cpp +++ b/libs/binder/RecordedTransaction.cpp @@ -230,8 +230,8 @@ std::optional<RecordedTransaction> RecordedTransaction::fromFile(const unique_fd } size_t memoryMappedSize = chunkPayloadSize + mmapPayloadStartOffset; - void* mappedMemory = - mmap(NULL, memoryMappedSize, PROT_READ, MAP_SHARED, fd.get(), mmapPageAlignedStart); + void* mappedMemory = mmap(nullptr, memoryMappedSize, PROT_READ, MAP_SHARED, fd.get(), + mmapPageAlignedStart); auto mmap_guard = make_scope_guard( [mappedMemory, memoryMappedSize] { munmap(mappedMemory, memoryMappedSize); }); @@ -382,7 +382,7 @@ android::status_t RecordedTransaction::dumpToFile(const unique_fd& fd) const { return UNKNOWN_ERROR; } - if (NO_ERROR != writeChunk(fd, END_CHUNK, 0, NULL)) { + if (NO_ERROR != writeChunk(fd, END_CHUNK, 0, nullptr)) { ALOGE("Failed to write end chunk to fd %d", fd.get()); return UNKNOWN_ERROR; } diff --git a/libs/binder/Stability.cpp b/libs/binder/Stability.cpp index 665dfea456..4fb8fa08bc 100644 --- a/libs/binder/Stability.cpp +++ b/libs/binder/Stability.cpp @@ -70,7 +70,7 @@ bool Stability::requiresVintfDeclaration(const sp<IBinder>& binder) { } void Stability::tryMarkCompilationUnit(IBinder* binder) { - (void)setRepr(binder, getLocalLevel(), REPR_NONE); + std::ignore = setRepr(binder, getLocalLevel(), REPR_NONE); } // after deprecation of the VNDK, these should be aliases. At some point diff --git a/libs/binder/include/binder/Parcel.h b/libs/binder/include/binder/Parcel.h index 5e18b9197d..eb730374d9 100644 --- a/libs/binder/include/binder/Parcel.h +++ b/libs/binder/include/binder/Parcel.h @@ -1009,7 +1009,7 @@ private: typename std::enable_if_t<is_specialization_v<CT, std::vector>, bool> = true> status_t writeData(const CT& c) { using T = first_template_type_t<CT>; // The T in CT == C<T, ...> - if (c.size() > std::numeric_limits<int32_t>::max()) return BAD_VALUE; + if (c.size() > static_cast<size_t>(std::numeric_limits<int32_t>::max())) return BAD_VALUE; const auto size = static_cast<int32_t>(c.size()); writeData(size); if constexpr (is_pointer_equivalent_array_v<T>) { diff --git a/libs/binder/ndk/include_cpp/android/binder_auto_utils.h b/libs/binder/ndk/include_cpp/android/binder_auto_utils.h index 18769b1454..8c62924beb 100644 --- a/libs/binder/ndk/include_cpp/android/binder_auto_utils.h +++ b/libs/binder/ndk/include_cpp/android/binder_auto_utils.h @@ -377,7 +377,7 @@ class ScopedAIBinder_Weak namespace internal { -static void closeWithError(int fd) { +inline void closeWithError(int fd) { if (fd == -1) return; int ret = close(fd); if (ret != 0) { diff --git a/libs/binder/tests/Android.bp b/libs/binder/tests/Android.bp index 6800a8d36c..bd24a20370 100644 --- a/libs/binder/tests/Android.bp +++ b/libs/binder/tests/Android.bp @@ -28,6 +28,11 @@ cc_defaults { cflags: [ "-Wall", "-Werror", + "-Wformat", + "-Wpessimizing-move", + "-Wsign-compare", + "-Wunused-result", + "-Wzero-as-null-pointer-constant", ], } diff --git a/libs/binder/tests/RpcTlsUtilsTest.cpp b/libs/binder/tests/RpcTlsUtilsTest.cpp index 530606c44a..48e3345ed1 100644 --- a/libs/binder/tests/RpcTlsUtilsTest.cpp +++ b/libs/binder/tests/RpcTlsUtilsTest.cpp @@ -52,9 +52,9 @@ TEST_P(RpcTlsUtilsKeyTest, Test) { << "\nactual: " << toDebugString(deserializedPkey.get()); } -INSTANTIATE_TEST_CASE_P(RpcTlsUtilsTest, RpcTlsUtilsKeyTest, - testing::Values(RpcKeyFormat::PEM, RpcKeyFormat::DER), - RpcTlsUtilsKeyTest::PrintParamInfo); +INSTANTIATE_TEST_SUITE_P(RpcTlsUtilsTest, RpcTlsUtilsKeyTest, + testing::Values(RpcKeyFormat::PEM, RpcKeyFormat::DER), + RpcTlsUtilsKeyTest::PrintParamInfo); class RpcTlsUtilsCertTest : public testing::TestWithParam<RpcCertificateFormat> { public: @@ -75,9 +75,9 @@ TEST_P(RpcTlsUtilsCertTest, Test) { EXPECT_EQ(0, X509_cmp(cert.get(), deserializedCert.get())); } -INSTANTIATE_TEST_CASE_P(RpcTlsUtilsTest, RpcTlsUtilsCertTest, - testing::Values(RpcCertificateFormat::PEM, RpcCertificateFormat::DER), - RpcTlsUtilsCertTest::PrintParamInfo); +INSTANTIATE_TEST_SUITE_P(RpcTlsUtilsTest, RpcTlsUtilsCertTest, + testing::Values(RpcCertificateFormat::PEM, RpcCertificateFormat::DER), + RpcTlsUtilsCertTest::PrintParamInfo); class RpcTlsUtilsKeyAndCertTest : public testing::TestWithParam<std::tuple<RpcKeyFormat, RpcCertificateFormat>> { @@ -105,10 +105,10 @@ TEST_P(RpcTlsUtilsKeyAndCertTest, TestCertFromDeserializedKey) { EXPECT_EQ(0, X509_cmp(cert.get(), deserializedCert.get())); } -INSTANTIATE_TEST_CASE_P(RpcTlsUtilsTest, RpcTlsUtilsKeyAndCertTest, - testing::Combine(testing::Values(RpcKeyFormat::PEM, RpcKeyFormat::DER), - testing::Values(RpcCertificateFormat::PEM, - RpcCertificateFormat::DER)), - RpcTlsUtilsKeyAndCertTest::PrintParamInfo); +INSTANTIATE_TEST_SUITE_P(RpcTlsUtilsTest, RpcTlsUtilsKeyAndCertTest, + testing::Combine(testing::Values(RpcKeyFormat::PEM, RpcKeyFormat::DER), + testing::Values(RpcCertificateFormat::PEM, + RpcCertificateFormat::DER)), + RpcTlsUtilsKeyAndCertTest::PrintParamInfo); } // namespace android diff --git a/libs/binder/tests/binderAllocationLimits.cpp b/libs/binder/tests/binderAllocationLimits.cpp index 7e0b59463a..c0c0aae80b 100644 --- a/libs/binder/tests/binderAllocationLimits.cpp +++ b/libs/binder/tests/binderAllocationLimits.cpp @@ -114,12 +114,12 @@ TEST(TestTheTest, OnMalloc) { { const auto on_malloc = OnMalloc([&](size_t bytes) { mallocs++; - EXPECT_EQ(bytes, 40); + EXPECT_EQ(bytes, 40u); }); imaginary_use = new int[10]; } - EXPECT_EQ(mallocs, 1); + EXPECT_EQ(mallocs, 1u); } @@ -196,9 +196,9 @@ TEST(BinderAllocation, InterfaceDescriptorTransaction) { // Happens to be SM package length. We could switch to forking // and registering our own service if it became an issue. #if defined(__LP64__) - EXPECT_EQ(bytes, 78); + EXPECT_EQ(bytes, 78u); #else - EXPECT_EQ(bytes, 70); + EXPECT_EQ(bytes, 70u); #endif }); @@ -206,7 +206,7 @@ TEST(BinderAllocation, InterfaceDescriptorTransaction) { a_binder->getInterfaceDescriptor(); a_binder->getInterfaceDescriptor(); - EXPECT_EQ(mallocs, 1); + EXPECT_EQ(mallocs, 1u); } TEST(BinderAllocation, SmallTransaction) { @@ -217,11 +217,11 @@ TEST(BinderAllocation, SmallTransaction) { const auto on_malloc = OnMalloc([&](size_t bytes) { mallocs++; // Parcel should allocate a small amount by default - EXPECT_EQ(bytes, 128); + EXPECT_EQ(bytes, 128u); }); manager->checkService(empty_descriptor); - EXPECT_EQ(mallocs, 1); + EXPECT_EQ(mallocs, 1u); } TEST(RpcBinderAllocation, SetupRpcServer) { @@ -250,8 +250,8 @@ TEST(RpcBinderAllocation, SetupRpcServer) { }); ASSERT_EQ(OK, remoteBinder->pingBinder()); } - EXPECT_EQ(mallocs, 1); - EXPECT_EQ(totalBytes, 40); + EXPECT_EQ(mallocs, 1u); + EXPECT_EQ(totalBytes, 40u); } int main(int argc, char** argv) { diff --git a/libs/binder/tests/binderClearBufTest.cpp b/libs/binder/tests/binderClearBufTest.cpp index e43ee5fcf5..3230a3f904 100644 --- a/libs/binder/tests/binderClearBufTest.cpp +++ b/libs/binder/tests/binderClearBufTest.cpp @@ -88,7 +88,7 @@ TEST(BinderClearBuf, ClearKernelBuffer) { // the buffer must have at least some length for the string, but we will // just check it has some length, to avoid assuming anything about the // format - EXPECT_GT(replyBuffer.size(), 0); + EXPECT_GT(replyBuffer.size(), 0u); for (size_t i = 0; i < replyBuffer.size(); i++) { EXPECT_EQ(replyBuffer[i], '0') << "reply buffer at " << i; diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp index 1f61f1852a..9788d9d1dd 100644 --- a/libs/binder/tests/binderLibTest.cpp +++ b/libs/binder/tests/binderLibTest.cpp @@ -528,8 +528,8 @@ TEST_F(BinderLibTest, Freeze) { EXPECT_EQ(NO_ERROR, IPCThreadState::self()->getProcessFreezeInfo(pid, &sync_received, &async_received)); - EXPECT_EQ(sync_received, 1); - EXPECT_EQ(async_received, 0); + EXPECT_EQ(sync_received, 1u); + EXPECT_EQ(async_received, 0u); EXPECT_EQ(NO_ERROR, IPCThreadState::self()->freeze(pid, false, 0)); EXPECT_EQ(NO_ERROR, m_server->transact(BINDER_LIB_TEST_NOP_TRANSACTION, data, &reply)); @@ -1238,13 +1238,13 @@ TEST_F(BinderLibTest, BufRejected) { data.setDataCapacity(1024); // Write a bogus object at offset 0 to get an entry in the offset table data.writeFileDescriptor(0); - EXPECT_EQ(data.objectsCount(), 1); + EXPECT_EQ(data.objectsCount(), 1u); uint8_t *parcelData = const_cast<uint8_t*>(data.data()); // And now, overwrite it with the buffer object memcpy(parcelData, &obj, sizeof(obj)); data.setDataSize(sizeof(obj)); - EXPECT_EQ(data.objectsCount(), 1); + EXPECT_EQ(data.objectsCount(), 1u); // Either the kernel should reject this transaction (if it's correct), but // if it's not, the server implementation should return an error if it @@ -1269,7 +1269,7 @@ TEST_F(BinderLibTest, WeakRejected) { data.setDataCapacity(1024); // Write a bogus object at offset 0 to get an entry in the offset table data.writeFileDescriptor(0); - EXPECT_EQ(data.objectsCount(), 1); + EXPECT_EQ(data.objectsCount(), 1u); uint8_t *parcelData = const_cast<uint8_t *>(data.data()); // And now, overwrite it with the weak binder memcpy(parcelData, &obj, sizeof(obj)); @@ -1279,7 +1279,7 @@ TEST_F(BinderLibTest, WeakRejected) { // test with an object that libbinder will actually try to release EXPECT_EQ(OK, data.writeStrongBinder(sp<BBinder>::make())); - EXPECT_EQ(data.objectsCount(), 2); + EXPECT_EQ(data.objectsCount(), 2u); // send it many times, since previous error was memory corruption, make it // more likely that the server crashes @@ -1648,8 +1648,8 @@ TEST_P(BinderLibRpcTestP, SetRpcClientDebugNoKeepAliveBinder) { EXPECT_THAT(binder->setRpcClientDebug(std::move(socket), nullptr), Debuggable(StatusEq(UNEXPECTED_NULL))); } -INSTANTIATE_TEST_CASE_P(BinderLibTest, BinderLibRpcTestP, testing::Bool(), - BinderLibRpcTestP::ParamToString); +INSTANTIATE_TEST_SUITE_P(BinderLibTest, BinderLibRpcTestP, testing::Bool(), + BinderLibRpcTestP::ParamToString); class BinderLibTestService : public BBinder { public: diff --git a/libs/binder/tests/binderRpcBenchmark.cpp b/libs/binder/tests/binderRpcBenchmark.cpp index 4f10d7417f..865f0ec9a4 100644 --- a/libs/binder/tests/binderRpcBenchmark.cpp +++ b/libs/binder/tests/binderRpcBenchmark.cpp @@ -216,7 +216,7 @@ void BM_repeatTwoPageString(benchmark::State& state) { // by how many binder calls work together (and by factors like the scheduler, // thermal throttling, core choice, etc..). std::string str = std::string(getpagesize() * 2, 'a'); - CHECK_EQ(str.size(), getpagesize() * 2); + CHECK_EQ(static_cast<ssize_t>(str.size()), getpagesize() * 2); while (state.KeepRunning()) { std::string out; diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp index 2769a88342..f0400d3c0d 100644 --- a/libs/binder/tests/binderRpcTest.cpp +++ b/libs/binder/tests/binderRpcTest.cpp @@ -177,7 +177,7 @@ public: EXPECT_NE(nullptr, session); EXPECT_NE(nullptr, session->state()); - EXPECT_EQ(0, session->state()->countBinders()) << (session->state()->dump(), "dump:"); + EXPECT_EQ(0u, session->state()->countBinders()) << (session->state()->dump(), "dump:"); wp<RpcSession> weakSession = session; session = nullptr; @@ -235,7 +235,7 @@ static unique_fd connectToUnixBootstrap(const RpcTransportFd& transportFd) { if (binder::os::sendMessageOnSocket(transportFd, &iov, 1, &fds) < 0) { PLOGF("Failed sendMessageOnSocket"); } - return std::move(sockClient); + return sockClient; } #endif // BINDER_RPC_TO_TRUSTY_TEST @@ -623,7 +623,7 @@ TEST_P(BinderRpc, OnewayCallQueueing) { for (size_t i = 0; i + 1 < kNumQueued; i++) { int n; proc.rootIface->blockingRecvInt(&n); - EXPECT_EQ(n, i); + EXPECT_EQ(n, static_cast<ssize_t>(i)); } saturateThreadPool(1 + kNumExtraServerThreads, proc.rootIface); @@ -1148,8 +1148,8 @@ static std::vector<BinderRpc::ParamType> getTrustyBinderRpcParams() { return ret; } -INSTANTIATE_TEST_CASE_P(Trusty, BinderRpc, ::testing::ValuesIn(getTrustyBinderRpcParams()), - BinderRpc::PrintParamInfo); +INSTANTIATE_TEST_SUITE_P(Trusty, BinderRpc, ::testing::ValuesIn(getTrustyBinderRpcParams()), + BinderRpc::PrintParamInfo); #else // BINDER_RPC_TO_TRUSTY_TEST bool testSupportVsockLoopback() { // We don't need to enable TLS to know if vsock is supported. @@ -1308,8 +1308,8 @@ static std::vector<BinderRpc::ParamType> getBinderRpcParams() { return ret; } -INSTANTIATE_TEST_CASE_P(PerSocket, BinderRpc, ::testing::ValuesIn(getBinderRpcParams()), - BinderRpc::PrintParamInfo); +INSTANTIATE_TEST_SUITE_P(PerSocket, BinderRpc, ::testing::ValuesIn(getBinderRpcParams()), + BinderRpc::PrintParamInfo); class BinderRpcServerRootObject : public ::testing::TestWithParam<std::tuple<bool, bool, RpcSecurity>> {}; @@ -1337,9 +1337,9 @@ TEST_P(BinderRpcServerRootObject, WeakRootObject) { EXPECT_EQ((isStrong2 ? binderRaw2 : nullptr), server->getRootObject()); } -INSTANTIATE_TEST_CASE_P(BinderRpc, BinderRpcServerRootObject, - ::testing::Combine(::testing::Bool(), ::testing::Bool(), - ::testing::ValuesIn(RpcSecurityValues()))); +INSTANTIATE_TEST_SUITE_P(BinderRpc, BinderRpcServerRootObject, + ::testing::Combine(::testing::Bool(), ::testing::Bool(), + ::testing::ValuesIn(RpcSecurityValues()))); class OneOffSignal { public: @@ -1384,7 +1384,7 @@ TEST(BinderRpc, Java) { auto binder = sm->checkService(String16("batteryproperties")); ASSERT_NE(nullptr, binder); auto descriptor = binder->getInterfaceDescriptor(); - ASSERT_GE(descriptor.size(), 0); + ASSERT_GE(descriptor.size(), 0u); ASSERT_EQ(OK, binder->pingBinder()); auto rpcServer = RpcServer::make(); @@ -1468,10 +1468,10 @@ TEST_P(BinderRpcServerOnly, Shutdown) { << "After server->shutdown() returns true, join() did not stop after 2s"; } -INSTANTIATE_TEST_CASE_P(BinderRpc, BinderRpcServerOnly, - ::testing::Combine(::testing::ValuesIn(RpcSecurityValues()), - ::testing::ValuesIn(testVersions())), - BinderRpcServerOnly::PrintTestParam); +INSTANTIATE_TEST_SUITE_P(BinderRpc, BinderRpcServerOnly, + ::testing::Combine(::testing::ValuesIn(RpcSecurityValues()), + ::testing::ValuesIn(testVersions())), + BinderRpcServerOnly::PrintTestParam); class RpcTransportTestUtils { public: @@ -2018,9 +2018,9 @@ TEST_P(RpcTransportTest, CheckWaitingForRead) { server->shutdown(); } -INSTANTIATE_TEST_CASE_P(BinderRpc, RpcTransportTest, - ::testing::ValuesIn(RpcTransportTest::getRpcTranportTestParams()), - RpcTransportTest::PrintParamInfo); +INSTANTIATE_TEST_SUITE_P(BinderRpc, RpcTransportTest, + ::testing::ValuesIn(RpcTransportTest::getRpcTranportTestParams()), + RpcTransportTest::PrintParamInfo); class RpcTransportTlsKeyTest : public testing::TestWithParam< @@ -2075,7 +2075,7 @@ TEST_P(RpcTransportTlsKeyTest, PreSignedCertificate) { client.run(); } -INSTANTIATE_TEST_CASE_P( +INSTANTIATE_TEST_SUITE_P( BinderRpc, RpcTransportTlsKeyTest, testing::Combine(testing::ValuesIn(testSocketTypes(false /* hasPreconnected*/)), testing::Values(RpcCertificateFormat::PEM, RpcCertificateFormat::DER), diff --git a/libs/binder/tests/binderRpcTestCommon.h b/libs/binder/tests/binderRpcTestCommon.h index 8832f1a6ba..dfce4417a0 100644 --- a/libs/binder/tests/binderRpcTestCommon.h +++ b/libs/binder/tests/binderRpcTestCommon.h @@ -210,7 +210,7 @@ static inline std::unique_ptr<RpcTransportCtxFactory> newTlsFactory( return RpcTransportCtxFactoryTls::make(std::move(verifier), std::move(auth)); } default: - LOG_ALWAYS_FATAL("Unknown RpcSecurity %d", rpcSecurity); + LOG_ALWAYS_FATAL("Unknown RpcSecurity %d", static_cast<int>(rpcSecurity)); } } @@ -256,7 +256,7 @@ public: mValue.reset(); lock.unlock(); mCvEmpty.notify_all(); - return std::move(v); + return v; } private: diff --git a/libs/binder/tests/binderRpcTestTrusty.cpp b/libs/binder/tests/binderRpcTestTrusty.cpp index 18751cc089..31c0eba1c1 100644 --- a/libs/binder/tests/binderRpcTestTrusty.cpp +++ b/libs/binder/tests/binderRpcTestTrusty.cpp @@ -45,7 +45,7 @@ std::unique_ptr<RpcTransportCtxFactory> BinderRpc::newFactory(RpcSecurity rpcSec case RpcSecurity::RAW: return RpcTransportCtxFactoryTipcTrusty::make(); default: - LOG_ALWAYS_FATAL("Unknown RpcSecurity %d", rpcSecurity); + LOG_ALWAYS_FATAL("Unknown RpcSecurity %d", static_cast<int>(rpcSecurity)); } } @@ -110,8 +110,8 @@ static std::vector<BinderRpc::ParamType> getTrustyBinderRpcParams() { return ret; } -INSTANTIATE_TEST_CASE_P(Trusty, BinderRpc, ::testing::ValuesIn(getTrustyBinderRpcParams()), - BinderRpc::PrintParamInfo); +INSTANTIATE_TEST_SUITE_P(Trusty, BinderRpc, ::testing::ValuesIn(getTrustyBinderRpcParams()), + BinderRpc::PrintParamInfo); } // namespace android |