Skip to content

Commit

Permalink
Address comments and apply suggestions from code review
Browse files Browse the repository at this point in the history
Co-authored-by: Boris Zbarsky <[email protected]>
  • Loading branch information
pidarped and bzbarsky-apple committed Jun 11, 2024
1 parent 5f31784 commit b0f4ca0
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 13 deletions.
4 changes: 2 additions & 2 deletions src/app/ReadHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -921,11 +921,11 @@ void ReadHandler::ClearStateFlag(ReadHandlerFlags aFlag)

size_t ReadHandler::GetReportBufferMaxSize()
{
size_t maxBufSize = chip::app::kMaxSecureSduLengthBytes;
size_t maxBufSize = kMaxSecureSduLengthBytes;
Transport::SecureSession * session = GetSession();
if (session && session->AllowsLargePayload())
{
maxBufSize = chip::app::kMaxLargeSecureSduLengthBytes;
maxBufSize = kMaxLargeSecureSduLengthBytes;
}

return maxBufSize;
Expand Down
5 changes: 3 additions & 2 deletions src/app/ReadHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -335,9 +335,10 @@ class ReadHandler : public Messaging::ExchangeDelegate

/*
* Get the appropriate size of a packet buffer to allocate for encoding a Report message.
* Depending on the underlying session, which may or may not support large
* payloads, a buffer with the corresponding max size would be allocated.
* This size might depend on the underlying session used by the ReadHandler.
*
* The size returned here is the size not including the various prepended headers
* (what System::PacketBuffer calls the "available size").
*/
size_t GetReportBufferMaxSize();

Expand Down
16 changes: 7 additions & 9 deletions src/app/reporting/Engine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@ CHIP_ERROR Engine::BuildAndSendSingleReportData(ReadHandler * apReadHandler)
uint16_t reservedSize = 0;
bool hasMoreChunks = false;
bool needCloseReadHandler = false;
size_t maxSduSize = 0;
size_t reportBufferMaxSize = 0;

// Reserved size for the MoreChunks boolean flag, which takes up 1 byte for the control tag and 1 byte for the context tag.
const uint32_t kReservedSizeForMoreChunksFlag = 1 + 1;
Expand All @@ -514,16 +514,14 @@ CHIP_ERROR Engine::BuildAndSendSingleReportData(ReadHandler * apReadHandler)
VerifyOrExit(apReadHandler != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrExit(apReadHandler->GetSession() != nullptr, err = CHIP_ERROR_INCORRECT_STATE);

// Depending on whether the session supports large payload or not, the
// appropriate max size would be returned for the Report buffer.
maxSduSize = apReadHandler->GetReportBufferMaxSize();
reportBufferMaxSize = apReadHandler->GetReportBufferMaxSize();

bufHandle = System::PacketBufferHandle::New(maxSduSize);
bufHandle = System::PacketBufferHandle::New(reportBufferMaxSize);
VerifyOrExit(!bufHandle.IsNull(), err = CHIP_ERROR_NO_MEMORY);

if (bufHandle->AvailableDataLength() > maxSduSize)
if (bufHandle->AvailableDataLength() > reportBufferMaxSize)
{
reservedSize = static_cast<uint16_t>(bufHandle->AvailableDataLength() - maxSduSize);
reservedSize = static_cast<uint16_t>(bufHandle->AvailableDataLength() - reportBufferMaxSize);
}

reportDataWriter.Init(std::move(bufHandle));
Expand All @@ -532,8 +530,8 @@ CHIP_ERROR Engine::BuildAndSendSingleReportData(ReadHandler * apReadHandler)
reportDataWriter.ReserveBuffer(mReservedSize);
#endif

// Always limit the size of the generated packet to fit within kMaxSecureSduLengthBytes regardless of the available buffer
// capacity.
// Always limit the size of the generated packet to fit within the max size returned by the ReadHandler regardless
// of the available buffer capacity.
// Also, we need to reserve some extra space for the MIC field.
reportDataWriter.ReserveBuffer(static_cast<uint32_t>(reservedSize + chip::Crypto::CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES));

Expand Down

0 comments on commit b0f4ca0

Please sign in to comment.