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

Commit 0c25e864 authored by Paul Ramirez's avatar Paul Ramirez
Browse files

Return message wrapped in Result from receiveMessage 

Changed receivedMessaged return type from status_t to
android::base::Result<>. Enforces checking valid state
before using the object.

Flag: EXEMPT refactor
Bug: 297226446
Test: TEST=libinput_tests; m $TEST && $ANDROID_HOST_OUT/nativetest64/$TEST/$TEST
Change-Id: Ic2285d38a2d0d2227c1fae92379270a5f52586c7
parent 9c879e83

include/input/InputTransport.h

+1 −1
Original line number Diff line number Diff line
@@ -275,7 +275,7 @@ public: 
     * Return DEAD_OBJECT if the channel's peer has been closed.

     * Other errors probably indicate that the channel is broken.

     */

    status_t receiveMessage(InputMessage* msg);

    android::base::Result<InputMessage> receiveMessage();



    /* Tells whether there is a message in the channel available to be received.

     *

libs/input/InputConsumer.cpp

+8 −7
Original line number Diff line number Diff line
@@ -235,8 +235,9 @@ status_t InputConsumer::consume(InputEventFactoryInterface* factory, bool consum 
            mMsgDeferred = false;

        } else {

            // Receive a fresh message.

            status_t result = mChannel->receiveMessage(&mMsg);

            if (result == OK) {

            android::base::Result<InputMessage> result = mChannel->receiveMessage();

            if (result.ok()) {

                mMsg = std::move(result.value());

                const auto [_, inserted] =

                        mConsumeTimes.emplace(mMsg.header.seq, systemTime(SYSTEM_TIME_MONOTONIC));

                LOG_ALWAYS_FATAL_IF(!inserted, "Already have a consume time for seq=%" PRIu32,
@@ -244,11 +245,11 @@ status_t InputConsumer::consume(InputEventFactoryInterface* factory, bool consum 


                // Trace the event processing timeline - event was just read from the socket

                ATRACE_ASYNC_BEGIN(mProcessingTraceTag.c_str(), /*cookie=*/mMsg.header.seq);

            }

            if (result) {

            } else {

                // Consume the next batched event unless batches are being held for later.

                if (consumeBatches || result != WOULD_BLOCK) {

                    result = consumeBatch(factory, frameTime, outSeq, outEvent);

                if (consumeBatches || result.error().code() != WOULD_BLOCK) {

                    result = android::base::Error(

                            consumeBatch(factory, frameTime, outSeq, outEvent));

                    if (*outEvent) {

                        ALOGD_IF(DEBUG_TRANSPORT_CONSUMER,

                                 "channel '%s' consumer ~ consumed batch event, seq=%u",
@@ -256,7 +257,7 @@ status_t InputConsumer::consume(InputEventFactoryInterface* factory, bool consum 
                        break;

                    }

                }

                return result;

                return result.error().code();

            }

        }

libs/input/InputConsumerNoResampling.cpp

+30 −30
Original line number Diff line number Diff line
@@ -362,10 +362,9 @@ void InputConsumerNoResampling::handleMessages(std::vector<InputMessage>&& messa 
std::vector<InputMessage> InputConsumerNoResampling::readAllMessages() {

    std::vector<InputMessage> messages;

    while (true) {

        InputMessage msg;

        status_t result = mChannel->receiveMessage(&msg);

        switch (result) {

            case OK: {

        android::base::Result<InputMessage> result = mChannel->receiveMessage();

        if (result.ok()) {

            const InputMessage& msg = *result;

            const auto [_, inserted] =

                    mConsumeTimes.emplace(msg.header.seq, systemTime(SYSTEM_TIME_MONOTONIC));

            LOG_ALWAYS_FATAL_IF(!inserted, "Already have a consume time for seq=%" PRIu32,
@@ -376,8 +375,8 @@ std::vector<InputMessage> InputConsumerNoResampling::readAllMessages() { 
            // in the same process.

            ATRACE_ASYNC_BEGIN("InputConsumer processing", /*cookie=*/msg.header.seq);

            messages.push_back(msg);

                break;

            }

        } else { // !result.ok()

            switch (result.error().code()) {

                case WOULD_BLOCK: {

                    return messages;

                }
@@ -390,12 +389,13 @@ std::vector<InputMessage> InputConsumerNoResampling::readAllMessages() { 
                    break;

                }

                default: {

                LOG(FATAL) << "Unexpected error: " << result;

                    LOG(FATAL) << "Unexpected error: " << result.error().message();

                    break;

                }

            }

        }

    }

}



void InputConsumerNoResampling::handleMessage(const InputMessage& msg) const {

    switch (msg.header.type) {

libs/input/InputTransport.cpp

+20 −18
Original line number Diff line number Diff line
@@ -424,10 +424,11 @@ status_t InputChannel::sendMessage(const InputMessage* msg) { 
    return OK;

}



status_t InputChannel::receiveMessage(InputMessage* msg) {

android::base::Result<InputMessage> InputChannel::receiveMessage() {

    ssize_t nRead;

    InputMessage msg;

    do {

        nRead = ::recv(getFd(), msg, sizeof(InputMessage), MSG_DONTWAIT);

        nRead = ::recv(getFd(), &msg, sizeof(InputMessage), MSG_DONTWAIT);

    } while (nRead == -1 && errno == EINTR);



    if (nRead < 0) {
@@ -435,36 +436,36 @@ status_t InputChannel::receiveMessage(InputMessage* msg) { 
        ALOGD_IF(DEBUG_CHANNEL_MESSAGES, "channel '%s' ~ receive message failed, errno=%d",

                 name.c_str(), errno);

        if (error == EAGAIN || error == EWOULDBLOCK) {

            return WOULD_BLOCK;

            return android::base::Error(WOULD_BLOCK);

        }

        if (error == EPIPE || error == ENOTCONN || error == ECONNREFUSED) {

            return DEAD_OBJECT;

            return android::base::Error(DEAD_OBJECT);

        }

        return -error;

        return android::base::Error(-error);

    }



    if (nRead == 0) { // check for EOF

        ALOGD_IF(DEBUG_CHANNEL_MESSAGES,

                 "channel '%s' ~ receive message failed because peer was closed", name.c_str());

        return DEAD_OBJECT;

        return android::base::Error(DEAD_OBJECT);

    }



    if (!msg->isValid(nRead)) {

    if (!msg.isValid(nRead)) {

        ALOGE("channel '%s' ~ received invalid message of size %zd", name.c_str(), nRead);

        return BAD_VALUE;

        return android::base::Error(BAD_VALUE);

    }



    ALOGD_IF(DEBUG_CHANNEL_MESSAGES, "channel '%s' ~ received message of type %s", name.c_str(),

             ftl::enum_string(msg->header.type).c_str());

             ftl::enum_string(msg.header.type).c_str());

    if (ATRACE_ENABLED()) {

        // Add an additional trace point to include data about the received message.

        std::string message =

                StringPrintf("receiveMessage(inputChannel=%s, seq=0x%" PRIx32 ", type=%s)",

                             name.c_str(), msg->header.seq,

                             ftl::enum_string(msg->header.type).c_str());

                             name.c_str(), msg.header.seq,

                             ftl::enum_string(msg.header.type).c_str());

        ATRACE_NAME(message.c_str());

    }

    return OK;

    return msg;

}



bool InputChannel::probablyHasInput() const {
@@ -729,15 +730,16 @@ status_t InputPublisher::publishTouchModeEvent(uint32_t seq, int32_t eventId, bo 
}



android::base::Result<InputPublisher::ConsumerResponse> InputPublisher::receiveConsumerResponse() {

    InputMessage msg;

    status_t result = mChannel->receiveMessage(&msg);

    if (result) {

        if (debugTransportPublisher() && result != WOULD_BLOCK) {

    android::base::Result<InputMessage> result = mChannel->receiveMessage();

    if (!result.ok()) {

        if (debugTransportPublisher() && result.error().code() != WOULD_BLOCK) {

            LOG(INFO) << "channel '" << mChannel->getName() << "' publisher ~ " << __func__ << ": "

                      << strerror(result);

                      << result.error().message();

        }

        return android::base::Error(result);

        return result.error();

    }



    const InputMessage& msg = *result;

    if (msg.header.type == InputMessage::Type::FINISHED) {

        ALOGD_IF(debugTransportPublisher(),

                 "channel '%s' publisher ~ %s: finished: seq=%u, handled=%s",

libs/input/tests/InputChannel_test.cpp

+19 −14
Original line number Diff line number Diff line
@@ -78,9 +78,10 @@ TEST_F(InputChannelTest, OpenInputChannelPair_ReturnsAPairOfConnectedChannels) { 
    EXPECT_EQ(OK, serverChannel->sendMessage(&serverMsg))

            << "server channel should be able to send message to client channel";



    InputMessage clientMsg;

    EXPECT_EQ(OK, clientChannel->receiveMessage(&clientMsg))

    android::base::Result<InputMessage> clientMsgResult = clientChannel->receiveMessage();

    ASSERT_TRUE(clientMsgResult.ok())

            << "client channel should be able to receive message from server channel";

    const InputMessage& clientMsg = *clientMsgResult;

    EXPECT_EQ(serverMsg.header.type, clientMsg.header.type)

            << "client channel should receive the correct message from server channel";

    EXPECT_EQ(serverMsg.body.key.action, clientMsg.body.key.action)
@@ -94,9 +95,10 @@ TEST_F(InputChannelTest, OpenInputChannelPair_ReturnsAPairOfConnectedChannels) { 
    EXPECT_EQ(OK, clientChannel->sendMessage(&clientReply))

            << "client channel should be able to send message to server channel";



    InputMessage serverReply;

    EXPECT_EQ(OK, serverChannel->receiveMessage(&serverReply))

    android::base::Result<InputMessage> serverReplyResult = serverChannel->receiveMessage();

    ASSERT_TRUE(serverReplyResult.ok())

            << "server channel should be able to receive message from client channel";

    const InputMessage& serverReply = *serverReplyResult;

    EXPECT_EQ(clientReply.header.type, serverReply.header.type)

            << "server channel should receive the correct message from client channel";

    EXPECT_EQ(clientReply.header.seq, serverReply.header.seq)
@@ -134,9 +136,10 @@ TEST_F(InputChannelTest, ProbablyHasInput) { 
            << "client channel should observe that message is available before receiving it";



    // Receive (consume) the message.

    InputMessage clientMsg;

    EXPECT_EQ(OK, receiverChannel->receiveMessage(&clientMsg))

    android::base::Result<InputMessage> clientMsgResult = receiverChannel->receiveMessage();

    ASSERT_TRUE(clientMsgResult.ok())

            << "client channel should be able to receive message from server channel";

    const InputMessage& clientMsg = *clientMsgResult;

    EXPECT_EQ(serverMsg.header.type, clientMsg.header.type)

            << "client channel should receive the correct message from server channel";

    EXPECT_EQ(serverMsg.body.key.action, clientMsg.body.key.action)
@@ -156,8 +159,8 @@ TEST_F(InputChannelTest, ReceiveSignal_WhenNoSignalPresent_ReturnsAnError) { 
    ASSERT_EQ(OK, result)

            << "should have successfully opened a channel pair";



    InputMessage msg;

    EXPECT_EQ(WOULD_BLOCK, clientChannel->receiveMessage(&msg))

    android::base::Result<InputMessage> msgResult = clientChannel->receiveMessage();

    EXPECT_EQ(WOULD_BLOCK, msgResult.error().code())

            << "receiveMessage should have returned WOULD_BLOCK";

}
@@ -172,8 +175,8 @@ TEST_F(InputChannelTest, ReceiveSignal_WhenPeerClosed_ReturnsAnError) { 


    serverChannel.reset(); // close server channel



    InputMessage msg;

    EXPECT_EQ(DEAD_OBJECT, clientChannel->receiveMessage(&msg))

    android::base::Result<InputMessage> msgResult = clientChannel->receiveMessage();

    EXPECT_EQ(DEAD_OBJECT, msgResult.error().code())

            << "receiveMessage should have returned DEAD_OBJECT";

}
@@ -207,7 +210,7 @@ TEST_F(InputChannelTest, SendAndReceive_MotionClassification) { 
        MotionClassification::DEEP_PRESS,

    };



    InputMessage serverMsg = {}, clientMsg;

    InputMessage serverMsg = {};

    serverMsg.header.type = InputMessage::Type::MOTION;

    serverMsg.header.seq = 1;

    serverMsg.body.motion.pointerCount = 1;
@@ -218,11 +221,13 @@ TEST_F(InputChannelTest, SendAndReceive_MotionClassification) { 
        EXPECT_EQ(OK, serverChannel->sendMessage(&serverMsg))

                << "server channel should be able to send message to client channel";



        EXPECT_EQ(OK, clientChannel->receiveMessage(&clientMsg))

        android::base::Result<InputMessage> clientMsgResult = clientChannel->receiveMessage();

        ASSERT_TRUE(clientMsgResult.ok())

                << "client channel should be able to receive message from server channel";

        const InputMessage& clientMsg = *clientMsgResult;

        EXPECT_EQ(serverMsg.header.type, clientMsg.header.type);

        EXPECT_EQ(classification, clientMsg.body.motion.classification) <<

                "Expected to receive " << motionClassificationToString(classification);

        EXPECT_EQ(classification, clientMsg.body.motion.classification)

                << "Expected to receive " << motionClassificationToString(classification);

    }

}