libbinder: Remove flexible array from RpcWireReply
We are going to change the size of this struct depending on the protocol
version and that gets messy when there is a flexible array member.
We could remove it from RpcWireTransaction as well, but that is a bigger
change and there is no motivation yet (besides consistency).
This change also happens to optimize out one allocation when the reply
parcel is zero bytes.
Bug: 185909244
Test: TH
Change-Id: I18a5712ba80e7b311b945ef54977b66ffa43e1ca
diff --git a/libs/binder/RpcState.cpp b/libs/binder/RpcState.cpp
index 7ec8e07..f16a9ab 100644
--- a/libs/binder/RpcState.cpp
+++ b/libs/binder/RpcState.cpp
@@ -563,7 +563,7 @@
static void cleanup_reply_data(Parcel* p, const uint8_t* data, size_t dataSize,
const binder_size_t* objects, size_t objectsCount) {
(void)p;
- delete[] const_cast<uint8_t*>(data - offsetof(RpcWireReply, data));
+ delete[] const_cast<uint8_t*>(data);
(void)dataSize;
LOG_ALWAYS_FATAL_IF(objects != nullptr);
LOG_ALWAYS_FATAL_IF(objectsCount != 0, "%zu objects remaining", objectsCount);
@@ -585,25 +585,30 @@
return status;
}
- CommandData data(command.bodySize);
- if (!data.valid()) return NO_MEMORY;
-
- iovec iov{data.data(), command.bodySize};
- if (status_t status = rpcRec(connection, session, "reply body", &iov, 1); status != OK)
- return status;
-
if (command.bodySize < sizeof(RpcWireReply)) {
ALOGE("Expecting %zu but got %" PRId32 " bytes for RpcWireReply. Terminating!",
sizeof(RpcWireReply), command.bodySize);
(void)session->shutdownAndWait(false);
return BAD_VALUE;
}
- RpcWireReply* rpcReply = reinterpret_cast<RpcWireReply*>(data.data());
- if (rpcReply->status != OK) return rpcReply->status;
+ RpcWireReply rpcReply;
+ CommandData data(command.bodySize - sizeof(RpcWireReply));
+ if (!data.valid()) return NO_MEMORY;
+
+ iovec iovs[]{
+ {&rpcReply, sizeof(RpcWireReply)},
+ {data.data(), data.size()},
+ };
+ if (status_t status = rpcRec(connection, session, "reply body", iovs, arraysize(iovs));
+ status != OK)
+ return status;
+ if (rpcReply.status != OK) return rpcReply.status;
+
+ uint8_t* parcelData = data.data();
+ size_t parcelDataSize = data.size();
data.release();
- reply->rpcSetDataReference(session, rpcReply->data,
- command.bodySize - offsetof(RpcWireReply, data), cleanup_reply_data);
+ reply->rpcSetDataReference(session, parcelData, parcelDataSize, cleanup_reply_data);
return OK;
}
diff --git a/libs/binder/RpcWireFormat.h b/libs/binder/RpcWireFormat.h
index 171550e..d9b5ca0 100644
--- a/libs/binder/RpcWireFormat.h
+++ b/libs/binder/RpcWireFormat.h
@@ -139,7 +139,6 @@
struct RpcWireReply {
int32_t status; // transact return
- uint8_t data[];
};
static_assert(sizeof(RpcWireReply) == 4);
diff --git a/libs/binder/tests/binderAllocationLimits.cpp b/libs/binder/tests/binderAllocationLimits.cpp
index dd1a8c3..2c34766 100644
--- a/libs/binder/tests/binderAllocationLimits.cpp
+++ b/libs/binder/tests/binderAllocationLimits.cpp
@@ -203,13 +203,15 @@
auto remoteBinder = session->getRootObject();
size_t mallocs = 0, totalBytes = 0;
- const auto on_malloc = OnMalloc([&](size_t bytes) {
- mallocs++;
- totalBytes += bytes;
- });
- CHECK_EQ(OK, remoteBinder->pingBinder());
- EXPECT_EQ(mallocs, 3);
- EXPECT_EQ(totalBytes, 60);
+ {
+ const auto on_malloc = OnMalloc([&](size_t bytes) {
+ mallocs++;
+ totalBytes += bytes;
+ });
+ CHECK_EQ(OK, remoteBinder->pingBinder());
+ }
+ EXPECT_EQ(mallocs, 2);
+ EXPECT_EQ(totalBytes, 56);
}
int main(int argc, char** argv) {