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

Some conversions to use PacketBufferHandle #3909

Merged
merged 10 commits into from
Nov 24, 2020
Merged

Some conversions to use PacketBufferHandle #3909

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ed7d6f8
Some conversions to use PacketBufferHandle
kpschoedel Nov 17, 2020
b863d12
Fix nRF build and TestInetLayerMulticast
kpschoedel Nov 18, 2020
167ce74
Fix src/inet/tests/TestInetLayerCommon.cpp
kpschoedel Nov 18, 2020
ef7362e
Wish there were working instructions to run this locally.
kpschoedel Nov 18, 2020
40a735d
Restyled by clang-format
restyled-commits Nov 18, 2020
a38934d
Review 1
kpschoedel Nov 19, 2020
571bb66
fix potential double free
kpschoedel Nov 19, 2020
8c03a67
Merge branch 'master' into x2707-02-receive
kpschoedel Nov 19, 2020
9c74f43
Merge remote-tracking branch 'origin/master' into x2707-02-receive
kpschoedel Nov 20, 2020
239a8c3
Merge remote-tracking branch 'origin/master' into x2707-02-receive
kpschoedel Nov 20, 2020
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
20 changes: 6 additions & 14 deletions examples/all-clusters-app/esp32/main/EchoServer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,14 +122,14 @@ class EchoServerCallback : public SecureSessionMgrDelegate
{
public:
void OnMessageReceived(const PacketHeader & header, const PayloadHeader & payloadHeader,
const Transport::PeerConnectionState * state, System::PacketBuffer * buffer,
const Transport::PeerConnectionState * state, System::PacketBufferHandle buffer,
SecureSessionMgrBase * mgr) override
{
CHIP_ERROR err;
const size_t data_len = buffer->DataLength();

// as soon as a client connects, assume it is connected
VerifyOrExit(mgr != NULL && buffer != NULL, ESP_LOGE(TAG, "Received data but couldn't process it..."));
VerifyOrExit(mgr != NULL && !buffer.IsNull(), ESP_LOGE(TAG, "Received data but couldn't process it..."));
VerifyOrExit(state->GetPeerNodeId() != kUndefinedNodeId, ESP_LOGE(TAG, "Unknown source for received message"));

{
Expand All @@ -147,8 +147,7 @@ class EchoServerCallback : public SecureSessionMgrDelegate
// port from data model processing.
if (ContentMayBeADataModelMessage(buffer))
{
HandleDataModelMessage(header, buffer, mgr);
buffer = NULL;
HandleDataModelMessage(header, std::move(buffer), mgr);
}
else
{
Expand All @@ -159,8 +158,7 @@ class EchoServerCallback : public SecureSessionMgrDelegate
ESP_LOGI(TAG, "Client sent: %s", logmsg);

// Attempt to echo back
err = mgr->SendMessage(header.GetSourceNodeId().Value(), buffer);
buffer = NULL;
err = mgr->SendMessage(header.GetSourceNodeId().Value(), buffer.Release_ForNow());
if (err != CHIP_NO_ERROR)
{
ESP_LOGE(TAG, "Unable to echo back to client: %s", ErrorStr(err));
Expand All @@ -171,13 +169,7 @@ class EchoServerCallback : public SecureSessionMgrDelegate
}
}

exit:

// SendTo calls Free on the buffer without an AddRef, if SendTo was not called, free the buffer.
if (buffer != NULL)
{
System::PacketBuffer::Free(buffer);
}
exit:;
}

void OnReceiveError(CHIP_ERROR error, const Transport::PeerAddress & source, SecureSessionMgrBase * mgr) override
Expand All @@ -200,7 +192,7 @@ class EchoServerCallback : public SecureSessionMgrDelegate
* Echo messages should generally not have a first byte with those values, so we
* can use that to try to distinguish between the two.
*/
bool ContentMayBeADataModelMessage(System::PacketBuffer * buffer)
bool ContentMayBeADataModelMessage(const System::PacketBufferHandle & buffer)
{
const size_t data_len = buffer->DataLength();
const uint8_t * data = buffer->Start();
Expand Down
314 changes: 157 additions & 157 deletions examples/chip-tool/commands/clusters/Commands.h
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion examples/chip-tool/commands/common/EchoCommand.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ void EchoCommand::SendEcho() const
}
}

void EchoCommand::ReceiveEcho(PacketBuffer * buffer) const
void EchoCommand::ReceiveEcho(PacketBufferHandle buffer) const
{
// attempt to print the incoming message
size_t data_len = buffer->DataLength();
Expand Down
4 changes: 2 additions & 2 deletions examples/chip-tool/commands/common/EchoCommand.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ class EchoCommand : public NetworkCommand

/////////// IPCommand Interface /////////
void OnConnect(ChipDeviceController * dc) override { mController = dc; }
void OnMessage(ChipDeviceController * dc, chip::System::PacketBuffer * buffer) override { ReceiveEcho(buffer); }
void OnMessage(ChipDeviceController * dc, chip::System::PacketBufferHandle buffer) override { ReceiveEcho(std::move(buffer)); }
void OnError(ChipDeviceController * dc, CHIP_ERROR err) override { mController = nullptr; }

private:
void SendEcho(void) const;
void ReceiveEcho(chip::System::PacketBuffer * buffer) const;
void ReceiveEcho(chip::System::PacketBufferHandle buffer) const;

ChipDeviceController * mController = nullptr;
};
13 changes: 6 additions & 7 deletions examples/chip-tool/commands/common/ModelCommand.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ void ModelCommand::OnError(ChipDeviceController * dc, CHIP_ERROR err)
UpdateWaitForResponse(false);
}

