From 10987969656a44eb1b3b5b6a944a9d1ee8684277 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Tue, 25 Oct 2022 08:44:18 -0400 Subject: [PATCH] Add a private method on SystemPacketBuffer to find where its reserved space starts. (#23284) Fixes https://github.com/project-chip/connectedhomeip/issues/5372 --- src/system/SystemPacketBuffer.cpp | 40 ++++++++++++++++++------------- src/system/SystemPacketBuffer.h | 8 +++++++ 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/src/system/SystemPacketBuffer.cpp b/src/system/SystemPacketBuffer.cpp index f47a54278433e2..e80ddd70b746ce 100644 --- a/src/system/SystemPacketBuffer.cpp +++ b/src/system/SystemPacketBuffer.cpp @@ -133,9 +133,9 @@ void PacketBufferHandle::InternalRightSize() } // Reallocate only if enough space will be saved. - uint8_t * const start = reinterpret_cast(mBuffer) + PacketBuffer::kStructureSize; - uint8_t * const payload = reinterpret_cast(mBuffer->payload); - const uint16_t usedSize = static_cast(payload - start + mBuffer->len); + const uint8_t * const start = mBuffer->ReserveStart(); + const uint8_t * const payload = mBuffer->Start(); + const uint16_t usedSize = static_cast(payload - start + mBuffer->len); if (usedSize + kRightSizingThreshold > mBuffer->alloc_size) { return; @@ -149,14 +149,14 @@ void PacketBufferHandle::InternalRightSize() return; } - uint8_t * const newStart = reinterpret_cast(newBuffer) + PacketBuffer::kStructureSize; + uint8_t * const newStart = newBuffer->ReserveStart(); newBuffer->next = nullptr; newBuffer->payload = newStart + (payload - start); newBuffer->tot_len = mBuffer->tot_len; newBuffer->len = mBuffer->len; newBuffer->ref = 1; newBuffer->alloc_size = static_cast(usedSize); - memcpy(reinterpret_cast(newBuffer) + PacketBuffer::kStructureSize, start, usedSize); + memcpy(newStart, start, usedSize); PacketBuffer::Free(mBuffer); mBuffer = newBuffer; @@ -193,7 +193,7 @@ void PacketBufferHandle::InternalRightSize() void PacketBuffer::SetStart(uint8_t * aNewStart) { - uint8_t * const kStart = reinterpret_cast(this) + kStructureSize; + uint8_t * const kStart = ReserveStart(); uint8_t * const kEnd = kStart + this->AllocSize(); if (aNewStart < kStart) @@ -236,9 +236,7 @@ void PacketBuffer::SetDataLength(uint16_t aNewLen, PacketBuffer * aChainHead) uint16_t PacketBuffer::MaxDataLength() const { - const uint8_t * const kStart = reinterpret_cast(this) + kStructureSize; - const ptrdiff_t kDelta = static_cast(this->payload) - kStart; - return static_cast(this->AllocSize() - kDelta); + return static_cast(AllocSize() - ReservedSize()); } uint16_t PacketBuffer::AvailableDataLength() const @@ -248,10 +246,19 @@ uint16_t PacketBuffer::AvailableDataLength() const uint16_t PacketBuffer::ReservedSize() const { - // Cast to size_t is safe because this->payload always points to "after" - // this. - const size_t kDelta = static_cast(static_cast(this->payload) - reinterpret_cast(this)); - return static_cast(kDelta - kStructureSize); + // Cast to uint16_t is safe because Start() always points to "after" + // ReserveStart(). At least when the payload is stored inline. + return static_cast(Start() - ReserveStart()); +} + +uint8_t * PacketBuffer::ReserveStart() +{ + return reinterpret_cast(this) + kStructureSize; +} + +const uint8_t * PacketBuffer::ReserveStart() const +{ + return reinterpret_cast(this) + kStructureSize; } void PacketBuffer::AddToEnd(PacketBufferHandle && aPacketHandle) @@ -282,7 +289,7 @@ void PacketBuffer::AddToEnd(PacketBufferHandle && aPacketHandle) void PacketBuffer::CompactHead() { - uint8_t * const kStart = reinterpret_cast(this) + kStructureSize; + uint8_t * const kStart = ReserveStart(); if (this->payload != kStart) { @@ -505,7 +512,7 @@ PacketBufferHandle PacketBufferHandle::New(size_t aAvailableSize, uint16_t aRese return PacketBufferHandle(); } - lPacket->payload = reinterpret_cast(lPacket) + PacketBuffer::kStructureSize + aReservedSize; + lPacket->payload = lPacket->ReserveStart() + aReservedSize; lPacket->len = lPacket->tot_len = 0; lPacket->next = nullptr; lPacket->ref = 1; @@ -669,8 +676,7 @@ PacketBufferHandle PacketBufferHandle::CloneData() const return PacketBufferHandle(); } clone.mBuffer->tot_len = clone.mBuffer->len = original->len; - memcpy(reinterpret_cast(clone.mBuffer) + PacketBuffer::kStructureSize, - reinterpret_cast(original) + PacketBuffer::kStructureSize, originalDataSize + originalReservedSize); + memcpy(clone->ReserveStart(), original->ReserveStart(), originalDataSize + originalReservedSize); if (cloneHead.IsNull()) { diff --git a/src/system/SystemPacketBuffer.h b/src/system/SystemPacketBuffer.h index c61cbcd89bf119..2e8b3635b823d0 100644 --- a/src/system/SystemPacketBuffer.h +++ b/src/system/SystemPacketBuffer.h @@ -383,6 +383,14 @@ class DLL_EXPORT PacketBuffer : private pbuf void Clear(); void SetDataLength(uint16_t aNewLen, PacketBuffer * aChainHead); + /** + * Get a pointer to the start of the reserved space (which comes before the + * payload). The actual reserved space is the ReservedSize() bytes starting + * at this pointer. + */ + uint8_t * ReserveStart(); + const uint8_t * ReserveStart() const; + friend class PacketBufferHandle; friend class ::PacketBufferTest; };