Address CI comments

project-chip · Nov 30, 2023 · efa7c10 · efa7c10
1 parent 21e888d
commit efa7c10
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 6 deletions.
diff --git 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
@@ -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<uint8_t>(7));
-        NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR);
+        NL_TEST_ASSERT(inSuite, writer.Put(TLV::AnonymousTag(), static_cast<uint8_t>(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);