void ModelCommand::OnMessage(ChipDeviceController * dc, PacketBuffer * buffer)
void ModelCommand::OnMessage(ChipDeviceController * dc, PacketBufferHandle buffer)
{
SetCommandExitStatus(ReceiveCommandResponse(dc, buffer));
SetCommandExitStatus(ReceiveCommandResponse(dc, std::move(buffer)));
UpdateWaitForResponse(false);
}

Expand Down Expand Up @@ -95,7 +95,7 @@ bool ModelCommand::SendCommand(ChipDeviceController * dc)
ChipLogProgress(chipTool, "Endpoint id: '0x%02x', Cluster id: '0x%04x', Command id: '0x%02x'", mEndPointId, mClusterId,
mCommandId);

uint16_t dataLength = EncodeCommand(buffer.Get_ForNow(), bufferSize, mEndPointId);
uint16_t dataLength = EncodeCommand(buffer, bufferSize, mEndPointId);
if (dataLength == 0)
{
ChipLogError(chipTool, "Error while encoding data for command: %s", GetName());
Expand All @@ -106,14 +106,14 @@ bool ModelCommand::SendCommand(ChipDeviceController * dc)
ChipLogDetail(chipTool, "Encoded data of length %d", dataLength);

#ifdef DEBUG
PrintBuffer(buffer.Get_ForNow());
PrintBuffer(buffer);
#endif

dc->SendMessage(NULL, buffer.Release_ForNow());
return true;
}

bool ModelCommand::ReceiveCommandResponse(ChipDeviceController * dc, PacketBuffer * buffer) const
bool ModelCommand::ReceiveCommandResponse(ChipDeviceController * dc, PacketBufferHandle buffer) const
{
EmberApsFrame frame;
uint8_t * message;
Expand All @@ -126,7 +126,6 @@ bool ModelCommand::ReceiveCommandResponse(ChipDeviceController * dc, PacketBuffe
if (extractApsFrame(buffer->Start(), buffer->DataLength(), &frame) == 0)
{
ChipLogError(chipTool, "APS frame processing failure!");
PacketBuffer::Free(buffer);
ExitNow();
}
ChipLogDetail(chipTool, "APS frame processing success!");
Expand Down Expand Up @@ -171,7 +170,7 @@ void ModelCommand::WaitForResponse()
}
}

