From efa7c1011ee195ce9ea25e6cbc8763eb6c077f1e Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Thu, 30 Nov 2023 21:56:53 +0000 Subject: [PATCH] Address CI comments --- src/system/TLVPacketBufferBackingStore.h | 7 ++++++- src/system/tests/TestTLVPacketBufferBackingStore.cpp | 8 +++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/system/TLVPacketBufferBackingStore.h b/src/system/TLVPacketBufferBackingStore.h index ea32ece84f1876..10ffad58f76faf 100644 --- a/src/system/TLVPacketBufferBackingStore.h +++ b/src/system/TLVPacketBufferBackingStore.h @@ -82,7 +82,12 @@ class TLVPacketBufferBackingStore : public chip::TLV::TLVBackingStore CHIP_ERROR FinalizeBuffer(chip::TLV::TLVWriter & writer, uint8_t * bufStart, uint32_t bufLen) override; virtual bool IsSafeToReserve() override { - // It is safe to reserve if there is no chained buffers. + // When using chained buffers there is no guarantee what the size of new buffer will + // be when calling GetNewBuffer. This makes it very difficult for a caller to safely + // ensure there is always enough space at the end of new buffer for reservation, + // when going from one buffer to the next. + // For non-chained buffers, caller is given one chunk of conigous memory so it is + // possible to reserve with buffer currently in use by caller. return !mUseChainedBuffers; } diff --git a/src/system/tests/TestTLVPacketBufferBackingStore.cpp b/src/system/tests/TestTLVPacketBufferBackingStore.cpp index 9837efc1923b76..0f9ff3d7c2ef20 100644 --- a/src/system/tests/TestTLVPacketBufferBackingStore.cpp +++ b/src/system/tests/TestTLVPacketBufferBackingStore.cpp @@ -35,14 +35,12 @@ using namespace ::chip; namespace { -void WriteUntilRemainingLessThan(nlTestSuite * inSuite, PacketBufferTLVWriter & writer, const uint32_t & remainingSize) +void WriteUntilRemainingLessThan(nlTestSuite * inSuite, PacketBufferTLVWriter & writer, const uint32_t remainingSize) { - CHIP_ERROR error; uint32_t lengthRemaining = writer.GetRemainingFreeLength(); while (lengthRemaining >= remainingSize) { - error = writer.Put(TLV::AnonymousTag(), static_cast(7)); - NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + NL_TEST_ASSERT(inSuite, writer.Put(TLV::AnonymousTag(), static_cast(7)) == CHIP_NO_ERROR); lengthRemaining = writer.GetRemainingFreeLength(); } } @@ -259,7 +257,7 @@ void TLVPacketBufferBackingStoreTest::NonChainedBufferCanReserve(nlTestSuite * i uint32_t smallSize = 5; uint32_t smallerSizeToReserver = smallSize - 1; - auto buffer = PacketBufferHandle::New(smallSize, 0); + auto buffer = PacketBufferHandle::New(smallSize, /* reservedSize = */ 0); PacketBufferTLVWriter writer; writer.Init(std::move(buffer), /* useChainedBuffers = */ false);