Skip to content

Commit

Permalink
Address CI comments
Browse files Browse the repository at this point in the history
  • Loading branch information
tehampson committed Dec 1, 2023
1 parent f0c9ba6 commit 1217558
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 14 deletions.
2 changes: 1 addition & 1 deletion src/lib/core/TLVBackingStore.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 2 additions & 5 deletions src/lib/core/TLVWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
10 changes: 3 additions & 7 deletions src/system/TLVPacketBufferBackingStore.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion src/system/tests/TestTLVPacketBufferBackingStore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<uint8_t>(7));
NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR);

Expand Down

0 comments on commit 1217558

Please sign in to comment.