Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 68aa3bc6 authored by Frederick Mayle's avatar Frederick Mayle
Browse files

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
parent 7514e3b9

libs/binder/RpcState.cpp

+17 −12
Original line number Diff line number Diff line
@@ -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;

}

libs/binder/RpcWireFormat.h

+0 −1
Original line number Diff line number Diff line
@@ -139,7 +139,6 @@ static_assert(sizeof(RpcWireTransaction) == 40); 


struct RpcWireReply {

    int32_t status; // transact return

    uint8_t data[];

};

static_assert(sizeof(RpcWireReply) == 4);

libs/binder/tests/binderAllocationLimits.cpp

+9 −7
Original line number Diff line number Diff line
@@ -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);

    }

    EXPECT_EQ(mallocs, 2);

    EXPECT_EQ(totalBytes, 56);

}



int main(int argc, char** argv) {