Skip to content

Commit

Permalink
Remove CHIP buffer size check within shell and leave this it to lower…
Browse files Browse the repository at this point in the history 
… layer (#7523)

* Remove CHIP buffer size check within shell and leave this check to transport layer

* Address review comments
  • Loading branch information
@yufengwangca
yufengwangca authored Jun 14, 2021
1 parent 1652913 commit 7124079
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 40 deletions.
34 changes: 12 additions & 22 deletions examples/shell/shell_common/cmd_ping.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class PingArguments
mLastEchoTime = 0;
mEchoCount = 0;
mEchoRespCount = 0;
mEchoReqSize = 32;
mPayloadSize = 32;
mWaitingForEchoResp = false;
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
mUsingTCP = false;
Expand All @@ -79,8 +79,8 @@ class PingArguments
uint32_t GetEchoInterval() const { return mEchoInterval; }
void SetEchoInterval(uint32_t value) { mEchoInterval = value; }

uint32_t GetEchoReqSize() const { return mEchoReqSize; }
void SetEchoReqSize(uint32_t value) { mEchoReqSize = value; }
uint32_t GetPayloadSize() const { return mPayloadSize; }
void SetPayloadSize(uint32_t value) { mPayloadSize = value; }

uint16_t GetEchoPort() const { return mEchoPort; }
void SetEchoPort(uint16_t value) { mEchoPort = value; }
Expand All @@ -107,7 +107,7 @@ class PingArguments
uint64_t mEchoRespCount;

// The CHIP Echo request payload size in bytes.
uint32_t mEchoReqSize;
uint32_t mPayloadSize;

// Max value for the number of echo requests sent.
uint32_t mMaxEchoCount;
Expand Down Expand Up @@ -143,19 +143,14 @@ CHIP_ERROR SendEchoRequest(streamer_t * stream)

Messaging::SendFlags sendFlags;
System::PacketBufferHandle payloadBuf;
char * requestData = nullptr;
uint32_t payloadSize = gPingArguments.GetPayloadSize();

uint32_t size = gPingArguments.GetEchoReqSize();
VerifyOrExit(size <= kMaxPayloadSize, err = CHIP_ERROR_MESSAGE_TOO_LONG);

requestData = static_cast<char *>(chip::Platform::MemoryAlloc(size));
VerifyOrExit(requestData != nullptr, err = CHIP_ERROR_NO_MEMORY);

snprintf(requestData, size, "Echo Message %" PRIu64 "\n", gPingArguments.GetEchoCount());

payloadBuf = MessagePacketBuffer::NewWithData(requestData, size);
payloadBuf = MessagePacketBuffer::New(payloadSize);
VerifyOrExit(!payloadBuf.IsNull(), err = CHIP_ERROR_NO_MEMORY);

memset(payloadBuf->Start(), 0, payloadSize);
payloadBuf->SetDataLength(payloadSize);

if (gPingArguments.IsUsingMRP())
{
sendFlags.Set(Messaging::SendMessageFlags::kNone);
Expand All @@ -167,7 +162,7 @@ CHIP_ERROR SendEchoRequest(streamer_t * stream)

gPingArguments.SetLastEchoTime(System::Timer::GetCurrentEpoch());

streamer_printf(stream, "\nSend echo request message with payload size: %d bytes to Node: %" PRIu64 "\n", size,
streamer_printf(stream, "\nSend echo request message with payload size: %d bytes to Node: %" PRIu64 "\n", payloadSize,
kTestDeviceNodeId);

err = gEchoClient.SendEchoRequest(std::move(payloadBuf), sendFlags);
Expand All @@ -179,11 +174,6 @@ CHIP_ERROR SendEchoRequest(streamer_t * stream)
}

exit:
if (requestData != nullptr)
{
chip::Platform::MemoryFree(requestData);
}

if (err != CHIP_NO_ERROR)
{
streamer_printf(stream, "Send echo request failed, err: %s\n", ErrorStr(err));
Expand Down Expand Up @@ -364,7 +354,7 @@ void PrintUsage(streamer_t * stream)
streamer_printf(stream, " -i <interval> ping interval time in seconds\n");
streamer_printf(stream, " -c <count> stop after <count> replies\n");
streamer_printf(stream, " -r <1|0> enable or disable MRP\n");
streamer_printf(stream, " -s <size> payload size in bytes\n");
streamer_printf(stream, " -s <size> application payload size in bytes\n");
}

int cmd_ping(int argc, char ** argv)
Expand Down Expand Up @@ -431,7 +421,7 @@ int cmd_ping(int argc, char ** argv)
}
else
{
gPingArguments.SetEchoReqSize(atol(argv[optIndex]));
gPingArguments.SetPayloadSize(atol(argv[optIndex]));
}
break;
case 'r':
Expand Down
25 changes: 8 additions & 17 deletions examples/shell/shell_common/cmd_send.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,7 @@ CHIP_ERROR SendMessage(streamer_t * stream)

Messaging::SendFlags sendFlags;
System::PacketBufferHandle payloadBuf;
char * requestData = nullptr;
uint32_t size = 0;
uint32_t payloadSize = gSendArguments.GetPayloadSize();

// Discard any existing exchange context. Effectively we can only have one exchange with
// a single node at any one time.
Expand All @@ -145,16 +144,12 @@ CHIP_ERROR SendMessage(streamer_t * stream)
gExchangeCtx = gExchangeManager.NewContext({ kTestDeviceNodeId, 0, gAdminId }, &gMockAppDelegate);
VerifyOrExit(gExchangeCtx != nullptr, err = CHIP_ERROR_NO_MEMORY);

size = gSendArguments.GetPayloadSize();
VerifyOrExit(size <= kMaxPayloadSize, err = CHIP_ERROR_INVALID_MESSAGE_LENGTH);

requestData = static_cast<char *>(chip::Platform::MemoryAlloc(size));
VerifyOrExit(requestData != nullptr, err = CHIP_ERROR_NO_MEMORY);

snprintf(requestData, size, "CHIP Message");
payloadBuf = MessagePacketBuffer::NewWithData(requestData, size);
payloadBuf = MessagePacketBuffer::New(payloadSize);
VerifyOrExit(!payloadBuf.IsNull(), err = CHIP_ERROR_NO_MEMORY);

memset(payloadBuf->Start(), 0, payloadSize);
payloadBuf->SetDataLength(payloadSize);

if (gSendArguments.IsUsingMRP())
{
sendFlags.Set(Messaging::SendMessageFlags::kNone);
Expand All @@ -169,7 +164,8 @@ CHIP_ERROR SendMessage(streamer_t * stream)

gSendArguments.SetLastSendTime(System::Timer::GetCurrentEpoch());

streamer_printf(stream, "\nSend CHIP message with payload size: %d bytes to Node: %" PRIu64 "\n", size, kTestDeviceNodeId);
streamer_printf(stream, "\nSend CHIP message with payload size: %d bytes to Node: %" PRIu64 "\n", payloadSize,
kTestDeviceNodeId);

err = gExchangeCtx->SendMessage(Protocols::Id(VendorId::Common, gSendArguments.GetProtocolId()),
gSendArguments.GetMessageType(), std::move(payloadBuf), sendFlags);
Expand All @@ -181,11 +177,6 @@ CHIP_ERROR SendMessage(streamer_t * stream)
}

exit:
if (requestData != nullptr)
{
chip::Platform::MemoryFree(requestData);
}

if (err != CHIP_NO_ERROR)
{
streamer_printf(stream, "Send CHIP message failed, err: %s\n", ErrorStr(err));
Expand Down Expand Up @@ -317,7 +308,7 @@ void PrintUsage(streamer_t * stream)
streamer_printf(stream, " -T <type> message type\n");
streamer_printf(stream, " -p <port> server port number\n");
streamer_printf(stream, " -r <1|0> enable or disable MRP\n");
streamer_printf(stream, " -s <size> payload size in bytes\n");
streamer_printf(stream, " -s <size> application payload size in bytes\n");
}

int cmd_send(int argc, char ** argv)
Expand Down
1 change: 0 additions & 1 deletion examples/shell/shell_common/include/Globals.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
constexpr size_t kMaxTcpActiveConnectionCount = 4;
constexpr size_t kMaxTcpPendingPackets = 4;
#endif
constexpr size_t kMaxPayloadSize = 1280;
constexpr size_t kResponseTimeOut = 1000;

extern chip::secure_channel::MessageCounterManager gMessageCounterManager;
Expand Down

0 comments on commit 7124079

Please sign in to comment.