void ModelCommand::PrintBuffer(PacketBuffer * buffer) const
void ModelCommand::PrintBuffer(const PacketBufferHandle & buffer) const
{
const size_t data_len = buffer->DataLength();

Expand Down
8 changes: 4 additions & 4 deletions examples/chip-tool/commands/common/ModelCommand.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,19 +50,19 @@ class ModelCommand : public NetworkCommand
/////////// IPCommand Interface /////////
void OnConnect(ChipDeviceController * dc) override;
void OnError(ChipDeviceController * dc, CHIP_ERROR err) override;
void OnMessage(ChipDeviceController * dc, chip::System::PacketBuffer * buffer) override;
void OnMessage(ChipDeviceController * dc, chip::System::PacketBufferHandle buffer) override;

virtual uint16_t EncodeCommand(chip::System::PacketBuffer * buffer, uint16_t bufferSize, uint8_t endPointId) = 0;
virtual uint16_t EncodeCommand(const chip::System::PacketBufferHandle & buffer, uint16_t bufferSize, uint8_t endPointId) = 0;
virtual bool HandleGlobalResponse(uint8_t commandId, uint8_t * message, uint16_t messageLen) const { return false; }
virtual bool HandleSpecificResponse(uint8_t commandId, uint8_t * message, uint16_t messageLen) const { return false; }

private:
bool SendCommand(ChipDeviceController * dc);
bool ReceiveCommandResponse(ChipDeviceController * dc, chip::System::PacketBuffer * buffer) const;
bool ReceiveCommandResponse(ChipDeviceController * dc, chip::System::PacketBufferHandle buffer) const;

void UpdateWaitForResponse(bool value);
void WaitForResponse(void);
void PrintBuffer(chip::System::PacketBuffer * buffer) const;
void PrintBuffer(const chip::System::PacketBufferHandle & buffer) const;

std::condition_variable cvWaitingForResponse;
std::mutex cvWaitingForResponseMutex;
Expand Down
6 changes: 2 additions & 4 deletions examples/chip-tool/commands/common/NetworkCommand.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,12 @@ static void onError(ChipDeviceController * dc, void * appReqState, CHIP_ERROR er
command->OnError(dc, err);
}

static void onMessage(ChipDeviceController * dc, void * appReqState, PacketBuffer * buffer)
static void onMessage(ChipDeviceController * dc, void * appReqState, PacketBufferHandle buffer)
{
ChipLogDetail(chipTool, "OnMessage: Received %zu bytes", buffer->DataLength());

NetworkCommand * command = reinterpret_cast<NetworkCommand *>(dc->AppState);
command->OnMessage(dc, buffer);

PacketBuffer::Free(buffer);
command->OnMessage(dc, std::move(buffer));
}

CHIP_ERROR NetworkCommand::Run(ChipDeviceController * dc, NodeId remoteId)
Expand Down
6 changes: 3 additions & 3 deletions examples/chip-tool/commands/common/NetworkCommand.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ class NetworkCommand : public Command

const char * GetNetworkName(void) const { return mName; }

virtual void OnConnect(ChipDeviceController * dc) = 0;
virtual void OnError(ChipDeviceController * dc, CHIP_ERROR err) = 0;
virtual void OnMessage(ChipDeviceController * dc, chip::System::PacketBuffer * buffer) = 0;
virtual void OnConnect(ChipDeviceController * dc) = 0;
virtual void OnError(ChipDeviceController * dc, CHIP_ERROR err) = 0;
virtual void OnMessage(ChipDeviceController * dc, chip::System::PacketBufferHandle buffer) = 0;

CHIP_ERROR Run(ChipDeviceController * dc, NodeId remoteId) override;

Expand Down
5 changes: 1 addition & 4 deletions examples/common/chip-app-server/DataModelHandler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ using namespace ::chip;
* @param [in] buffer The buffer holding the message. This function guarantees
* that it will free the buffer before returning.
*/
void HandleDataModelMessage(const PacketHeader & header, System::PacketBuffer * buffer, SecureSessionMgrBase * mgr)
void HandleDataModelMessage(const PacketHeader & header, System::PacketBufferHandle buffer, SecureSessionMgrBase * mgr)
{
EmberApsFrame frame;
bool ok = extractApsFrame(buffer->Start(), buffer->DataLength(), &frame) > 0;
Expand All @@ -51,7 +51,6 @@ void HandleDataModelMessage(const PacketHeader & header, System::PacketBuffer *
else
{
ChipLogProgress(Zcl, "APS frame processing failure!");
System::PacketBuffer::Free(buffer);
return;
}

Expand All @@ -63,8 +62,6 @@ void HandleDataModelMessage(const PacketHeader & header, System::PacketBuffer *
header.GetSourceNodeId().Value(), // source identifier
NULL);

System::PacketBuffer::Free(buffer);

if (ok)
{
ChipLogProgress(Zcl, "Data model processing success!");
Expand Down
5 changes: 1 addition & 4 deletions examples/common/chip-app-server/RendezvousServer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,7 @@ void RendezvousServer::OnRendezvousConnectionClosed()
ChipLogProgress(AppServer, "OnRendezvousConnectionClosed");
}

void RendezvousServer::OnRendezvousMessageReceived(PacketBuffer * buffer)
{
chip::System::PacketBuffer::Free(buffer);
}
void RendezvousServer::OnRendezvousMessageReceived(System::PacketBufferHandle buffer) {}

void RendezvousServer::OnRendezvousComplete()
{
Expand Down
15 changes: 4 additions & 11 deletions examples/common/chip-app-server/Server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,14 @@ class ServerCallback : public SecureSessionMgrDelegate
{
public:
void OnMessageReceived(const PacketHeader & header, const PayloadHeader & payloadHeader,
const Transport::PeerConnectionState * state, System::PacketBuffer * buffer,
const Transport::PeerConnectionState * state, System::PacketBufferHandle buffer,
SecureSessionMgrBase * mgr) override
{
const size_t data_len = buffer->DataLength();
char src_addr[PeerAddress::kMaxToStringSize];

// as soon as a client connects, assume it is connected
VerifyOrExit(buffer != NULL, ChipLogProgress(AppServer, "Received data but couldn't process it..."));
VerifyOrExit(!buffer.IsNull(), ChipLogProgress(AppServer, "Received data but couldn't process it..."));
VerifyOrExit(header.GetSourceNodeId().HasValue(), ChipLogProgress(AppServer, "Unknown source for received message"));

VerifyOrExit(state->GetPeerNodeId() != kUndefinedNodeId, ChipLogProgress(AppServer, "Unknown source for received message"));
Expand All @@ -62,16 +62,9 @@ class ServerCallback : public SecureSessionMgrDelegate

ChipLogProgress(AppServer, "Packet received from %s: %zu bytes", src_addr, static_cast<size_t>(data_len));

HandleDataModelMessage(header, buffer, mgr);
buffer = NULL;
HandleDataModelMessage(header, std::move(buffer), mgr);

exit:
// HandleDataModelMessage calls Free on the buffer without an AddRef, if HandleDataModelMessage was not called, free the
// buffer.
if (buffer != NULL)
{
System::PacketBuffer::Free(buffer);
}
exit:;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as we increasingly start having empty exit labels, I would propose to move to 'return' rather than 'gotol'. This will probably need to wait for #3880. Not for this PR, just thinking aloud (exit:; looks ugly).

}

void OnNewConnection(const Transport::PeerConnectionState * state, SecureSessionMgrBase * mgr) override
Expand Down
2 changes: 1 addition & 1 deletion examples/common/chip-app-server/include/DataModelHandler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,6 @@
* @param [in] buffer The buffer holding the message. This function guarantees
* that it will free the buffer before returning.
*/
void HandleDataModelMessage(const chip::PacketHeader & header, chip::System::PacketBuffer * buffer,
void HandleDataModelMessage(const chip::PacketHeader & header, chip::System::PacketBufferHandle buffer,
chip::SecureSessionMgrBase * mgr);
void InitDataModelHandler();
2 changes: 1 addition & 1 deletion examples/common/chip-app-server/include/RendezvousServer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class RendezvousServer : public RendezvousSessionDelegate
void OnRendezvousConnectionOpened() override;
void OnRendezvousConnectionClosed() override;
void OnRendezvousError(CHIP_ERROR err) override;
void OnRendezvousMessageReceived(System::PacketBuffer * buffer) override;
void OnRendezvousMessageReceived(System::PacketBufferHandle buffer) override;
void OnRendezvousComplete() override;
void OnRendezvousStatusUpdate(Status status, CHIP_ERROR err) override;

Expand Down
17 changes: 5 additions & 12 deletions examples/temperature-measurement-app/esp32/main/ResponseServer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,14 @@ class ResponseServerCallback : public SecureSessionMgrDelegate
{
public:
void OnMessageReceived(const PacketHeader & header, const PayloadHeader & payloadHeader,
const Transport::PeerConnectionState * state, System::PacketBuffer * buffer,
const Transport::PeerConnectionState * state, System::PacketBufferHandle buffer,
SecureSessionMgrBase * mgr) override
{
CHIP_ERROR err;
const size_t data_len = buffer->DataLength();

// as soon as a client connects, assume it is connected
VerifyOrExit(mgr != nullptr && buffer != nullptr, ESP_LOGE(TAG, "Received data but couldn't process it..."));
VerifyOrExit(mgr != nullptr && !buffer.IsNull(), ESP_LOGE(TAG, "Received data but couldn't process it..."));
VerifyOrExit(state->GetPeerNodeId() != kUndefinedNodeId, ESP_LOGE(TAG, "Unknown source for received message"));

{
Expand All @@ -79,16 +79,9 @@ class ResponseServerCallback : public SecureSessionMgrDelegate
ESP_LOGI(TAG, "Packet received from %s: %zu bytes", src_addr, static_cast<size_t>(data_len));
}

HandleDataModelMessage(header, buffer, mgr);
buffer = nullptr;
HandleDataModelMessage(header, std::move(buffer), mgr);

exit:

// SendTo calls Free on the buffer without an AddRef, if SendTo was not called, free the buffer.
if (buffer != nullptr)
{
System::PacketBuffer::Free(buffer);
}
exit:;
}

void OnReceiveError(CHIP_ERROR error, const Transport::PeerAddress & source, SecureSessionMgrBase * mgr) override
Expand All @@ -110,7 +103,7 @@ class ResponseServerCallback : public SecureSessionMgrDelegate
* Echo messages should generally not have a first byte with those values, so we
* can use that to try to distinguish between the two.
*/
bool ContentMayBeADataModelMessage(System::PacketBuffer * buffer)
bool ContentMayBeADataModelMessage(const System::PacketBufferHandle & buffer)
{
const size_t data_len = buffer->DataLength();
const uint8_t * data = buffer->Start();
Expand Down
Loading