From 12175585b3ec355d54808f8a3de48f4f7f23b9ad Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Fri, 1 Dec 2023 14:15:19 +0000 Subject: [PATCH] Address CI comments --- src/lib/core/TLVBackingStore.h | 2 +- src/lib/core/TLVWriter.cpp | 7 ++----- src/system/TLVPacketBufferBackingStore.h | 10 +++------- src/system/tests/TestTLVPacketBufferBackingStore.cpp | 2 +- 4 files changed, 7 insertions(+), 14 deletions(-) diff --git a/src/lib/core/TLVBackingStore.h b/src/lib/core/TLVBackingStore.h index 23291a0d4d64c0..1e232c24ed79f7 100644 --- a/src/lib/core/TLVBackingStore.h +++ b/src/lib/core/TLVBackingStore.h @@ -156,7 +156,7 @@ class DLL_EXPORT TLVBackingStore */ virtual CHIP_ERROR FinalizeBuffer(TLVWriter & writer, uint8_t * bufStart, uint32_t bufLen) = 0; - virtual bool IsSafeToReserve() { return false; } + virtual bool GetNewBufferWillAlwaysFail() { return false; } }; } // namespace TLV diff --git a/src/lib/core/TLVWriter.cpp b/src/lib/core/TLVWriter.cpp index fb04ecd7f2d96f..99e44f0753e73e 100644 --- a/src/lib/core/TLVWriter.cpp +++ b/src/lib/core/TLVWriter.cpp @@ -101,12 +101,9 @@ CHIP_ERROR TLVWriter::ReserveBuffer(uint32_t aBufferSize) { VerifyOrReturnError(mRemainingLen >= aBufferSize, CHIP_ERROR_NO_MEMORY); - uint32_t spaceLeftInCurrentBuffer = mReservedSize + mRemainingLen; - bool canPossiblyAllocateAdditionalMemory = spaceLeftInCurrentBuffer < mMaxLen; - - if (mBackingStore && canPossiblyAllocateAdditionalMemory) + if (mBackingStore) { - VerifyOrReturnError(mBackingStore->IsSafeToReserve(), CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(mBackingStore->GetNewBufferWillAlwaysFail(), CHIP_ERROR_INCORRECT_STATE); } mReservedSize += aBufferSize; mRemainingLen -= aBufferSize; diff --git a/src/system/TLVPacketBufferBackingStore.h b/src/system/TLVPacketBufferBackingStore.h index 10ffad58f76faf..6702cda259cfff 100644 --- a/src/system/TLVPacketBufferBackingStore.h +++ b/src/system/TLVPacketBufferBackingStore.h @@ -80,14 +80,10 @@ class TLVPacketBufferBackingStore : public chip::TLV::TLVBackingStore CHIP_ERROR OnInit(chip::TLV::TLVWriter & writer, uint8_t *& bufStart, uint32_t & bufLen) override; CHIP_ERROR GetNewBuffer(chip::TLV::TLVWriter & writer, uint8_t *& bufStart, uint32_t & bufLen) override; CHIP_ERROR FinalizeBuffer(chip::TLV::TLVWriter & writer, uint8_t * bufStart, uint32_t bufLen) override; - virtual bool IsSafeToReserve() override + virtual bool GetNewBufferWillAlwaysFail() override { - // 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. + // For non-chained buffers, caller is given one chunk of contiguous memory all calls to + // GetNewBuffer will fail with CHIP_ERROR_NO_MEMORY. return !mUseChainedBuffers; } diff --git a/src/system/tests/TestTLVPacketBufferBackingStore.cpp b/src/system/tests/TestTLVPacketBufferBackingStore.cpp index 0f9ff3d7c2ef20..22c39212e36b0e 100644 --- a/src/system/tests/TestTLVPacketBufferBackingStore.cpp +++ b/src/system/tests/TestTLVPacketBufferBackingStore.cpp @@ -285,7 +285,7 @@ void TLVPacketBufferBackingStoreTest::TestWriterReserveUnreserveDoesNotOverflow( uint32_t lengthRemaining = writer.GetRemainingFreeLength(); NL_TEST_ASSERT(inSuite, lengthRemaining == 1); // Lets try to overflow by getting next buffer in the chain, - // unreserving then writting until the end of the current buffer. + // unreserving then writing until the end of the current buffer. error = writer.Put(TLV::AnonymousTag(), static_cast(7)); NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR);