Skip to content

Commit

Permalink
Remove support for PacketBufferTLVReader using chained buffers.
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 committed Sep 10, 2021
1 parent 5d2654a commit 25813e3
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 25813e3

Please sign in to comment.