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

3 files changed, 26 insertions, 20 deletions

diff --git a/libs/binder/RpcState.cpp b/libs/binder/RpcState.cpp
index 7ec8e0738d..f16a9ab98f 100644
--- a/libs/binder/RpcState.cpp
+++ b/libs/binder/RpcState.cpp
@@ -563,7 +563,7 @@ status_t RpcState::transactAddress(const sp<RpcSession::RpcConnection>& connecti
 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 @@ status_t RpcState::waitForReply(const sp<RpcSession::RpcConnection>& connection,
             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 171550e620..d9b5ca0dc1 100644
--- a/libs/binder/RpcWireFormat.h
+++ b/libs/binder/RpcWireFormat.h
@@ -139,7 +139,6 @@ static_assert(sizeof(RpcWireTransaction) == 40);
 
 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 dd1a8c341d..2c34766faa 100644
--- a/libs/binder/tests/binderAllocationLimits.cpp
+++ b/libs/binder/tests/binderAllocationLimits.cpp
@@ -203,13 +203,15 @@ TEST(RpcBinderAllocation, SetupRpcServer) {
     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) {