From 1ddb9fe75d49a453f28b6191b2a750aff6b44a0b Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Mon, 18 Dec 2023 16:40:49 +0000 Subject: [PATCH] CommandSender and CommandHandler reserves space for Finalize to succeed --- src/app/CommandHandler.cpp | 7 ++ src/app/CommandSender.cpp | 9 ++ src/app/MessageDef/InvokeRequestMessage.cpp | 9 ++ src/app/MessageDef/InvokeRequestMessage.h | 7 ++ src/app/MessageDef/InvokeRequests.cpp | 6 ++ src/app/MessageDef/InvokeRequests.h | 7 ++ src/app/MessageDef/InvokeResponseIBs.cpp | 6 ++ src/app/MessageDef/InvokeResponseIBs.h | 7 ++ src/app/MessageDef/InvokeResponseMessage.cpp | 9 ++ src/app/MessageDef/InvokeResponseMessage.h | 7 ++ src/app/tests/TestMessageDef.cpp | 100 +++++++++++++++++++ 11 files changed, 174 insertions(+) diff --git a/src/app/CommandHandler.cpp b/src/app/CommandHandler.cpp index 1b4f8ea92998ba..b7c22d388e53a5 100644 --- a/src/app/CommandHandler.cpp +++ b/src/app/CommandHandler.cpp @@ -69,6 +69,10 @@ CHIP_ERROR CommandHandler::AllocateBuffer() mInvokeResponseBuilder.CreateInvokeResponses(); ReturnErrorOnFailure(mInvokeResponseBuilder.GetError()); + + auto * tlvWriter = mInvokeResponseBuilder.GetWriter(); + ReturnErrorOnFailure(tlvWriter->ReserveBuffer(mInvokeResponseBuilder.GetInvokeResponses().GetSizeToEndInvokeResponses())); + ReturnErrorOnFailure(tlvWriter->ReserveBuffer(mInvokeResponseBuilder.GetSizeToEndInvokeResponseMessage())); mBufferAllocated = true; } @@ -651,6 +655,9 @@ CHIP_ERROR CommandHandler::FinishCommand(bool aStartDataStruct) ReturnErrorOnFailure(commandData.Ref(mRefForResponse.Value())); } + auto * tlvWriter = mInvokeResponseBuilder.GetWriter(); + ReturnErrorOnFailure(tlvWriter->UnreserveBuffer(mInvokeResponseBuilder.GetInvokeResponses().GetSizeToEndInvokeResponses())); + ReturnErrorOnFailure(tlvWriter->UnreserveBuffer(mInvokeResponseBuilder.GetSizeToEndInvokeResponseMessage())); ReturnErrorOnFailure(commandData.EndOfCommandDataIB()); ReturnErrorOnFailure(mInvokeResponseBuilder.GetInvokeResponses().GetInvokeResponse().EndOfInvokeResponseIB()); MoveToState(State::AddedCommand); diff --git a/src/app/CommandSender.cpp b/src/app/CommandSender.cpp index a17022d80a1f77..0e7963637b4062 100644 --- a/src/app/CommandSender.cpp +++ b/src/app/CommandSender.cpp @@ -91,6 +91,10 @@ CHIP_ERROR CommandSender::AllocateBuffer() mInvokeRequestBuilder.CreateInvokeRequests(); ReturnErrorOnFailure(mInvokeRequestBuilder.GetError()); + auto * tlvWriter = mInvokeRequestBuilder.GetWriter(); + ReturnErrorOnFailure(tlvWriter->ReserveBuffer(mInvokeRequestBuilder.GetInvokeRequests().GetSizeToEndInvokeRequests())); + ReturnErrorOnFailure(tlvWriter->ReserveBuffer(mInvokeRequestBuilder.GetSizeToEndInvokeRequestMessage())); + mBufferAllocated = true; } @@ -508,6 +512,11 @@ CHIP_ERROR CommandSender::FinishCommand(const Optional & aTimedInvokeT CHIP_ERROR CommandSender::Finalize(System::PacketBufferHandle & commandPacket) { VerifyOrReturnError(mState == State::AddedCommand, CHIP_ERROR_INCORRECT_STATE); + + auto * tlvWriter = mInvokeRequestBuilder.GetWriter(); + ReturnErrorOnFailure(tlvWriter->UnreserveBuffer(mInvokeRequestBuilder.GetInvokeRequests().GetSizeToEndInvokeRequests())); + ReturnErrorOnFailure(tlvWriter->UnreserveBuffer(mInvokeRequestBuilder.GetSizeToEndInvokeRequestMessage())); + ReturnErrorOnFailure(mInvokeRequestBuilder.GetInvokeRequests().EndOfInvokeRequests()); ReturnErrorOnFailure(mInvokeRequestBuilder.EndOfInvokeRequestMessage()); return mCommandMessageWriter.Finalize(&commandPacket); diff --git a/src/app/MessageDef/InvokeRequestMessage.cpp b/src/app/MessageDef/InvokeRequestMessage.cpp index 3cd3066f03e4ef..c6fc665fae1966 100644 --- a/src/app/MessageDef/InvokeRequestMessage.cpp +++ b/src/app/MessageDef/InvokeRequestMessage.cpp @@ -152,5 +152,14 @@ CHIP_ERROR InvokeRequestMessage::Builder::EndOfInvokeRequestMessage() } return GetError(); } + +uint32_t InvokeRequestMessage::Builder::GetSizeToEndInvokeRequestMessage() +{ + // This encodes uint8_t into Tag 0xFF. This means 1 bytes for tag, 1 byte for length, 1 byte for value + uint32_t kEncodeInteractionModelSize = 1 + 1 + 1; + uint32_t kEndOfContainerSize = 1; + + return kEncodeInteractionModelSize + kEndOfContainerSize; +} }; // namespace app }; // namespace chip diff --git a/src/app/MessageDef/InvokeRequestMessage.h b/src/app/MessageDef/InvokeRequestMessage.h index 0678e03ec10480..3c7be83be46012 100644 --- a/src/app/MessageDef/InvokeRequestMessage.h +++ b/src/app/MessageDef/InvokeRequestMessage.h @@ -106,6 +106,13 @@ class Builder : public MessageBuilder */ CHIP_ERROR EndOfInvokeRequestMessage(); + /** + * @brief Get number of bytes required to call EndOfInvokeRequestMessage() + * + * @return Expected number of bytes required to call EndOfInvokeRequestMessage() + */ + uint32_t GetSizeToEndInvokeRequestMessage(); + private: InvokeRequests::Builder mInvokeRequests; }; diff --git a/src/app/MessageDef/InvokeRequests.cpp b/src/app/MessageDef/InvokeRequests.cpp index 56456121de9b3d..4e46cf45799d07 100644 --- a/src/app/MessageDef/InvokeRequests.cpp +++ b/src/app/MessageDef/InvokeRequests.cpp @@ -84,5 +84,11 @@ CHIP_ERROR InvokeRequests::Builder::EndOfInvokeRequests() EndOfContainer(); return GetError(); } + +uint32_t InvokeRequests::Builder::GetSizeToEndInvokeRequests() +{ + uint32_t kEndOfContainerSize = 1; + return kEndOfContainerSize; +} } // namespace app } // namespace chip diff --git a/src/app/MessageDef/InvokeRequests.h b/src/app/MessageDef/InvokeRequests.h index 71f9f74c328725..06f4402d37c052 100644 --- a/src/app/MessageDef/InvokeRequests.h +++ b/src/app/MessageDef/InvokeRequests.h @@ -61,6 +61,13 @@ class Builder : public ArrayBuilder */ CHIP_ERROR EndOfInvokeRequests(); + /** + * @brief Get number of bytes required to call EndOfInvokeRequests() + * + * @return Expected number of bytes required to call EndOfInvokeRequests() + */ + uint32_t GetSizeToEndInvokeRequests(); + private: CommandDataIB::Builder mCommandData; }; diff --git a/src/app/MessageDef/InvokeResponseIBs.cpp b/src/app/MessageDef/InvokeResponseIBs.cpp index 536d18abb25687..e22e9e72a93ddf 100644 --- a/src/app/MessageDef/InvokeResponseIBs.cpp +++ b/src/app/MessageDef/InvokeResponseIBs.cpp @@ -84,5 +84,11 @@ CHIP_ERROR InvokeResponseIBs::Builder::EndOfInvokeResponses() EndOfContainer(); return GetError(); } + +uint32_t InvokeResponseIBs::Builder::GetSizeToEndInvokeResponses() +{ + uint32_t kEndOfContainerSize = 1; + return kEndOfContainerSize; +} } // namespace app } // namespace chip diff --git a/src/app/MessageDef/InvokeResponseIBs.h b/src/app/MessageDef/InvokeResponseIBs.h index 2531785fd6fa94..e739664664b5cd 100644 --- a/src/app/MessageDef/InvokeResponseIBs.h +++ b/src/app/MessageDef/InvokeResponseIBs.h @@ -61,6 +61,13 @@ class Builder : public ArrayBuilder */ CHIP_ERROR EndOfInvokeResponses(); + /** + * @brief Get number of bytes required to call EndOfInvokeResponses() + * + * @return Expected number of bytes required to call EndOfInvokeResponses() + */ + uint32_t GetSizeToEndInvokeResponses(); + private: InvokeResponseIB::Builder mInvokeResponse; }; diff --git a/src/app/MessageDef/InvokeResponseMessage.cpp b/src/app/MessageDef/InvokeResponseMessage.cpp index cb36aa15160633..a19bd4affab967 100644 --- a/src/app/MessageDef/InvokeResponseMessage.cpp +++ b/src/app/MessageDef/InvokeResponseMessage.cpp @@ -152,5 +152,14 @@ CHIP_ERROR InvokeResponseMessage::Builder::EndOfInvokeResponseMessage() } return GetError(); } + +uint32_t InvokeResponseMessage::Builder::GetSizeToEndInvokeResponseMessage() +{ + // This encodes uint8_t into Tag 0xFF. This means 1 bytes for tag, 1 byte for length, 1 byte for value + uint32_t kEncodeInteractionModelSize = 1 + 1 + 1; + uint32_t kEndOfContainerSize = 1; + + return kEncodeInteractionModelSize + kEndOfContainerSize; +} } // namespace app } // namespace chip diff --git a/src/app/MessageDef/InvokeResponseMessage.h b/src/app/MessageDef/InvokeResponseMessage.h index 34cc69aecfe976..365261f86c195e 100644 --- a/src/app/MessageDef/InvokeResponseMessage.h +++ b/src/app/MessageDef/InvokeResponseMessage.h @@ -111,6 +111,13 @@ class Builder : public MessageBuilder */ CHIP_ERROR EndOfInvokeResponseMessage(); + /** + * @brief Get number of bytes required to call EndOfInvokeResponseMessage() + * + * @return Expected number of bytes required to call EndOfInvokeResponseMessage() + */ + uint32_t GetSizeToEndInvokeResponseMessage(); + private: InvokeResponseIBs::Builder mInvokeResponses; }; diff --git a/src/app/tests/TestMessageDef.cpp b/src/app/tests/TestMessageDef.cpp index c78545926f59b2..578cd535b230ff 100644 --- a/src/app/tests/TestMessageDef.cpp +++ b/src/app/tests/TestMessageDef.cpp @@ -2057,6 +2057,54 @@ void InvokeInvokeRequestMessageTest(nlTestSuite * apSuite, void * apContext) ParseInvokeRequestMessage(apSuite, reader); } +void InvokeRequestMessageEndOfMessageReservationTest(nlTestSuite * apSuite, void * apContext) +{ + CHIP_ERROR err = CHIP_NO_ERROR; + chip::System::PacketBufferTLVWriter writer; + InvokeRequestMessage::Builder invokeRequestMessageBuilder; + const uint32_t kSmallBufferSize = 100; + writer.Init(chip::System::PacketBufferHandle::New(kSmallBufferSize, /* aReservedSize = */ 0), /* useChainedBuffers = */ false); + err = invokeRequestMessageBuilder.Init(&writer); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + uint32_t sizeToReserve = invokeRequestMessageBuilder.GetSizeToEndInvokeRequestMessage(); + err = writer.ReserveBuffer(sizeToReserve); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + uint32_t remianingLengthAfterReservation = writer.GetRemainingFreeLength(); + + err = writer.UnreserveBuffer(sizeToReserve); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + err = invokeRequestMessageBuilder.EndOfInvokeRequestMessage(); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + uint32_t remianingLengthAfterEndingInvokeRequestMessage = writer.GetRemainingFreeLength(); + NL_TEST_ASSERT(apSuite, remianingLengthAfterReservation == remianingLengthAfterEndingInvokeRequestMessage); +} + +void InvokeRequestsEndOfRequestReservationTest(nlTestSuite * apSuite, void * apContext) +{ + CHIP_ERROR err = CHIP_NO_ERROR; + chip::System::PacketBufferTLVWriter writer; + InvokeRequests::Builder invokeRequestsBuilder; + const uint32_t kSmallBufferSize = 100; + writer.Init(chip::System::PacketBufferHandle::New(kSmallBufferSize, /* aReservedSize = */ 0), /* useChainedBuffers = */ false); + err = invokeRequestsBuilder.Init(&writer); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + uint32_t sizeToReserve = invokeRequestsBuilder.GetSizeToEndInvokeRequests(); + err = writer.ReserveBuffer(sizeToReserve); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + uint32_t remianingLengthAfterReservation = writer.GetRemainingFreeLength(); + + err = writer.UnreserveBuffer(sizeToReserve); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + err = invokeRequestsBuilder.EndOfInvokeRequests(); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + uint32_t remianingLengthAfterEndingInvokeRequests = writer.GetRemainingFreeLength(); + NL_TEST_ASSERT(apSuite, remianingLengthAfterReservation == remianingLengthAfterEndingInvokeRequests); +} + void InvokeInvokeResponseMessageTest(nlTestSuite * apSuite, void * apContext) { CHIP_ERROR err = CHIP_NO_ERROR; @@ -2074,6 +2122,54 @@ void InvokeInvokeResponseMessageTest(nlTestSuite * apSuite, void * apContext) ParseInvokeResponseMessage(apSuite, reader); } +void InvokeResponseMessageEndOfMessageReservationTest(nlTestSuite * apSuite, void * apContext) +{ + CHIP_ERROR err = CHIP_NO_ERROR; + chip::System::PacketBufferTLVWriter writer; + InvokeResponseMessage::Builder invokeResponseMessageBuilder; + const uint32_t kSmallBufferSize = 100; + writer.Init(chip::System::PacketBufferHandle::New(kSmallBufferSize, /* aReservedSize = */ 0), /* useChainedBuffers = */ false); + err = invokeResponseMessageBuilder.Init(&writer); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + uint32_t sizeToReserve = invokeResponseMessageBuilder.GetSizeToEndInvokeResponseMessage(); + err = writer.ReserveBuffer(sizeToReserve); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + uint32_t remianingLengthAfterReservation = writer.GetRemainingFreeLength(); + + err = writer.UnreserveBuffer(sizeToReserve); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + err = invokeResponseMessageBuilder.EndOfInvokeResponseMessage(); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + uint32_t remianingLengthAfterEndingInvokeResponseMessage = writer.GetRemainingFreeLength(); + NL_TEST_ASSERT(apSuite, remianingLengthAfterReservation == remianingLengthAfterEndingInvokeResponseMessage); +} + +void InvokeResponsesEndOfResponseReservationTest(nlTestSuite * apSuite, void * apContext) +{ + CHIP_ERROR err = CHIP_NO_ERROR; + chip::System::PacketBufferTLVWriter writer; + InvokeResponseIBs::Builder invokeResponsesBuilder; + const uint32_t kSmallBufferSize = 100; + writer.Init(chip::System::PacketBufferHandle::New(kSmallBufferSize, /* aReservedSize = */ 0), /* useChainedBuffers = */ false); + err = invokeResponsesBuilder.Init(&writer); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + uint32_t sizeToReserve = invokeResponsesBuilder.GetSizeToEndInvokeResponses(); + err = writer.ReserveBuffer(sizeToReserve); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + uint32_t remianingLengthAfterReservation = writer.GetRemainingFreeLength(); + + err = writer.UnreserveBuffer(sizeToReserve); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + err = invokeResponsesBuilder.EndOfInvokeResponses(); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + uint32_t remianingLengthAfterEndingInvokeResponses = writer.GetRemainingFreeLength(); + NL_TEST_ASSERT(apSuite, remianingLengthAfterReservation == remianingLengthAfterEndingInvokeResponses); +} + void ReportDataMessageTest(nlTestSuite * apSuite, void * apContext) { CHIP_ERROR err = CHIP_NO_ERROR; @@ -2283,7 +2379,11 @@ const nlTest sTests[] = NL_TEST_DEF("InvokeRequestsTest", InvokeRequestsTest), NL_TEST_DEF("InvokeResponsesTest", InvokeResponsesTest), NL_TEST_DEF("InvokeInvokeRequestMessageTest", InvokeInvokeRequestMessageTest), + NL_TEST_DEF("InvokeRequestMessageEndOfMessageReservationTest", InvokeRequestMessageEndOfMessageReservationTest), + NL_TEST_DEF("InvokeRequestsEndOfRequestReservationTest", InvokeRequestsEndOfRequestReservationTest), NL_TEST_DEF("InvokeInvokeResponseMessageTest", InvokeInvokeResponseMessageTest), + NL_TEST_DEF("InvokeResponseMessageEndOfMessageReservationTest", InvokeResponseMessageEndOfMessageReservationTest), + NL_TEST_DEF("InvokeResponsesEndOfResponseReservationTest", InvokeResponsesEndOfResponseReservationTest), NL_TEST_DEF("ReportDataMessageTest", ReportDataMessageTest), NL_TEST_DEF("ReadRequestMessageTest", ReadRequestMessageTest), NL_TEST_DEF("WriteRequestMessageTest", WriteRequestMessageTest),