Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revise PacketBufferWriter interface #4874

Merged
merged 5 commits into from
Feb 17, 2021
Merged

Revise PacketBufferWriter interface #4874

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
05bb7a3
Revise PacketBufferWriter interface
kpschoedel Feb 16, 2021
af6e552
* fix doxygen
kpschoedel Feb 16, 2021
a318fbf
Merge remote-tracking branch 'origin/master' into pbw3
kpschoedel Feb 16, 2021
607deb0
* review
kpschoedel Feb 16, 2021
317fc80
* review 2
kpschoedel Feb 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/app/encoder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ uint16_t encodeApsFrame(uint8_t * buffer, uint16_t buf_length, EmberApsFrame * a
#define COMMAND_HEADER(name, clusterId) \
const char * kName = name; \
\
PacketBufferWriter buf(kMaxBufferSize); \
PacketBufferWriter buf(System::PacketBufferHandle::New(kMaxBufferSize)); \
if (buf.IsNull()) \
{ \
ChipLogError(Zcl, "Could not allocate packet buffer while trying to encode %s command", kName); \
Expand Down
2 changes: 1 addition & 1 deletion src/app/zap-templates/templates/chip/encoder-src.zapt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ uint16_t encodeApsFrame(uint8_t * buffer, uint16_t buf_length, EmberApsFrame * a
#define COMMAND_HEADER(name, clusterId) \
const char * kName = name; \
\
PacketBufferWriter buf(kMaxBufferSize); \
PacketBufferWriter buf(System::PacketBufferHandle::New(kMaxBufferSize)); \
if (buf.IsNull()) \
{ \
ChipLogError(Zcl, "Could not allocate packet buffer while trying to encode %s command", kName); \
Expand Down
2 changes: 1 addition & 1 deletion src/protocols/bdx/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,6 @@ static_library("bdx") {
"${chip_root}/src/lib/core",
"${chip_root}/src/lib/support",
"${chip_root}/src/system",
"${chip_root}/src/transport/raw",
"${chip_root}/src/transport",
]
}
6 changes: 3 additions & 3 deletions src/protocols/bdx/BdxTransferSession.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include <support/CodeUtils.h>
#include <support/ReturnMacros.h>
#include <system/SystemPacketBuffer.h>
#include <transport/SecureSessionMgr.h>

namespace {
constexpr uint8_t kBdxVersion = 0; ///< The version of this implementation of the BDX spec
Expand All @@ -25,7 +26,7 @@ constexpr size_t kStatusReportMinSize = 2 + 4 + 2; ///< 16 bits for GeneralCode,
CHIP_ERROR WriteToPacketBuffer(const ::chip::bdx::BdxMessage & msgStruct, ::chip::System::PacketBufferHandle & msgBuf)
{
size_t msgDataSize = msgStruct.MessageSize();
::chip::Encoding::LittleEndian::PacketBufferWriter bbuf(msgDataSize);
::chip::Encoding::LittleEndian::PacketBufferWriter bbuf(chip::MessagePacketBuffer::New(msgDataSize), msgDataSize);
if (bbuf.IsNull())
{
return CHIP_ERROR_NO_MEMORY;
Expand All @@ -36,7 +37,6 @@ CHIP_ERROR WriteToPacketBuffer(const ::chip::bdx::BdxMessage & msgStruct, ::chip
{
return CHIP_ERROR_NO_MEMORY;
}

return CHIP_NO_ERROR;
}

Expand Down Expand Up @@ -848,7 +848,7 @@ void TransferSession::PrepareStatusReport(StatusCode code)
{
mStatusReportData.StatusCode = code;

Encoding::LittleEndian::PacketBufferWriter bbuf(kStatusReportMinSize);
Encoding::LittleEndian::PacketBufferWriter bbuf(chip::MessagePacketBuffer::New(kStatusReportMinSize), kStatusReportMinSize);
VerifyOrReturn(!bbuf.IsNull());

bbuf.Put16(static_cast<uint16_t>(Protocols::Common::StatusCode::Failure));
Expand Down
2 changes: 1 addition & 1 deletion src/protocols/bdx/tests/TestBdxMessages.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ void TestHelperWrittenAndParsedMatch(nlTestSuite * inSuite, void * inContext, Ms
CHIP_ERROR err = CHIP_NO_ERROR;

size_t msgSize = testMsg.MessageSize();
Encoding::LittleEndian::PacketBufferWriter bbuf(msgSize);
Encoding::LittleEndian::PacketBufferWriter bbuf(System::PacketBufferHandle::New(msgSize));
NL_TEST_ASSERT(inSuite, !bbuf.IsNull());

testMsg.WriteToBuffer(bbuf);
Expand Down
10 changes: 0 additions & 10 deletions src/system/SystemPacketBuffer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -619,16 +619,6 @@ PacketBufferHandle PacketBufferHandle::CloneData(uint16_t aAdditionalSize, uint1

namespace Encoding {

void PacketBufferWriterUtil::Initialize(BufferWriter & aBufferWriter, System::PacketBufferHandle & aPacket, size_t aAvailableSize,
uint16_t aReservedSize)
{
aPacket = System::PacketBufferHandle::New(aAvailableSize, aReservedSize);
if (!aPacket.IsNull())
{
aBufferWriter = Encoding::BufferWriter(aPacket->Start(), aAvailableSize);
}
}

System::PacketBufferHandle PacketBufferWriterUtil::Finalize(BufferWriter & aBufferWriter, System::PacketBufferHandle & aPacket)
{
if (!aPacket.IsNull() && aBufferWriter.Fit())
Expand Down
29 changes: 17 additions & 12 deletions src/system/SystemPacketBuffer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include <system/SystemError.h>

#include <stddef.h>
#include <utility>

#if CHIP_SYSTEM_CONFIG_USE_LWIP
#include <lwip/mem.h>
Expand All @@ -60,7 +61,6 @@ class PacketBufferTest;

#undef CHIP_SYSTEM_PACKETBUFFER_HAS_RIGHT_SIZE // True if RightSize() has a nontrivial implementation
#undef CHIP_SYSTEM_PACKETBUFFER_HAS_CHECK // True if Check() has a nontrivial implementation

#if CHIP_SYSTEM_CONFIG_USE_LWIP
#if LWIP_PBUF_FROM_CUSTOM_POOLS
#define CHIP_SYSTEM_PACKETBUFFER_STORE CHIP_SYSTEM_PACKETBUFFER_STORE_LWIP_CUSTOM
Expand Down Expand Up @@ -697,8 +697,6 @@ class PacketBufferWriterUtil
private:
template <typename>
friend class PacketBufferWriterBase;
static void Initialize(BufferWriter & aBufferWriter, System::PacketBufferHandle & aPacket, size_t aAvailableSize,
uint16_t aReservedSize);
static System::PacketBufferHandle Finalize(BufferWriter & aBufferWriter, System::PacketBufferHandle & aPacket);
};

Expand All @@ -721,19 +719,26 @@ class PacketBufferWriterBase : public Writer
{
public:
/**
* Constructs a BufferWriter that writes into a newly allocated packet buffer.
* Constructs a BufferWriter that writes into a packet buffer, using all avaiable space.
*
* If no memory is available, or if the size requested is too large, then \c IsNull() will be true.
* Otherwise, it is guaranteed that the BufferWriter length is \a aAvailableSize. (The underlying packet
* buffer may be larger.)
* @param[in] aPacket A handle to PacketBuffer, to be used as backing store for the BufferWriter.
*/
PacketBufferWriterBase(System::PacketBufferHandle && aPacket) :
Writer(aPacket->Start() + aPacket->DataLength(), aPacket->AvailableDataLength())
{
mPacket = std::move(aPacket);
}

/**
* Constructs a BufferWriter that writes into a packet buffer, using no more than the requested space.
*
* @param[in] aAvailableSize Length bound of the BufferWriter.
* @param[in] aReservedSize Reserved packet buffer space for protocol headers; see \c PacketBufferHandle::New().
* @param[in] aPacket A handle to PacketBuffer, to be used as backing store for the BufferWriter.
* @param[in] aSize Maximum number of octects to write into the packet buffer.
*/
PacketBufferWriterBase(size_t aAvailableSize, uint16_t aReservedSize = CHIP_SYSTEM_CONFIG_HEADER_RESERVE_SIZE) :
Writer(nullptr, 0)
PacketBufferWriterBase(System::PacketBufferHandle && aPacket, size_t aSize) :
Writer(aPacket->Start() + aPacket->DataLength(), chip::min(aSize, static_cast<size_t>(aPacket->AvailableDataLength())))
{
PacketBufferWriterUtil::Initialize(*this, mPacket, aAvailableSize, aReservedSize);
mPacket = std::move(aPacket);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/system/tests/TestSystemPacketBuffer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1715,8 +1715,8 @@ void PacketBufferTest::CheckPacketBufferWriter(nlTestSuite * inSuite, void * inC

const char kPayload[] = "Hello, world!";

PacketBufferWriter yay(sizeof(kPayload));
PacketBufferWriter nay(sizeof(kPayload) - 2);
PacketBufferWriter yay(PacketBufferHandle::New(sizeof(kPayload)));
PacketBufferWriter nay(PacketBufferHandle::New(sizeof(kPayload)), sizeof(kPayload) - 2);
NL_TEST_ASSERT(inSuite, !yay.IsNull());
NL_TEST_ASSERT(inSuite, !nay.IsNull());

Expand Down
4 changes: 2 additions & 2 deletions src/transport/NetworkProvisioning.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ CHIP_ERROR NetworkProvisioning::SendNetworkCredentials(const char * ssid, const
const size_t bufferSize = EncodedStringSize(ssid) + EncodedStringSize(passwd);
VerifyOrExit(CanCastTo<uint16_t>(bufferSize), err = CHIP_ERROR_INVALID_ARGUMENT);
{
Encoding::LittleEndian::PacketBufferWriter bbuf(bufferSize + MessagePacketBuffer::kMaxFooterSize);
Encoding::LittleEndian::PacketBufferWriter bbuf(MessagePacketBuffer::New(bufferSize), bufferSize);

ChipLogProgress(NetworkProvisioning, "Sending Network Creds. Delegate %p\n", mDelegate);
VerifyOrExit(mDelegate != nullptr, err = CHIP_ERROR_INCORRECT_STATE);
Expand Down Expand Up @@ -264,7 +264,7 @@ CHIP_ERROR NetworkProvisioning::SendThreadCredentials(const DeviceLayer::Interna
4; // threadData.ThreadChannel, threadData.FieldPresent.ThreadExtendedPANId,
// threadData.FieldPresent.ThreadMeshPrefix, threadData.FieldPresent.ThreadPSKc
/* clang-format on */
Encoding::LittleEndian::PacketBufferWriter bbuf(credentialSize + MessagePacketBuffer::kMaxFooterSize);
Encoding::LittleEndian::PacketBufferWriter bbuf(MessagePacketBuffer::New(credentialSize), credentialSize);

ChipLogProgress(NetworkProvisioning, "Sending Thread Credentials");
VerifyOrExit(mDelegate != nullptr, err = CHIP_ERROR_INCORRECT_STATE);
Expand Down
4 changes: 2 additions & 2 deletions src/transport/PASESession.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -628,7 +628,7 @@ CHIP_ERROR PASESession::HandleMsg1_and_SendMsg2(const PacketHeader & header, con
data_len = static_cast<uint16_t>(Y_len + verifier_len);

{
Encoding::PacketBufferWriter bbuf(data_len);
Encoding::PacketBufferWriter bbuf(System::PacketBufferHandle::New(data_len));
VerifyOrExit(!bbuf.IsNull(), err = CHIP_SYSTEM_ERROR_NO_MEMORY);
bbuf.Put(&Y[0], Y_len);
bbuf.Put(verifier, verifier_len);
Expand Down Expand Up @@ -684,7 +684,7 @@ CHIP_ERROR PASESession::HandleMsg2_and_SendMsg3(const PacketHeader & header, con
}

{
Encoding::PacketBufferWriter bbuf(verifier_len);
Encoding::PacketBufferWriter bbuf(System::PacketBufferHandle::New(verifier_len));
VerifyOrExit(!bbuf.IsNull(), err = CHIP_SYSTEM_ERROR_NO_MEMORY);

bbuf.Put(verifier, verifier_len);
Expand Down