libbinder: RPC use 'status_t' over 'bool'
For ease of debugging/handling errors (I realized that binderRpcTest
currently crashes on Pixel 3 because of some missing kernel patches -
this would have been easier with this, and one potential (even if
temporary) solution would be to check the return code).
Bug: 167966510
Test: binderRpcTest, binderRpcBenchmark, binder_parcel_fuzzer,
binder_rpc_fuzzer, binderHostDeviceTest
Change-Id: I1baa2e9380e0ec8f82f8ceb250f3eeb632dc5fbc
diff --git a/libs/binder/tests/binderHostDeviceTest.cpp b/libs/binder/tests/binderHostDeviceTest.cpp
index 5dd9212..3f72b8f 100644
--- a/libs/binder/tests/binderHostDeviceTest.cpp
+++ b/libs/binder/tests/binderHostDeviceTest.cpp
@@ -101,8 +101,9 @@
[[nodiscard]] static sp<IBinder> get(unsigned int hostPort) {
auto rpcSession = RpcSession::make();
- if (!rpcSession->setupInetClient("127.0.0.1", hostPort)) {
- ADD_FAILURE() << "Failed to setupInetClient on " << hostPort;
+ if (status_t status = rpcSession->setupInetClient("127.0.0.1", hostPort); status != OK) {
+ ADD_FAILURE() << "Failed to setupInetClient on " << hostPort << ": "
+ << statusToString(status);
return nullptr;
}
return rpcSession->getRootObject();
diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp
index 65db7f6..eea7d8c 100644
--- a/libs/binder/tests/binderLibTest.cpp
+++ b/libs/binder/tests/binderLibTest.cpp
@@ -1192,8 +1192,8 @@
if (rpcServer == nullptr) return {};
rpcServer->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction();
unsigned int port;
- if (!rpcServer->setupInetServer("127.0.0.1", 0, &port)) {
- ADD_FAILURE() << "setupInetServer failed";
+ if (status_t status = rpcServer->setupInetServer("127.0.0.1", 0, &port); status != OK) {
+ ADD_FAILURE() << "setupInetServer failed" << statusToString(status);
return {};
}
return {rpcServer->releaseServer(), port};
diff --git a/libs/binder/tests/binderRpcBenchmark.cpp b/libs/binder/tests/binderRpcBenchmark.cpp
index 5f4a7b5..8d75a53 100644
--- a/libs/binder/tests/binderRpcBenchmark.cpp
+++ b/libs/binder/tests/binderRpcBenchmark.cpp
@@ -42,6 +42,8 @@
using android::RpcServer;
using android::RpcSession;
using android::sp;
+using android::status_t;
+using android::statusToString;
using android::String16;
using android::binder::Status;
@@ -160,7 +162,7 @@
sp<RpcServer> server = RpcServer::make();
server->setRootObject(sp<MyBinderRpcBenchmark>::make());
server->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction();
- CHECK(server->setupUnixDomainServer(addr.c_str()));
+ CHECK_EQ(OK, server->setupUnixDomainServer(addr.c_str()));
server->join();
exit(1);
}
@@ -182,11 +184,13 @@
CHECK_NE(nullptr, gKernelBinder.get());
#endif
+ status_t status;
for (size_t tries = 0; tries < 5; tries++) {
usleep(10000);
- if (gSession->setupUnixDomainClient(addr.c_str())) goto success;
+ status = gSession->setupUnixDomainClient(addr.c_str());
+ if (status == OK) goto success;
}
- LOG(FATAL) << "Could not connect.";
+ LOG(FATAL) << "Could not connect: " << statusToString(status).c_str();
success:
::benchmark::RunSpecifiedBenchmarks();
diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp
index 36a6804..6dd4019 100644
--- a/libs/binder/tests/binderRpcTest.cpp
+++ b/libs/binder/tests/binderRpcTest.cpp
@@ -89,7 +89,7 @@
auto server = RpcServer::make(newFactory(GetParam()));
server->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction();
ASSERT_FALSE(server->hasServer());
- ASSERT_TRUE(server->setupExternalServer(std::move(sink)));
+ ASSERT_EQ(OK, server->setupExternalServer(std::move(sink)));
ASSERT_TRUE(server->hasServer());
base::unique_fd retrieved = server->releaseServer();
ASSERT_FALSE(server->hasServer());
@@ -484,13 +484,13 @@
case SocketType::PRECONNECTED:
[[fallthrough]];
case SocketType::UNIX:
- CHECK(server->setupUnixDomainServer(addr.c_str())) << addr;
+ CHECK_EQ(OK, server->setupUnixDomainServer(addr.c_str())) << addr;
break;
case SocketType::VSOCK:
- CHECK(server->setupVsockServer(vsockPort));
+ CHECK_EQ(OK, server->setupVsockServer(vsockPort));
break;
case SocketType::INET: {
- CHECK(server->setupInetServer(kLocalInetAddress, 0, &outPort));
+ CHECK_EQ(OK, server->setupInetServer(kLocalInetAddress, 0, &outPort));
CHECK_NE(0, outPort);
break;
}
@@ -516,30 +516,35 @@
CHECK_NE(0, outPort);
}
+ status_t status;
+
for (size_t i = 0; i < options.numSessions; i++) {
sp<RpcSession> session = RpcSession::make(newFactory(rpcSecurity));
session->setMaxThreads(options.numIncomingConnections);
switch (socketType) {
case SocketType::PRECONNECTED:
- if (session->setupPreconnectedClient({}, [=]() {
- return connectToUds(addr.c_str());
- }))
- goto success;
+ status = session->setupPreconnectedClient({}, [=]() {
+ return connectToUds(addr.c_str());
+ });
+ if (status == OK) goto success;
break;
case SocketType::UNIX:
- if (session->setupUnixDomainClient(addr.c_str())) goto success;
+ status = session->setupUnixDomainClient(addr.c_str());
+ if (status == OK) goto success;
break;
case SocketType::VSOCK:
- if (session->setupVsockClient(VMADDR_CID_LOCAL, vsockPort)) goto success;
+ status = session->setupVsockClient(VMADDR_CID_LOCAL, vsockPort);
+ if (status == OK) goto success;
break;
case SocketType::INET:
- if (session->setupInetClient("127.0.0.1", outPort)) goto success;
+ status = session->setupInetClient("127.0.0.1", outPort);
+ if (status == OK) goto success;
break;
default:
LOG_ALWAYS_FATAL("Unknown socket type");
}
- LOG_ALWAYS_FATAL("Could not connect");
+ LOG_ALWAYS_FATAL("Could not connect %s", statusToString(status).c_str());
success:
ret.sessions.push_back({session, session->getRootObject()});
}
@@ -1191,14 +1196,14 @@
unsigned int vsockPort = allocateVsockPort();
sp<RpcServer> server = RpcServer::make(RpcTransportCtxFactoryRaw::make());
server->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction();
- CHECK(server->setupVsockServer(vsockPort));
+ CHECK_EQ(OK, server->setupVsockServer(vsockPort));
server->start();
sp<RpcSession> session = RpcSession::make(RpcTransportCtxFactoryRaw::make());
- bool okay = session->setupVsockClient(VMADDR_CID_LOCAL, vsockPort);
+ status_t status = session->setupVsockClient(VMADDR_CID_LOCAL, vsockPort);
while (!server->shutdown()) usleep(10000);
- ALOGE("Detected vsock loopback supported: %d", okay);
- return okay;
+ ALOGE("Detected vsock loopback supported: %s", statusToString(status).c_str());
+ return status == OK;
}
static std::vector<SocketType> testSocketTypes() {
@@ -1274,7 +1279,7 @@
unlink(addr.c_str());
auto server = RpcServer::make(newFactory(GetParam()));
server->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction();
- ASSERT_TRUE(server->setupUnixDomainServer(addr.c_str()));
+ ASSERT_EQ(OK, server->setupUnixDomainServer(addr.c_str()));
auto joinEnds = std::make_shared<OneOffSignal>();
// If things are broken and the thread never stops, don't block other tests. Because the thread
@@ -1315,14 +1320,14 @@
auto rpcServer = RpcServer::make();
rpcServer->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction();
unsigned int port;
- ASSERT_TRUE(rpcServer->setupInetServer(kLocalInetAddress, 0, &port));
+ ASSERT_EQ(OK, rpcServer->setupInetServer(kLocalInetAddress, 0, &port));
auto socket = rpcServer->releaseServer();
auto keepAlive = sp<BBinder>::make();
ASSERT_EQ(OK, binder->setRpcClientDebug(std::move(socket), keepAlive));
auto rpcSession = RpcSession::make();
- ASSERT_TRUE(rpcSession->setupInetClient("127.0.0.1", port));
+ ASSERT_EQ(OK, rpcSession->setupInetClient("127.0.0.1", port));
auto rpcBinder = rpcSession->getRootObject();
ASSERT_NE(nullptr, rpcBinder);
diff --git a/libs/binder/tests/parcel_fuzzer/random_parcel.cpp b/libs/binder/tests/parcel_fuzzer/random_parcel.cpp
index a2472f8..8bf04cc 100644
--- a/libs/binder/tests/parcel_fuzzer/random_parcel.cpp
+++ b/libs/binder/tests/parcel_fuzzer/random_parcel.cpp
@@ -37,7 +37,7 @@
void fillRandomParcel(Parcel* p, FuzzedDataProvider&& provider) {
if (provider.ConsumeBool()) {
auto session = RpcSession::make(RpcTransportCtxFactoryRaw::make());
- CHECK(session->addNullDebuggingClient());
+ CHECK_EQ(OK, session->addNullDebuggingClient());
p->markForRpc(session);
fillRandomParcelData(p, std::move(provider));
return;
diff --git a/libs/binder/tests/rpc_fuzzer/main.cpp b/libs/binder/tests/rpc_fuzzer/main.cpp
index 9fc496f..230f5c7 100644
--- a/libs/binder/tests/rpc_fuzzer/main.cpp
+++ b/libs/binder/tests/rpc_fuzzer/main.cpp
@@ -60,7 +60,7 @@
sp<RpcServer> server = RpcServer::make();
server->setRootObject(sp<SomeBinder>::make());
server->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction();
- CHECK(server->setupUnixDomainServer(kSock.c_str()));
+ CHECK_EQ(OK, server->setupUnixDomainServer(kSock.c_str()));
std::thread serverThread([=] { (void)server->join(); });