Skip to content

Commit

Permalink
Remove support for PacketBufferTLVReader using chained buffers. (#9592)
Browse files Browse the repository at this point in the history
It was unused, and if we can guarantee a contiguous buffer consumers can be safer
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Sep 21, 2021
1 parent fa4fdd8 commit 3111257
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 17 deletions.
24 changes: 16 additions & 8 deletions src/lib/core/tests/TestCHIPTLV.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1967,7 +1967,7 @@ void CheckPacketBuffer(nlTestSuite * inSuite, void * inContext)

ReadEncoding1(inSuite, reader);

reader.Init(buf.Retain(), buf->MaxDataLength());
reader.Init(buf.Retain());
reader.ImplicitProfileId = TestProfile_2;

ReadEncoding1(inSuite, reader);
Expand Down Expand Up @@ -2477,33 +2477,41 @@ void CheckCHIPTLVSkipCircular(nlTestSuite * inSuite, void * inContext)
*/
void CheckBufferOverflow(nlTestSuite * inSuite, void * inContext)
{
System::PacketBufferTLVWriter writer;
System::PacketBufferTLVReader reader;

System::PacketBufferHandle buf = System::PacketBufferHandle::New(sizeof(Encoding1), 0);
uint16_t maxDataLen = buf->MaxDataLength();
uint16_t reserve = static_cast<uint16_t>((sizeof(Encoding1) < maxDataLen) ? (maxDataLen - sizeof(Encoding1)) + 2 : 0);

// Repeatedly write and read a TLV encoding to a chain of PacketBuffers. Use progressively larger
// and larger amounts of space in the first buffer to force the encoding/decoding to overlap the
// and larger amounts of space in the first buffer to force the encoding to overlap the
// end of the buffer and the beginning of the next.
for (; reserve < maxDataLen; reserve++)
{
buf->SetStart(buf->Start() + reserve);

writer.Init(buf.Retain(), /* useChainedBuffers = */ true);
writer.ImplicitProfileId = TestProfile_2;
{
System::PacketBufferTLVWriter writer;
// Scope for writer because we want it to go out of scope before we
// mess with the chain after writing is done.
writer.Init(buf.Retain(), /* useChainedBuffers = */ true);
writer.ImplicitProfileId = TestProfile_2;

WriteEncoding1(inSuite, writer);
WriteEncoding1(inSuite, writer);
}

TestBufferContents(inSuite, buf, Encoding1, sizeof(Encoding1));

reader.Init(buf.Retain(), /* useChainedBuffers = */ true);
// Compact the buffer, since we don't allow reading from chained
// buffers.
buf->CompactHead();

reader.Init(buf.Retain());
reader.ImplicitProfileId = TestProfile_2;

ReadEncoding1(inSuite, reader);

buf = System::PacketBufferHandle::New(System::PacketBuffer::kMaxSizeWithoutReserve, 0);
buf = System::PacketBufferHandle::New(sizeof(Encoding1), 0);
}
}

Expand Down
17 changes: 8 additions & 9 deletions src/system/TLVPacketBufferBackingStore.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,25 +87,24 @@ class TLVPacketBufferBackingStore : public chip::TLV::TLVBackingStore
bool mUseChainedBuffers;
};

class DLL_EXPORT PacketBufferTLVReader : public chip::TLV::TLVReader
class DLL_EXPORT PacketBufferTLVReader : public TLV::ContiguousBufferTLVReader
{
public:
/**
* Initializes a TLVReader object to read from a PacketBuffer.
*
* @param[in] buffer A handle to PacketBuffer, to be used as backing store for a TLV class.
* @param[in] useChainedBuffers
* If true, advance to the next buffer in the chain once all data
* in the current buffer has been consumed.
* @param[in] buffer A handle to PacketBuffer, to be used as backing
* store for a TLV class. If the buffer is chained,
* only the head of the chain will be used.
*/
void Init(chip::System::PacketBufferHandle && buffer, bool useChainedBuffers = false)
void Init(chip::System::PacketBufferHandle && buffer)
{
mBackingStore.Init(std::move(buffer), useChainedBuffers);
chip::TLV::TLVReader::Init(mBackingStore);
mBuffer = std::move(buffer);
TLV::ContiguousBufferTLVReader::Init(mBuffer->Start(), mBuffer->DataLength());
}

private:
TLVPacketBufferBackingStore mBackingStore;
PacketBufferHandle mBuffer;
};

class DLL_EXPORT PacketBufferTLVWriter : public chip::TLV::TLVWriter
Expand Down

0 comments on commit 3111257

Please sign in to comment.