From 16463710238c1f917e86d1fc389a1e914ad07b85 Mon Sep 17 00:00:00 2001 From: Kevin Schoedel <67607049+kpschoedel@users.noreply.github.com> Date: Tue, 1 Dec 2020 14:49:51 -0500 Subject: [PATCH] Some conversions to use PacketBufferHandle (#4011) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Some conversions to use PacketBufferHandle #### Problem Code should use `PacketBufferHandle` rather than `PacketBuffer *`. #### Summary of Changes - Converts remaining receive path in //src/inet and //src/transport. - Converts most of //src/ble. - Introduces Handle versions of the `AddToEnd`/`DetachTail` pair. Part of issue #2707 - Figure out a way to express PacketBuffer ownership in the type system * Restyled by clang-format * review * revive BtpEngine::Clear[TR]xPacket() * simplify conditional * (void) message.DetachTail() → message.FreeHead() * remove ExchangeContext::kSendFlag_RetainBuffer * missed pBuf.IsNull() * DetachHead() → PopTail() * typos Co-authored-by: Restyled.io --- .../esp32/main/EchoServer.cpp | 2 +- .../commands/common/NetworkCommand.cpp | 2 +- src/app/util/chip-message-send.cpp | 2 +- src/ble/BLEEndPoint.cpp | 147 +++++++----------- src/ble/BLEEndPoint.h | 17 +- src/ble/BleLayer.cpp | 13 +- src/ble/BleLayer.h | 5 +- src/ble/BlePlatformDelegate.h | 7 +- src/ble/BtpEngine.cpp | 61 +++----- src/ble/BtpEngine.h | 22 +-- src/controller/CHIPDevice.cpp | 32 +--- src/controller/CHIPDevice.h | 13 -- .../CHIPDeviceController_deprecated.cpp | 6 +- .../CHIPDeviceController_deprecated.h | 2 +- .../java/AndroidBlePlatformDelegate.cpp | 14 +- .../java/AndroidBlePlatformDelegate.h | 6 +- .../java/CHIPDeviceController-JNI.cpp | 4 +- ...ipDeviceController-BlePlatformDelegate.cpp | 24 ++- ...ChipDeviceController-BlePlatformDelegate.h | 6 +- .../CHIP/CHIPDeviceStatusDelegateBridge.h | 2 +- .../CHIP/CHIPDeviceStatusDelegateBridge.mm | 10 +- src/inet/InetLayer.cpp | 9 +- src/inet/RawEndPoint.cpp | 12 +- src/inet/TCPEndPoint.cpp | 52 +++---- src/inet/TCPEndPoint.h | 6 +- src/inet/UDPEndPoint.cpp | 5 +- src/inet/tests/TestInetLayer.cpp | 2 +- src/lib/core/CHIPTLVWriter.cpp | 2 +- src/messaging/ExchangeContext.cpp | 13 +- src/messaging/ExchangeContext.h | 2 +- src/messaging/tests/TestExchangeMgr.cpp | 6 +- src/platform/Darwin/BlePlatformDelegate.h | 8 +- .../Darwin/BlePlatformDelegateImpl.mm | 22 +-- src/platform/EFR32/BLEManagerImpl.cpp | 7 +- src/platform/EFR32/BLEManagerImpl.h | 6 +- src/platform/ESP32/BLEManagerImpl.h | 8 +- .../ESP32/bluedroid/BLEManagerImpl.cpp | 28 ++-- src/platform/ESP32/nimble/BLEManagerImpl.cpp | 7 +- src/platform/K32W/BLEManagerImpl.h | 6 +- src/platform/Linux/BLEManagerImpl.cpp | 8 +- src/platform/Linux/BLEManagerImpl.h | 6 +- src/platform/Linux/CHIPBluezHelper.cpp | 9 +- src/platform/Linux/CHIPBluezHelper.h | 2 +- .../Zephyr/GenericBLEManagerImpl_Zephyr.cpp | 8 +- .../Zephyr/GenericBLEManagerImpl_Zephyr.h | 9 +- src/platform/qpg6100/BLEManagerImpl.cpp | 7 +- src/platform/qpg6100/BLEManagerImpl.h | 6 +- src/protocols/echo/EchoClient.cpp | 2 +- src/protocols/echo/EchoServer.cpp | 2 +- src/system/SystemPacketBuffer.cpp | 23 ++- src/system/SystemPacketBuffer.h | 35 ++++- src/system/tests/TestSystemPacketBuffer.cpp | 8 +- src/transport/BLE.cpp | 2 +- src/transport/NetworkProvisioning.cpp | 6 +- src/transport/NetworkProvisioning.h | 4 +- src/transport/RendezvousSession.cpp | 3 +- src/transport/SecurePairingSession.cpp | 12 +- src/transport/SecurePairingSession.h | 6 +- src/transport/SecureSessionMgr.cpp | 8 +- src/transport/SecureSessionMgr.h | 8 +- src/transport/raw/TCP.cpp | 4 +- src/transport/tests/TestSecureSessionMgr.cpp | 2 +- 62 files changed, 354 insertions(+), 424 deletions(-) diff --git a/examples/all-clusters-app/esp32/main/EchoServer.cpp b/examples/all-clusters-app/esp32/main/EchoServer.cpp index 232de50c0f7cab..566d1bd836c767 100644 --- a/examples/all-clusters-app/esp32/main/EchoServer.cpp +++ b/examples/all-clusters-app/esp32/main/EchoServer.cpp @@ -159,7 +159,7 @@ class EchoServerCallback : public SecureSessionMgrDelegate ESP_LOGI(TAG, "Client sent: %s", logmsg); // Attempt to echo back - err = mgr->SendMessage(header.GetSourceNodeId().Value(), buffer.Release_ForNow()); + err = mgr->SendMessage(header.GetSourceNodeId().Value(), std::move(buffer)); if (err != CHIP_NO_ERROR) { ESP_LOGE(TAG, "Unable to echo back to client: %s", ErrorStr(err)); diff --git a/examples/chip-tool/commands/common/NetworkCommand.cpp b/examples/chip-tool/commands/common/NetworkCommand.cpp index 10b7bb98be6f84..e79cc7f11c8bfb 100644 --- a/examples/chip-tool/commands/common/NetworkCommand.cpp +++ b/examples/chip-tool/commands/common/NetworkCommand.cpp @@ -81,7 +81,7 @@ CHIP_ERROR NetworkCommand::RunCommandInternal(ChipDevice * device) PrintBuffer(buffer); #endif - err = device->SendMessage(buffer.Release_ForNow()); + err = device->SendMessage(std::move(buffer)); VerifyOrExit(err == CHIP_NO_ERROR, ChipLogError(chipTool, "Failed to send message: %s", ErrorStr(err))); exit: diff --git a/src/app/util/chip-message-send.cpp b/src/app/util/chip-message-send.cpp index c0e3e5d15d0fb8..12aa752106021b 100644 --- a/src/app/util/chip-message-send.cpp +++ b/src/app/util/chip-message-send.cpp @@ -73,7 +73,7 @@ EmberStatus chipSendUnicast(NodeId destination, EmberApsFrame * apsFrame, uint16 memcpy(buffer->Start() + frameSize, message, messageLength); buffer->SetDataLength(dataLength); - CHIP_ERROR err = SessionManager().SendMessage(destination, buffer.Release_ForNow()); + CHIP_ERROR err = SessionManager().SendMessage(destination, std::move(buffer)); if (err != CHIP_NO_ERROR) { // FIXME: Figure out better translations between our error types? diff --git a/src/ble/BLEEndPoint.cpp b/src/ble/BLEEndPoint.cpp index a72485f1d3a66c..6d1b01317bf7eb 100644 --- a/src/ble/BLEEndPoint.cpp +++ b/src/ble/BLEEndPoint.cpp @@ -140,7 +140,11 @@ BLE_ERROR BLEEndPoint::StartConnect() SuccessOrExit(err); // Send BLE transport capabilities request to peripheral via GATT write. - if (!SendWrite(buf.Get_ForNow())) + // Add reference to message fragment for duration of platform's GATT write attempt. CHIP retains partial + // ownership of message fragment's PacketBuffer, since this is the same buffer as that of the whole message, just + // with a fragmenter-modified payload offset and data length, by a Retain() on the handle when calling this function. + // Buffer must be decref'd (i.e. PacketBuffer::Free'd) by platform when BLE GATT operation completes. + if (!SendWrite(buf.Retain())) { err = BLE_ERROR_GATT_WRITE_FAILED; ExitNow(); @@ -148,7 +152,7 @@ BLE_ERROR BLEEndPoint::StartConnect() // Free request buffer on write confirmation. Stash a reference to it in mSendQueue, which we don't use anyway // until the connection has been set up. - QueueTx(buf.Release_ForNow(), kType_Data); + QueueTx(std::move(buf), kType_Data); exit: // If we failed to initiate the connection, close the end point. @@ -214,17 +218,20 @@ void BLEEndPoint::HandleSubscribeReceived() BLE_ERROR err = BLE_NO_ERROR; VerifyOrExit(mState == kState_Connecting || mState == kState_Aborting, err = BLE_ERROR_INCORRECT_STATE); - VerifyOrExit(mSendQueue != nullptr, err = BLE_ERROR_INCORRECT_STATE); + VerifyOrExit(!mSendQueue.IsNull(), err = BLE_ERROR_INCORRECT_STATE); // Send BTP capabilities response to peripheral via GATT indication. #if CHIP_ENABLE_CHIPOBLE_TEST VerifyOrExit(mBtpEngine.PopPacketTag(mSendQueue) == kType_Data, err = BLE_ERROR_INVALID_BTP_HEADER_FLAGS); #endif - if (!SendIndication(mSendQueue)) + // Add reference to message fragment for duration of platform's GATT indication attempt. CHIP retains partial + // ownership of message fragment's PacketBuffer, since this is the same buffer as that of the whole message, just + // with a fragmenter-modified payload offset and data length. Buffer must be decref'd (i.e. PacketBuffer::Free'd) by + // platform when BLE GATT operation completes. + if (!SendIndication(mSendQueue.Retain())) { // Ensure transmit queue is empty and set to NULL. QueueTxLock(); - PacketBuffer::Free(mSendQueue); mSendQueue = nullptr; QueueTxUnlock(); @@ -371,7 +378,6 @@ void BLEEndPoint::FinalizeClose(uint8_t oldState, uint8_t flags, BLE_ERROR err) // Ensure transmit queue is empty and set to NULL. QueueTxLock(); - PacketBuffer::Free(mSendQueue); mSendQueue = nullptr; QueueTxUnlock(); @@ -485,7 +491,7 @@ void BLEEndPoint::Free() FreeBtpEngine(); // Clear pending ack buffer, if any. - PacketBuffer::Free(mAckToSend); + mAckToSend = nullptr; // Cancel all timers. StopConnectTimer(); @@ -513,17 +519,11 @@ void BLEEndPoint::Free() void BLEEndPoint::FreeBtpEngine() { - PacketBuffer * buf; - // Free transmit disassembly buffer - buf = mBtpEngine.TxPacket(); mBtpEngine.ClearTxPacket(); - PacketBuffer::Free(buf); // Free receive reassembly buffer - buf = mBtpEngine.RxPacket(); mBtpEngine.ClearRxPacket(); - PacketBuffer::Free(buf); } BLE_ERROR BLEEndPoint::Init(BleLayer * bleLayer, BLE_CONNECTION_OBJECT connObj, BleRole role, bool autoClose) @@ -539,12 +539,6 @@ BLE_ERROR BLEEndPoint::Init(BleLayer * bleLayer, BLE_CONNECTION_OBJECT connObj, VerifyOrExit(connObj != BLE_CONNECTION_UNINITIALIZED, err = BLE_ERROR_BAD_ARGS); VerifyOrExit((role == kBleRole_Central || role == kBleRole_Peripheral), err = BLE_ERROR_BAD_ARGS); - // Null-initialize callbacks and data members. - // - // Beware this line should we ever use virtuals in this class or its - // super(s). See similar lines in chip::System::Layer end points. - memset(this, 0, sizeof(*this)); - // If end point plays peripheral role, expect ack for indication sent as last step of BTP handshake. // If central, periperal's handshake indication 'ack's write sent by central to kick off the BTP handshake. expectInitialAck = (role == kBleRole_Peripheral); @@ -596,13 +590,13 @@ BLE_ERROR BLEEndPoint::Init(BleLayer * bleLayer, BLE_CONNECTION_OBJECT connObj, return err; } -BLE_ERROR BLEEndPoint::SendCharacteristic(PacketBuffer * buf) +BLE_ERROR BLEEndPoint::SendCharacteristic(PacketBufferHandle buf) { BLE_ERROR err = BLE_NO_ERROR; if (mRole == kBleRole_Central) { - if (!SendWrite(buf)) + if (!SendWrite(std::move(buf))) { err = BLE_ERROR_GATT_WRITE_FAILED; } @@ -615,7 +609,7 @@ BLE_ERROR BLEEndPoint::SendCharacteristic(PacketBuffer * buf) } else // (mRole == kBleRole_Peripheral), verified on Init { - if (!SendIndication(buf)) + if (!SendIndication(std::move(buf))) { err = BLE_ERROR_GATT_INDICATE_FAILED; } @@ -635,7 +629,7 @@ BLE_ERROR BLEEndPoint::SendCharacteristic(PacketBuffer * buf) * kType_Data(0) - data packet * kType_Control(1) - control packet */ -void BLEEndPoint::QueueTx(PacketBuffer * data, PacketType_t type) +void BLEEndPoint::QueueTx(PacketBufferHandle data, PacketType_t type) { #if CHIP_ENABLE_CHIPOBLE_TEST ChipLogDebugBleEndPoint(Ble, "%s: data->%p, type %d, len %d", __FUNCTION__, data, type, data->DataLength()); @@ -644,27 +638,27 @@ void BLEEndPoint::QueueTx(PacketBuffer * data, PacketType_t type) QueueTxLock(); - if (mSendQueue == nullptr) + if (mSendQueue.IsNull()) { - mSendQueue = data; + mSendQueue = std::move(data); ChipLogDebugBleEndPoint(Ble, "%s: Set data as new mSendQueue %p, type %d", __FUNCTION__, mSendQueue, type); } else { - mSendQueue->AddToEnd(data); + mSendQueue->AddToEnd(std::move(data)); ChipLogDebugBleEndPoint(Ble, "%s: Append data to mSendQueue %p, type %d", __FUNCTION__, mSendQueue, type); } QueueTxUnlock(); } -BLE_ERROR BLEEndPoint::Send(PacketBuffer * data) +BLE_ERROR BLEEndPoint::Send(PacketBufferHandle data) { ChipLogDebugBleEndPoint(Ble, "entered Send"); BLE_ERROR err = BLE_NO_ERROR; - VerifyOrExit(data != nullptr, err = BLE_ERROR_BAD_ARGS); + VerifyOrExit(!data.IsNull(), err = BLE_ERROR_BAD_ARGS); VerifyOrExit(IsConnected(mState), err = BLE_ERROR_INCORRECT_STATE); // Ensure outgoing message fits in a single contiguous PacketBuffer, as currently required by the @@ -681,8 +675,7 @@ BLE_ERROR BLEEndPoint::Send(PacketBuffer * data) } // Add new message to send queue. - QueueTx(data, kType_Data); - data = nullptr; // Buffer freed when send queue freed on close, or on completion of current message transmission. + QueueTx(std::move(data), kType_Data); // Send first fragment of new message, if we can. err = DriveSending(); @@ -690,12 +683,6 @@ BLE_ERROR BLEEndPoint::Send(PacketBuffer * data) exit: ChipLogDebugBleEndPoint(Ble, "exiting Send"); - - if (data != nullptr) - { - PacketBuffer::Free(data); - } - if (err != BLE_NO_ERROR) { DoClose(kBleCloseFlag_AbortTransmission, err); @@ -704,7 +691,7 @@ BLE_ERROR BLEEndPoint::Send(PacketBuffer * data) return err; } -bool BLEEndPoint::PrepareNextFragment(PacketBuffer * data, bool & sentAck) +bool BLEEndPoint::PrepareNextFragment(PacketBufferHandle data, bool & sentAck) { // If we have a pending fragment acknowledgement to send, piggyback it on the fragment we're about to transmit. if (GetFlag(mTimerStateFlags, kTimerState_SendAckTimerRunning)) @@ -722,9 +709,7 @@ bool BLEEndPoint::PrepareNextFragment(PacketBuffer * data, bool & sentAck) sentAck = false; } - System::PacketBufferHandle data_ForNow; - data_ForNow.Adopt(data); - return mBtpEngine.HandleCharacteristicSend(std::move(data_ForNow), sentAck); + return mBtpEngine.HandleCharacteristicSend(std::move(data), sentAck); } BLE_ERROR BLEEndPoint::SendNextMessage() @@ -736,16 +721,15 @@ BLE_ERROR BLEEndPoint::SendNextMessage() QueueTxLock(); #if CHIP_ENABLE_CHIPOBLE_TEST // Return if tx queue is empty - // Note: DetachTail() does not check an empty queue - if (mSendQueue == NULL) + // Note: PopHead() does not check an empty queue + if (mSendQueue.IsNull()) { QueueTxUnlock(); return err; } #endif - PacketBuffer * data = mSendQueue; - mSendQueue = mSendQueue->DetachTail(); + PacketBufferHandle data = mSendQueue.PopHead(); QueueTxUnlock(); #if CHIP_ENABLE_CHIPOBLE_TEST @@ -756,8 +740,7 @@ BLE_ERROR BLEEndPoint::SendNextMessage() #endif // Hand whole message payload to the fragmenter. - VerifyOrExit(PrepareNextFragment(data, sentAck), err = BLE_ERROR_CHIPOBLE_PROTOCOL_ABORT); - data = nullptr; // Ownership passed to fragmenter's tx buf on PrepareNextFragment success. + VerifyOrExit(PrepareNextFragment(std::move(data), sentAck), err = BLE_ERROR_CHIPOBLE_PROTOCOL_ABORT); /* // Todo: reenabled it after integrating fault injection @@ -774,7 +757,7 @@ BLE_ERROR BLEEndPoint::SendNextMessage() ExitNow(); }); */ - err = SendCharacteristic(mBtpEngine.TxPacket()); + err = SendCharacteristic(mBtpEngine.BorrowTxPacket()); SuccessOrExit(err); if (sentAck) @@ -788,11 +771,6 @@ BLE_ERROR BLEEndPoint::SendNextMessage() SuccessOrExit(err); exit: - if (data != nullptr) - { - PacketBuffer::Free(data); - } - return err; } @@ -811,7 +789,7 @@ BLE_ERROR BLEEndPoint::ContinueMessageSend() ExitNow(); } - err = SendCharacteristic(mBtpEngine.TxPacket()); + err = SendCharacteristic(mBtpEngine.BorrowTxPacket()); SuccessOrExit(err); if (sentAck) @@ -837,7 +815,7 @@ BLE_ERROR BLEEndPoint::HandleHandshakeConfirmationReceived() // Free capabilities request/response payload. QueueTxLock(); - mSendQueue = PacketBuffer::FreeHead(mSendQueue); + mSendQueue.FreeHead(); QueueTxUnlock(); if (mRole == kBleRole_Central) @@ -861,7 +839,7 @@ BLE_ERROR BLEEndPoint::HandleHandshakeConfirmationReceived() { // If local receive window size has shrunk to or below immediate ack threshold, AND a message fragment is not // pending on which to piggyback an ack, send immediate stand-alone ack. - if (mLocalReceiveWindowSize <= BLE_CONFIG_IMMEDIATE_ACK_WINDOW_THRESHOLD && mSendQueue == nullptr) + if (mLocalReceiveWindowSize <= BLE_CONFIG_IMMEDIATE_ACK_WINDOW_THRESHOLD && mSendQueue.IsNull()) { err = DriveStandAloneAck(); // Encode stand-alone ack and drive sending. SuccessOrExit(err); @@ -915,7 +893,6 @@ BLE_ERROR BLEEndPoint::HandleFragmentConfirmationReceived() if (GetFlag(mConnStateFlags, kConnState_StandAloneAckInFlight)) { // If confirmation was received for stand-alone ack, free its tx buffer. - PacketBuffer::Free(mAckToSend); mAckToSend = nullptr; SetFlag(mConnStateFlags, kConnState_StandAloneAckInFlight, false); @@ -927,8 +904,8 @@ BLE_ERROR BLEEndPoint::HandleFragmentConfirmationReceived() // This check covers the case where the local receive window has shrunk between transmission and confirmation of // the stand-alone ack, and also the case where a window size < the immediate ack threshold was detected in // Receive(), but the stand-alone ack was deferred due to a pending outbound message fragment. - if (mLocalReceiveWindowSize <= BLE_CONFIG_IMMEDIATE_ACK_WINDOW_THRESHOLD && - !(mSendQueue != nullptr || mBtpEngine.TxState() == BtpEngine::kState_InProgress)) + if (mLocalReceiveWindowSize <= BLE_CONFIG_IMMEDIATE_ACK_WINDOW_THRESHOLD && mSendQueue.IsNull() && + mBtpEngine.TxState() != BtpEngine::kState_InProgress) { err = DriveStandAloneAck(); // Encode stand-alone ack and drive sending. SuccessOrExit(err); @@ -974,10 +951,10 @@ BLE_ERROR BLEEndPoint::DriveStandAloneAck() StopSendAckTimer(); // If stand-alone ack not already pending, allocate new payload buffer here. - if (mAckToSend == nullptr) + if (mAckToSend.IsNull()) { - mAckToSend = PacketBuffer::New().Release_ForNow(); - VerifyOrExit(mAckToSend != nullptr, err = BLE_ERROR_NO_MEMORY); + mAckToSend = PacketBuffer::New(); + VerifyOrExit(!mAckToSend.IsNull(), err = BLE_ERROR_NO_MEMORY); } // Attempt to send stand-alone ack. @@ -994,7 +971,7 @@ BLE_ERROR BLEEndPoint::DoSendStandAloneAck() // Encode and transmit stand-alone ack. mBtpEngine.EncodeStandAloneAck(mAckToSend); - BLE_ERROR err = SendCharacteristic(mAckToSend); + BLE_ERROR err = SendCharacteristic(mAckToSend.Retain()); SuccessOrExit(err); // Reset local receive window counter. @@ -1020,7 +997,7 @@ BLE_ERROR BLEEndPoint::DriveSending() // If receiver's window is almost closed and we don't have an ack to send, OR we do have an ack to send but // receiver's window is completely empty, OR another GATT operation is in flight, awaiting confirmation... if ((mRemoteReceiveWindowSize <= BTP_WINDOW_NO_ACK_SEND_THRESHOLD && - !GetFlag(mTimerStateFlags, kTimerState_SendAckTimerRunning) && mAckToSend == nullptr) || + !GetFlag(mTimerStateFlags, kTimerState_SendAckTimerRunning) && mAckToSend.IsNull()) || (mRemoteReceiveWindowSize == 0) || (GetFlag(mConnStateFlags, kConnState_GattOperationInFlight))) { #ifdef CHIP_BLE_END_POINT_DEBUG_LOGGING_ENABLED @@ -1047,7 +1024,7 @@ BLE_ERROR BLEEndPoint::DriveSending() // Otherwise, let's see what we can send. - if (mAckToSend != nullptr) // If immediate, stand-alone ack is pending, send it. + if (!mAckToSend.IsNull()) // If immediate, stand-alone ack is pending, send it. { err = DoSendStandAloneAck(); SuccessOrExit(err); @@ -1055,7 +1032,7 @@ BLE_ERROR BLEEndPoint::DriveSending() else if (mBtpEngine.TxState() == BtpEngine::kState_Idle) // Else send next message fragment, if any. { // Fragmenter's idle, let's see what's in the send queue... - if (mSendQueue != nullptr) + if (!mSendQueue.IsNull()) { // Transmit first fragment of next whole message in send queue. err = SendNextMessage(); @@ -1075,17 +1052,14 @@ BLE_ERROR BLEEndPoint::DriveSending() else if (mBtpEngine.TxState() == BtpEngine::kState_Complete) { // Clear fragmenter's pointer to sent message buffer and reset its Tx state. - PacketBuffer * sentBuf = mBtpEngine.TxPacket(); + // Buffer will be freed at scope exit. + PacketBufferHandle sentBuf = mBtpEngine.TakeTxPacket(); #if CHIP_ENABLE_CHIPOBLE_TEST mBtpEngineTest.DoTxTiming(sentBuf, BTP_TX_DONE); #endif // CHIP_ENABLE_CHIPOBLE_TEST mBtpEngine.ClearTxPacket(); - // Free sent buffer. - PacketBuffer::Free(sentBuf); - sentBuf = nullptr; - - if (mSendQueue != nullptr) + if (!mSendQueue.IsNull()) { // Transmit first fragment of next whole message in send queue. err = SendNextMessage(); @@ -1119,7 +1093,7 @@ BLE_ERROR BLEEndPoint::HandleCapabilitiesRequestReceived(PacketBufferHandle data mState = kState_Connecting; // Decode BTP capabilities request. - err = BleTransportCapabilitiesRequestMessage::Decode((*data), req); + err = BleTransportCapabilitiesRequestMessage::Decode(data, req); SuccessOrExit(err); responseBuf = PacketBuffer::New(); @@ -1185,7 +1159,7 @@ BLE_ERROR BLEEndPoint::HandleCapabilitiesRequestReceived(PacketBufferHandle data SuccessOrExit(err); // Stash capabilities response payload and wait for subscription from central. - QueueTx(responseBuf.Release_ForNow(), kType_Data); + QueueTx(std::move(responseBuf), kType_Data); // Start receive timer. Canceled when end point freed or connection established. err = StartReceiveConnectionTimer(); @@ -1203,7 +1177,7 @@ BLE_ERROR BLEEndPoint::HandleCapabilitiesResponseReceived(PacketBufferHandle dat VerifyOrExit(!data.IsNull(), err = BLE_ERROR_BAD_ARGS); // Decode BTP capabilities response. - err = BleTransportCapabilitiesResponseMessage::Decode((*data), resp); + err = BleTransportCapabilitiesResponseMessage::Decode(data, resp); SuccessOrExit(err); VerifyOrExit(resp.mFragmentSize > 0, err = BLE_ERROR_INVALID_FRAGMENT_SIZE); @@ -1380,7 +1354,7 @@ BLE_ERROR BLEEndPoint::Receive(PacketBufferHandle data) ChipLogDebugBleEndPoint(Ble, "got ack for last outstanding fragment"); StopAckReceivedTimer(); - if (mState == kState_Closing && mSendQueue == nullptr && mBtpEngine.TxState() == BtpEngine::kState_Idle) + if (mState == kState_Closing && mSendQueue.IsNull() && mBtpEngine.TxState() == BtpEngine::kState_Idle) { // If end point closing, got confirmation for last send, and waiting for last ack, finalize close. FinalizeClose(mState, kBleCloseFlag_SuppressCallback, BLE_NO_ERROR); @@ -1446,8 +1420,7 @@ BLE_ERROR BLEEndPoint::Receive(PacketBufferHandle data) if (mBtpEngine.RxState() == BtpEngine::kState_Complete) { // Take ownership of message PacketBuffer - System::PacketBufferHandle full_packet; - full_packet.Adopt(mBtpEngine.RxPacket()); + System::PacketBufferHandle full_packet = mBtpEngine.TakeRxPacket(); mBtpEngine.ClearRxPacket(); ChipLogDebugBleEndPoint(Ble, "reassembled whole msg, len = %d", full_packet->DataLength()); @@ -1481,30 +1454,18 @@ BLE_ERROR BLEEndPoint::Receive(PacketBufferHandle data) return err; } -bool BLEEndPoint::SendWrite(PacketBuffer * buf) +bool BLEEndPoint::SendWrite(PacketBufferHandle buf) { - // Add reference to message fragment for duration of platform's GATT write attempt. CHIP retains partial - // ownership of message fragment's PacketBuffer, since this is the same buffer as that of the whole message, just - // with a fragmenter-modified payload offset and data length. Buffer must be decref'd (i.e. PacketBuffer::Free'd) by - // platform when BLE GATT operation completes. - buf->AddRef(); - SetFlag(mConnStateFlags, kConnState_GattOperationInFlight, true); - return mBle->mPlatformDelegate->SendWriteRequest(mConnObj, &CHIP_BLE_SVC_ID, &mBle->CHIP_BLE_CHAR_1_ID, buf); + return mBle->mPlatformDelegate->SendWriteRequest(mConnObj, &CHIP_BLE_SVC_ID, &mBle->CHIP_BLE_CHAR_1_ID, std::move(buf)); } -bool BLEEndPoint::SendIndication(PacketBuffer * buf) +bool BLEEndPoint::SendIndication(PacketBufferHandle buf) { - // Add reference to message fragment for duration of platform's GATT indication attempt. CHIP retains partial - // ownership of message fragment's PacketBuffer, since this is the same buffer as that of the whole message, just - // with a fragmenter-modified payload offset and data length. Buffer must be decref'd (i.e. PacketBuffer::Free'd) by - // platform when BLE GATT operation completes. - buf->AddRef(); - SetFlag(mConnStateFlags, kConnState_GattOperationInFlight, true); - return mBle->mPlatformDelegate->SendIndication(mConnObj, &CHIP_BLE_SVC_ID, &mBle->CHIP_BLE_CHAR_2_ID, buf); + return mBle->mPlatformDelegate->SendIndication(mConnObj, &CHIP_BLE_SVC_ID, &mBle->CHIP_BLE_CHAR_2_ID, std::move(buf)); } BLE_ERROR BLEEndPoint::StartConnectTimer() diff --git a/src/ble/BLEEndPoint.h b/src/ble/BLEEndPoint.h index 7e3865468adb24..f133559fd62864 100644 --- a/src/ble/BLEEndPoint.h +++ b/src/ble/BLEEndPoint.h @@ -38,7 +38,6 @@ namespace chip { namespace Ble { -using ::chip::System::PacketBuffer; using ::chip::System::PacketBufferHandle; enum @@ -97,7 +96,7 @@ class DLL_EXPORT BLEEndPoint : public BleLayerObject #endif // Public functions: - BLE_ERROR Send(PacketBuffer * data); + BLE_ERROR Send(PacketBufferHandle data); BLE_ERROR Receive(PacketBufferHandle data); BLE_ERROR StartConnect(); @@ -139,11 +138,11 @@ class DLL_EXPORT BLEEndPoint : public BleLayerObject // // Re-used during connection setup to cache capabilities request and response payloads; payloads are freed when // connection is established. - PacketBuffer * mSendQueue; + PacketBufferHandle mSendQueue; // Pending stand-alone BTP acknolwedgement. Pre-empts regular send queue or fragmented message transmission in // progress. - PacketBuffer * mAckToSend; + PacketBufferHandle mAckToSend; BtpEngine mBtpEngine; BleRole mRole; @@ -167,13 +166,13 @@ class DLL_EXPORT BLEEndPoint : public BleLayerObject // Transmit path: BLE_ERROR DriveSending(); BLE_ERROR DriveStandAloneAck(); - bool PrepareNextFragment(PacketBuffer * data, bool & sentAck); + bool PrepareNextFragment(PacketBufferHandle data, bool & sentAck); BLE_ERROR SendNextMessage(); BLE_ERROR ContinueMessageSend(); BLE_ERROR DoSendStandAloneAck(); - BLE_ERROR SendCharacteristic(PacketBuffer * buf); - bool SendIndication(PacketBuffer * buf); - bool SendWrite(PacketBuffer * buf); + BLE_ERROR SendCharacteristic(PacketBufferHandle buf); + bool SendIndication(PacketBufferHandle buf); + bool SendWrite(PacketBufferHandle buf); // Receive path: BLE_ERROR HandleConnectComplete(); @@ -224,7 +223,7 @@ class DLL_EXPORT BLEEndPoint : public BleLayerObject inline void QueueTxLock() {} inline void QueueTxUnlock() {} #endif - void QueueTx(PacketBuffer * data, PacketType_t type); + void QueueTx(PacketBufferHandle data, PacketType_t type); }; } /* namespace Ble */ diff --git a/src/ble/BleLayer.cpp b/src/ble/BleLayer.cpp index f25a0a29061bd0..cebf1909092912 100644 --- a/src/ble/BleLayer.cpp +++ b/src/ble/BleLayer.cpp @@ -222,13 +222,14 @@ BLE_ERROR BleTransportCapabilitiesRequestMessage::Encode(const PacketBufferHandl return err; } -BLE_ERROR BleTransportCapabilitiesRequestMessage::Decode(const PacketBuffer & msgBuf, BleTransportCapabilitiesRequestMessage & msg) +BLE_ERROR BleTransportCapabilitiesRequestMessage::Decode(const PacketBufferHandle & msgBuf, + BleTransportCapabilitiesRequestMessage & msg) { - const uint8_t * p = msgBuf.Start(); + const uint8_t * p = msgBuf->Start(); BLE_ERROR err = BLE_NO_ERROR; // Verify we can read the fixed-length request without running into the end of the buffer. - VerifyOrExit(msgBuf.DataLength() >= CAPABILITIES_REQUEST_LEN, err = BLE_ERROR_MESSAGE_INCOMPLETE); + VerifyOrExit(msgBuf->DataLength() >= CAPABILITIES_REQUEST_LEN, err = BLE_ERROR_MESSAGE_INCOMPLETE); VerifyOrExit(CAPABILITIES_MSG_CHECK_BYTE_1 == chip::Encoding::Read8(p), err = BLE_ERROR_INVALID_MESSAGE); VerifyOrExit(CAPABILITIES_MSG_CHECK_BYTE_2 == chip::Encoding::Read8(p), err = BLE_ERROR_INVALID_MESSAGE); @@ -268,14 +269,14 @@ BLE_ERROR BleTransportCapabilitiesResponseMessage::Encode(const PacketBufferHand return err; } -BLE_ERROR BleTransportCapabilitiesResponseMessage::Decode(const PacketBuffer & msgBuf, +BLE_ERROR BleTransportCapabilitiesResponseMessage::Decode(const PacketBufferHandle & msgBuf, BleTransportCapabilitiesResponseMessage & msg) { - const uint8_t * p = msgBuf.Start(); + const uint8_t * p = msgBuf->Start(); BLE_ERROR err = BLE_NO_ERROR; // Verify we can read the fixed-length response without running into the end of the buffer. - VerifyOrExit(msgBuf.DataLength() >= CAPABILITIES_RESPONSE_LEN, err = BLE_ERROR_MESSAGE_INCOMPLETE); + VerifyOrExit(msgBuf->DataLength() >= CAPABILITIES_RESPONSE_LEN, err = BLE_ERROR_MESSAGE_INCOMPLETE); VerifyOrExit(CAPABILITIES_MSG_CHECK_BYTE_1 == chip::Encoding::Read8(p), err = BLE_ERROR_INVALID_MESSAGE); VerifyOrExit(CAPABILITIES_MSG_CHECK_BYTE_2 == chip::Encoding::Read8(p), err = BLE_ERROR_INVALID_MESSAGE); diff --git a/src/ble/BleLayer.h b/src/ble/BleLayer.h index b1ddd0af11abe0..6eeb85820027f1 100644 --- a/src/ble/BleLayer.h +++ b/src/ble/BleLayer.h @@ -156,7 +156,7 @@ class BleTransportCapabilitiesRequestMessage /// Must be able to reserve 20 byte data length in msgBuf. BLE_ERROR Encode(const System::PacketBufferHandle & msgBuf) const; - static BLE_ERROR Decode(const PacketBuffer & msgBuf, BleTransportCapabilitiesRequestMessage & msg); + static BLE_ERROR Decode(const System::PacketBufferHandle & msgBuf, BleTransportCapabilitiesRequestMessage & msg); }; class BleTransportCapabilitiesResponseMessage @@ -190,7 +190,7 @@ class BleTransportCapabilitiesResponseMessage /// Must be able to reserve 20 byte data length in msgBuf. BLE_ERROR Encode(const System::PacketBufferHandle & msgBuf) const; - static BLE_ERROR Decode(const PacketBuffer & msgBuf, BleTransportCapabilitiesResponseMessage & msg); + static BLE_ERROR Decode(const System::PacketBufferHandle & msgBuf, BleTransportCapabilitiesResponseMessage & msg); }; /** @@ -343,7 +343,6 @@ class DLL_EXPORT BleLayer chip::System::Layer * mSystemLayer; // Private functions: - void HandleDataReceived(BLE_CONNECTION_OBJECT connObj, PacketBuffer * pBuf); void HandleAckReceived(BLE_CONNECTION_OBJECT connObj); void DriveSending(); BLE_ERROR HandleBleTransportConnectionInitiated(BLE_CONNECTION_OBJECT connObj, System::PacketBufferHandle pBuf); diff --git a/src/ble/BlePlatformDelegate.h b/src/ble/BlePlatformDelegate.h index 91790823d6be9b..f166e1d2835196 100644 --- a/src/ble/BlePlatformDelegate.h +++ b/src/ble/BlePlatformDelegate.h @@ -35,6 +35,7 @@ namespace chip { namespace Ble { using ::chip::System::PacketBuffer; +using ::chip::System::PacketBufferHandle; // Platform-agnostic BLE interface class DLL_EXPORT BlePlatformDelegate @@ -77,15 +78,15 @@ class DLL_EXPORT BlePlatformDelegate // Send GATT characteristic indication request virtual bool SendIndication(BLE_CONNECTION_OBJECT connObj, const ChipBleUUID * svcId, const ChipBleUUID * charId, - PacketBuffer * pBuf) = 0; + PacketBufferHandle pBuf) = 0; // Send GATT characteristic write request virtual bool SendWriteRequest(BLE_CONNECTION_OBJECT connObj, const ChipBleUUID * svcId, const ChipBleUUID * charId, - PacketBuffer * pBuf) = 0; + PacketBufferHandle pBuf) = 0; // Send GATT characteristic read request virtual bool SendReadRequest(BLE_CONNECTION_OBJECT connObj, const ChipBleUUID * svcId, const ChipBleUUID * charId, - PacketBuffer * pBuf) = 0; + PacketBufferHandle pBuf) = 0; // Send response to remote host's GATT chacteristic read response virtual bool SendReadResponse(BLE_CONNECTION_OBJECT connObj, BLE_READ_REQUEST_CONTEXT requestContext, const ChipBleUUID * svcId, diff --git a/src/ble/BtpEngine.cpp b/src/ble/BtpEngine.cpp index 6530deb6e33c42..976ec6b99457db 100644 --- a/src/ble/BtpEngine.cpp +++ b/src/ble/BtpEngine.cpp @@ -77,7 +77,7 @@ static inline bool DidReceiveData(uint8_t rx_flags) GetFlag(rx_flags, BtpEngine::kHeaderFlag_EndMessage)); } -static void PrintBufDebug(const System::PacketBuffer * buf) +static void PrintBufDebug(const System::PacketBufferHandle & buf) { #ifdef CHIP_BTP_PROTOCOL_ENGINE_DEBUG_LOGGING_ENABLED uint8_t * b = buf->Start(); @@ -216,7 +216,7 @@ BLE_ERROR BtpEngine::HandleAckReceived(SequenceNumber_t ack_num) // Calling convention: // EncodeStandAloneAck may only be called if data arg is commited for immediate, synchronous subsequent transmission. -BLE_ERROR BtpEngine::EncodeStandAloneAck(PacketBuffer * data) +BLE_ERROR BtpEngine::EncodeStandAloneAck(const PacketBufferHandle & data) { BLE_ERROR err = BLE_NO_ERROR; uint8_t * characteristic; @@ -310,7 +310,7 @@ BLE_ERROR BtpEngine::HandleCharacteristicReceived(System::PacketBufferHandle dat data->SetDataLength(chip::min(data->DataLength(), mRxFragmentSize)); ChipLogDebugBtpEngine(Ble, ">>> BTP reassembler received data:"); - PrintBufDebug(data.Get_ForNow()); + PrintBufDebug(data); if (mRxState == kState_Idle) { @@ -325,11 +325,11 @@ BLE_ERROR BtpEngine::HandleCharacteristicReceived(System::PacketBufferHandle dat data->SetStart(&(characteristic[cursor])); // Create a new buffer for use as the Rx re-assembly area. - mRxBuf = PacketBuffer::New().Release_ForNow(); + mRxBuf = PacketBuffer::New(); - VerifyOrExit(mRxBuf != nullptr, err = BLE_ERROR_NO_MEMORY); + VerifyOrExit(!mRxBuf.IsNull(), err = BLE_ERROR_NO_MEMORY); - mRxBuf->AddToEnd(data.Release_ForNow()); + mRxBuf->AddToEnd(std::move(data)); mRxBuf->CompactHead(); // will free 'data' and adjust rx buf's end/length } else if (mRxState == kState_InProgress) @@ -343,7 +343,7 @@ BLE_ERROR BtpEngine::HandleCharacteristicReceived(System::PacketBufferHandle dat // Add received fragment to reassembled message buffer. data->SetStart(&(characteristic[cursor])); - mRxBuf->AddToEnd(data.Release_ForNow()); + mRxBuf->AddToEnd(std::move(data)); mRxBuf->CompactHead(); // will free 'data' and adjust rx buf's end/length // For now, limit BtpEngine message size to max length of 1 pbuf, as we do for chip messages sent via IP. @@ -385,7 +385,7 @@ BLE_ERROR BtpEngine::HandleCharacteristicReceived(System::PacketBufferHandle dat { ChipLogError(Ble, "With rx'd ack = %u", receivedAck); } - if (mRxBuf != nullptr) + if (!mRxBuf.IsNull()) { ChipLogError(Ble, "With rx buf data length = %u", mRxBuf->DataLength()); } @@ -394,13 +394,13 @@ BLE_ERROR BtpEngine::HandleCharacteristicReceived(System::PacketBufferHandle dat if (!data.IsNull()) { // Tack received data onto rx buffer, to be freed when end point resets protocol engine on close. - if (mRxBuf != nullptr) + if (!mRxBuf.IsNull()) { - mRxBuf->AddToEnd(data.Release_ForNow()); + mRxBuf->AddToEnd(std::move(data)); } else { - mRxBuf = data.Release_ForNow(); + mRxBuf = std::move(data); } } } @@ -408,30 +408,20 @@ BLE_ERROR BtpEngine::HandleCharacteristicReceived(System::PacketBufferHandle dat return err; } -PacketBuffer * BtpEngine::RxPacket() -{ - return mRxBuf; -} - -bool BtpEngine::ClearRxPacket() +void BtpEngine::ClearRxPacket() { if (mRxState == kState_Complete) { mRxState = kState_Idle; - mRxBuf = nullptr; - // do not reset mRxNextSeqNum - return true; } - - return false; + mRxBuf = nullptr; } // Calling convention: // May only be called if data arg is commited for immediate, synchronous subsequent transmission. // Returns false on error. Caller must free data arg on error. -bool BtpEngine::HandleCharacteristicSend(System::PacketBufferHandle data_ForNow, bool send_ack) +bool BtpEngine::HandleCharacteristicSend(System::PacketBufferHandle data, bool send_ack) { - PacketBuffer * data = data_ForNow.Release_ForNow(); uint8_t * characteristic; mTxCharCount++; @@ -443,12 +433,12 @@ bool BtpEngine::HandleCharacteristicSend(System::PacketBufferHandle data_ForNow, if (mTxState == kState_Idle) { - if (data == nullptr) + if (data.IsNull()) { return false; } - mTxBuf = data; + mTxBuf = std::move(data); mTxState = kState_InProgress; mTxLength = mTxBuf->DataLength(); @@ -513,7 +503,7 @@ bool BtpEngine::HandleCharacteristicSend(System::PacketBufferHandle data_ForNow, } else if (mTxState == kState_InProgress) { - if (data != nullptr) + if (!data.IsNull()) { return false; } @@ -571,22 +561,13 @@ bool BtpEngine::HandleCharacteristicSend(System::PacketBufferHandle data_ForNow, return true; } -PacketBuffer * BtpEngine::TxPacket() -{ - return mTxBuf; -} - -bool BtpEngine::ClearTxPacket() +void BtpEngine::ClearTxPacket() { if (mTxState == kState_Complete) { mTxState = kState_Idle; - mTxBuf = nullptr; - // do not reset mTxNextSeqNum - return true; } - - return false; + mTxBuf = nullptr; } void BtpEngine::LogState() const @@ -595,7 +576,7 @@ void BtpEngine::LogState() const ChipLogError(Ble, "mRxFragmentSize: %d", mRxFragmentSize); ChipLogError(Ble, "mRxState: %d", mRxState); - ChipLogError(Ble, "mRxBuf: %p", mRxBuf); + ChipLogError(Ble, "mRxBuf: %p", mRxBuf.Get_ForNow()); ChipLogError(Ble, "mRxNextSeqNum: %d", mRxNextSeqNum); ChipLogError(Ble, "mRxNewestUnackedSeqNum: %d", mRxNewestUnackedSeqNum); ChipLogError(Ble, "mRxOldestUnackedSeqNum: %d", mRxOldestUnackedSeqNum); @@ -604,7 +585,7 @@ void BtpEngine::LogState() const ChipLogError(Ble, "mTxFragmentSize: %d", mTxFragmentSize); ChipLogError(Ble, "mTxState: %d", mTxState); - ChipLogError(Ble, "mTxBuf: %p", mTxBuf); + ChipLogError(Ble, "mTxBuf: %p", mTxBuf.Get_ForNow()); ChipLogError(Ble, "mTxNextSeqNum: %d", mTxNextSeqNum); ChipLogError(Ble, "mTxNewestUnackedSeqNum: %d", mTxNewestUnackedSeqNum); ChipLogError(Ble, "mTxOldestUnackedSeqNum: %d", mTxOldestUnackedSeqNum); diff --git a/src/ble/BtpEngine.h b/src/ble/BtpEngine.h index 14984f637e6488..cf600f6fdd9981 100644 --- a/src/ble/BtpEngine.h +++ b/src/ble/BtpEngine.h @@ -44,6 +44,7 @@ namespace chip { namespace Ble { using ::chip::System::PacketBuffer; +using ::chip::System::PacketBufferHandle; typedef uint8_t SequenceNumber_t; // If type changed from uint8_t, adjust assumptions in BtpEngine::IsValidAck and // BLEEndPoint::AdjustReceiveWindow. @@ -118,12 +119,12 @@ class BtpEngine inline SequenceNumber_t TxPacketSeq() { return mTxPacketSeq; } inline SequenceNumber_t RxPacketSeq() { return mRxPacketSeq; } inline bool IsCommandPacket(const PacketBufferHandle & p) { return GetFlag(*(p->Start()), kHeaderFlag_CommandMessage); } - inline void PushPacketTag(PacketBuffer * p, PacketType_t type) + inline void PushPacketTag(const PacketBufferHandle & p, PacketType_t type) { p->SetStart(p->Start() - sizeof(type)); memcpy(p->Start(), &type, sizeof(type)); } - inline PacketType_t PopPacketTag(PacketBuffer * p) + inline PacketType_t PopPacketTag(const PacketBufferHandle & p) { PacketType_t type; memcpy(&type, p->Start(), sizeof(type)); @@ -136,13 +137,14 @@ class BtpEngine BLE_ERROR HandleCharacteristicReceived(System::PacketBufferHandle data, SequenceNumber_t & receivedAck, bool & didReceiveAck); bool HandleCharacteristicSend(System::PacketBufferHandle data, bool send_ack); - BLE_ERROR EncodeStandAloneAck(PacketBuffer * data); + BLE_ERROR EncodeStandAloneAck(const PacketBufferHandle & data); - PacketBuffer * RxPacket(); - PacketBuffer * TxPacket(); - - bool ClearRxPacket(); - bool ClearTxPacket(); + void ClearRxPacket(); + PacketBufferHandle TakeRxPacket() { return std::move(mRxBuf); } + PacketBufferHandle BorrowRxPacket() { return mRxBuf.Retain(); } + void ClearTxPacket(); + PacketBufferHandle TakeTxPacket() { return std::move(mTxBuf); } + PacketBufferHandle BorrowTxPacket() { return mTxBuf.Retain(); } void LogState() const; void LogStateDebug() const; @@ -158,7 +160,7 @@ class BtpEngine State_t mRxState; uint16_t mRxLength; void * mAppState; - PacketBuffer * mRxBuf; + System::PacketBufferHandle mRxBuf; SequenceNumber_t mRxNextSeqNum; SequenceNumber_t mRxNewestUnackedSeqNum; SequenceNumber_t mRxOldestUnackedSeqNum; @@ -166,7 +168,7 @@ class BtpEngine State_t mTxState; uint16_t mTxLength; - PacketBuffer * mTxBuf; + System::PacketBufferHandle mTxBuf; SequenceNumber_t mTxNextSeqNum; SequenceNumber_t mTxNewestUnackedSeqNum; SequenceNumber_t mTxOldestUnackedSeqNum; diff --git a/src/controller/CHIPDevice.cpp b/src/controller/CHIPDevice.cpp index 451b33231cdc22..27e5e07b84b567 100644 --- a/src/controller/CHIPDevice.cpp +++ b/src/controller/CHIPDevice.cpp @@ -47,14 +47,14 @@ using namespace chip::Callback; namespace chip { namespace Controller { -CHIP_ERROR Device::SendMessage(System::PacketBuffer * buffer) +CHIP_ERROR Device::SendMessage(System::PacketBufferHandle buffer) { CHIP_ERROR err = CHIP_NO_ERROR; - System::PacketBuffer * resend = nullptr; + System::PacketBufferHandle resend; VerifyOrExit(mSessionManager != nullptr, err = CHIP_ERROR_INCORRECT_STATE); - VerifyOrExit(buffer != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrExit(!buffer.IsNull(), err = CHIP_ERROR_INVALID_ARGUMENT); // If there is no secure connection to the device, try establishing it if (mState != ConnectionState::SecureConnected) @@ -66,49 +66,31 @@ CHIP_ERROR Device::SendMessage(System::PacketBuffer * buffer) { // Secure connection already exists // Hold on to the buffer, in case session resumption and resend is needed - buffer->AddRef(); - resend = buffer; + resend = buffer.Retain(); } - err = mSessionManager->SendMessage(mDeviceId, buffer); + err = mSessionManager->SendMessage(mDeviceId, std::move(buffer)); buffer = nullptr; ChipLogDetail(Controller, "SendMessage returned %d", err); // The send could fail due to network timeouts (e.g. broken pipe) // Try sesion resumption if needed - if (err != CHIP_NO_ERROR && resend != nullptr && mState == ConnectionState::SecureConnected) + if (err != CHIP_NO_ERROR && !resend.IsNull() && mState == ConnectionState::SecureConnected) { mState = ConnectionState::NotConnected; err = LoadSecureSessionParameters(); SuccessOrExit(err); - err = mSessionManager->SendMessage(mDeviceId, resend); - resend = nullptr; + err = mSessionManager->SendMessage(mDeviceId, std::move(resend)); ChipLogDetail(Controller, "Re-SendMessage returned %d", err); SuccessOrExit(err); } exit: - - if (buffer != nullptr) - { - PacketBuffer::Free(buffer); - } - - if (resend != nullptr) - { - PacketBuffer::Free(resend); - } - return err; } -CHIP_ERROR Device::SendMessage(System::PacketBufferHandle message) -{ - return SendMessage(message.Release_ForNow()); -} - CHIP_ERROR Device::Serialize(SerializedDevice & output) { CHIP_ERROR error = CHIP_NO_ERROR; diff --git a/src/controller/CHIPDevice.h b/src/controller/CHIPDevice.h index c13bd7139def06..116a08f27d2450 100644 --- a/src/controller/CHIPDevice.h +++ b/src/controller/CHIPDevice.h @@ -63,19 +63,6 @@ class DLL_EXPORT Device void SetDelegate(DeviceStatusDelegate * delegate) { mStatusDelegate = delegate; } // ----- Messaging ----- - /** - * @brief - * Send the provided message to the device - * - * @param[in] message The message to be sent. The ownership of the message buffer - * is handed over to Device object. SendMessage() will - * decrement the reference count of the message buffer before - * returning. - * - * @return CHIP_ERROR CHIP_NO_ERROR on success, or corresponding error - */ - CHIP_ERROR SendMessage(System::PacketBuffer * message); - /** * @brief * Send the provided message to the device diff --git a/src/controller/CHIPDeviceController_deprecated.cpp b/src/controller/CHIPDeviceController_deprecated.cpp index 20c94a32e215d8..4c69c931c0f723 100644 --- a/src/controller/CHIPDeviceController_deprecated.cpp +++ b/src/controller/CHIPDeviceController_deprecated.cpp @@ -184,11 +184,11 @@ CHIP_ERROR ChipDeviceController::DisconnectDevice() return CHIP_NO_ERROR; } -CHIP_ERROR ChipDeviceController::SendMessage(void * appReqState, PacketBuffer * buffer, NodeId peerDevice) +CHIP_ERROR ChipDeviceController::SendMessage(void * appReqState, PacketBufferHandle buffer, NodeId peerDevice) { CHIP_ERROR err = CHIP_NO_ERROR; - VerifyOrExit(buffer != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrExit(!buffer.IsNull(), err = CHIP_ERROR_INVALID_ARGUMENT); mAppReqState = appReqState; @@ -214,7 +214,7 @@ CHIP_ERROR ChipDeviceController::SendMessage(void * appReqState, PacketBuffer * VerifyOrExit(mDevice != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT); mDevice->SetDelegate(this); - err = mDevice->SendMessage(buffer); + err = mDevice->SendMessage(std::move(buffer)); exit: diff --git a/src/controller/CHIPDeviceController_deprecated.h b/src/controller/CHIPDeviceController_deprecated.h index 462523dea8670a..d851700fac028b 100644 --- a/src/controller/CHIPDeviceController_deprecated.h +++ b/src/controller/CHIPDeviceController_deprecated.h @@ -155,7 +155,7 @@ class DLL_EXPORT ChipDeviceController : public Controller::DeviceStatusDelegate * @param[in] peerDevice Device ID of the peer device * @return CHIP_ERROR The return status */ - CHIP_ERROR SendMessage(void * appReqState, System::PacketBuffer * buffer, NodeId peerDevice = kUndefinedNodeId); + CHIP_ERROR SendMessage(void * appReqState, System::PacketBufferHandle buffer, NodeId peerDevice = kUndefinedNodeId); // ----- IO ----- /** diff --git a/src/controller/java/AndroidBlePlatformDelegate.cpp b/src/controller/java/AndroidBlePlatformDelegate.cpp index 0747be9c993109..c4cb416d93e2e2 100644 --- a/src/controller/java/AndroidBlePlatformDelegate.cpp +++ b/src/controller/java/AndroidBlePlatformDelegate.cpp @@ -77,17 +77,15 @@ bool AndroidBlePlatformDelegate::CloseConnection(BLE_CONNECTION_OBJECT connObj) } bool AndroidBlePlatformDelegate::SendIndication(BLE_CONNECTION_OBJECT connObj, const ChipBleUUID * svcId, - const ChipBleUUID * charId, PacketBuffer * pBuf) + const ChipBleUUID * charId, PacketBufferHandle pBuf) { - // Release delegate's reference to pBuf. pBuf will be freed when both + // Going out of scope releases delegate's reference to pBuf. pBuf will be freed when both // platform delegate and Chip stack free their references to it. - PacketBuffer::Free(pBuf); - return false; } bool AndroidBlePlatformDelegate::SendWriteRequest(BLE_CONNECTION_OBJECT connObj, const ChipBleUUID * svcId, - const ChipBleUUID * charId, PacketBuffer * pBuf) + const ChipBleUUID * charId, PacketBufferHandle pBuf) { bool rc = true; if (SendWriteRequestCb) @@ -96,17 +94,15 @@ bool AndroidBlePlatformDelegate::SendWriteRequest(BLE_CONNECTION_OBJECT connObj, pBuf->Start(), pBuf->DataLength()); } - // Release delegate's reference to pBuf. pBuf will be freed when both + // Going out of scope releases delegate's reference to pBuf. pBuf will be freed when both // platform delegate and Chip stack free their references to it. // We release pBuf's reference here since its payload bytes were copied // onto the Java heap by SendWriteRequestCb. - PacketBuffer::Free(pBuf); - return rc; } bool AndroidBlePlatformDelegate::SendReadRequest(BLE_CONNECTION_OBJECT connObj, const ChipBleUUID * svcId, - const ChipBleUUID * charId, PacketBuffer * pBuf) + const ChipBleUUID * charId, PacketBufferHandle pBuf) { return true; } diff --git a/src/controller/java/AndroidBlePlatformDelegate.h b/src/controller/java/AndroidBlePlatformDelegate.h index ba3f436647b57e..f29317d1cf69ce 100644 --- a/src/controller/java/AndroidBlePlatformDelegate.h +++ b/src/controller/java/AndroidBlePlatformDelegate.h @@ -47,11 +47,11 @@ class AndroidBlePlatformDelegate : public chip::Ble::BlePlatformDelegate bool CloseConnection(BLE_CONNECTION_OBJECT connObj); uint16_t GetMTU(BLE_CONNECTION_OBJECT connObj) const; bool SendIndication(BLE_CONNECTION_OBJECT connObj, const chip::Ble::ChipBleUUID * svcId, const chip::Ble::ChipBleUUID * charId, - chip::System::PacketBuffer * pBuf); + chip::System::PacketBufferHandle pBuf); bool SendWriteRequest(BLE_CONNECTION_OBJECT connObj, const chip::Ble::ChipBleUUID * svcId, - const chip::Ble::ChipBleUUID * charId, chip::System::PacketBuffer * pBuf); + const chip::Ble::ChipBleUUID * charId, chip::System::PacketBufferHandle pBuf); bool SendReadRequest(BLE_CONNECTION_OBJECT connObj, const chip::Ble::ChipBleUUID * svcId, const chip::Ble::ChipBleUUID * charId, - chip::System::PacketBuffer * pBuf); + chip::System::PacketBufferHandle pBuf); bool SendReadResponse(BLE_CONNECTION_OBJECT connObj, BLE_READ_REQUEST_CONTEXT requestContext, const chip::Ble::ChipBleUUID * svcId, const chip::Ble::ChipBleUUID * charId); diff --git a/src/controller/java/CHIPDeviceController-JNI.cpp b/src/controller/java/CHIPDeviceController-JNI.cpp index cbb9457bbd66d0..fa578afb879905 100644 --- a/src/controller/java/CHIPDeviceController-JNI.cpp +++ b/src/controller/java/CHIPDeviceController-JNI.cpp @@ -362,7 +362,7 @@ JNI_METHOD(void, beginSendMessage)(JNIEnv * env, jobject self, jlong handle, jst { memcpy(buffer->Start(), messageStr, messageLen); buffer->SetDataLength(messageLen); - err = wrapper->Controller()->SendMessage((void *) "SendMessage", buffer.Release_ForNow()); + err = wrapper->Controller()->SendMessage((void *) "SendMessage", std::move(buffer)); } } @@ -426,7 +426,7 @@ JNI_METHOD(void, beginSendCommand)(JNIEnv * env, jobject self, jlong handle, job buffer->SetDataLength(dataLength); // Hardcode endpoint to 1 for now - err = wrapper->Controller()->SendMessage((void *) "SendMessage", buffer.Release_ForNow()); + err = wrapper->Controller()->SendMessage((void *) "SendMessage", std::move(buffer)); } } diff --git a/src/controller/python/ChipDeviceController-BlePlatformDelegate.cpp b/src/controller/python/ChipDeviceController-BlePlatformDelegate.cpp index 62d966fbb928c2..c753b65b848405 100644 --- a/src/controller/python/ChipDeviceController-BlePlatformDelegate.cpp +++ b/src/controller/python/ChipDeviceController-BlePlatformDelegate.cpp @@ -75,39 +75,37 @@ uint16_t DeviceController_BlePlatformDelegate::GetMTU(BLE_CONNECTION_OBJECT conn } bool DeviceController_BlePlatformDelegate::SendIndication(BLE_CONNECTION_OBJECT connObj, const chip::Ble::ChipBleUUID * svcId, - const chip::Ble::ChipBleUUID * charId, chip::System::PacketBuffer * pBuf) + const chip::Ble::ChipBleUUID * charId, + chip::System::PacketBufferHandle pBuf) { // TODO Python queue-based implementation - // Release delegate's reference to pBuf. pBuf will be freed when both platform delegate and Chip stack free their references to - // it. - chip::System::PacketBuffer::Free(pBuf); - + // Going out of scope releases delegate's reference to pBuf. pBuf will be freed when both platform delegate and Chip stack free + // their references to it. return false; } bool DeviceController_BlePlatformDelegate::SendWriteRequest(BLE_CONNECTION_OBJECT connObj, const chip::Ble::ChipBleUUID * svcId, const chip::Ble::ChipBleUUID * charId, - chip::System::PacketBuffer * pBuf) + chip::System::PacketBufferHandle pBuf) { bool ret = false; - if (writeCB && svcId && charId && pBuf) + if (writeCB && svcId && charId && !pBuf.IsNull()) { ret = writeCB(connObj, static_cast(svcId->bytes), static_cast(charId->bytes), static_cast(pBuf->Start()), pBuf->DataLength()); } - // Release delegate's reference to pBuf. pBuf will be freed when both platform delegate and Chip stack free their references to - // it. We release pBuf's reference here since its payload bytes were copied into a new NSData object by ChipBleMgr.py's writeCB, - // and in both the error and succees cases this code has no further use for the pBuf PacketBuffer. - chip::System::PacketBuffer::Free(pBuf); - + // Going out of scope releases delegate's reference to pBuf. pBuf will be freed when both platform delegate and Chip stack free + // their references to it. We release pBuf's reference here since its payload bytes were copied into a new NSData object by + // ChipBleMgr.py's writeCB, and in both the error and succees cases this code has no further use for the pBuf PacketBuffer. return ret; } bool DeviceController_BlePlatformDelegate::SendReadRequest(BLE_CONNECTION_OBJECT connObj, const chip::Ble::ChipBleUUID * svcId, - const chip::Ble::ChipBleUUID * charId, chip::System::PacketBuffer * pBuf) + const chip::Ble::ChipBleUUID * charId, + chip::System::PacketBufferHandle pBuf) { // TODO Python queue-based implementation return false; diff --git a/src/controller/python/ChipDeviceController-BlePlatformDelegate.h b/src/controller/python/ChipDeviceController-BlePlatformDelegate.h index 5703fef591f374..9f14ea7988f313 100644 --- a/src/controller/python/ChipDeviceController-BlePlatformDelegate.h +++ b/src/controller/python/ChipDeviceController-BlePlatformDelegate.h @@ -45,11 +45,11 @@ class DeviceController_BlePlatformDelegate : public chip::Ble::BlePlatformDelega bool CloseConnection(BLE_CONNECTION_OBJECT connObj); uint16_t GetMTU(BLE_CONNECTION_OBJECT connObj) const; bool SendIndication(BLE_CONNECTION_OBJECT connObj, const chip::Ble::ChipBleUUID * svcId, const chip::Ble::ChipBleUUID * charId, - chip::System::PacketBuffer * pBuf); + chip::System::PacketBufferHandle pBuf); bool SendWriteRequest(BLE_CONNECTION_OBJECT connObj, const chip::Ble::ChipBleUUID * svcId, - const chip::Ble::ChipBleUUID * charId, chip::System::PacketBuffer * pBuf); + const chip::Ble::ChipBleUUID * charId, chip::System::PacketBufferHandle pBuf); bool SendReadRequest(BLE_CONNECTION_OBJECT connObj, const chip::Ble::ChipBleUUID * svcId, const chip::Ble::ChipBleUUID * charId, - chip::System::PacketBuffer * pBuf); + chip::System::PacketBufferHandle pBuf); bool SendReadResponse(BLE_CONNECTION_OBJECT connObj, BLE_READ_REQUEST_CONTEXT requestContext, const chip::Ble::ChipBleUUID * svcId, const chip::Ble::ChipBleUUID * charId); diff --git a/src/darwin/Framework/CHIP/CHIPDeviceStatusDelegateBridge.h b/src/darwin/Framework/CHIP/CHIPDeviceStatusDelegateBridge.h index bf9ebebeb36136..c15cc935eb73a0 100644 --- a/src/darwin/Framework/CHIP/CHIPDeviceStatusDelegateBridge.h +++ b/src/darwin/Framework/CHIP/CHIPDeviceStatusDelegateBridge.h @@ -30,7 +30,7 @@ class CHIPDeviceStatusDelegateBridge : public chip::Controller::DeviceStatusDele void setDelegate(id delegate, dispatch_queue_t queue); - void OnMessage(chip::System::PacketBuffer * message) override; + void OnMessage(chip::System::PacketBufferHandle message) override; void OnStatusChange() override; diff --git a/src/darwin/Framework/CHIP/CHIPDeviceStatusDelegateBridge.mm b/src/darwin/Framework/CHIP/CHIPDeviceStatusDelegateBridge.mm index c63ab4a78309cc..fb020fb8e95663 100644 --- a/src/darwin/Framework/CHIP/CHIPDeviceStatusDelegateBridge.mm +++ b/src/darwin/Framework/CHIP/CHIPDeviceStatusDelegateBridge.mm @@ -36,23 +36,21 @@ } } -void CHIPDeviceStatusDelegateBridge::OnMessage(chip::System::PacketBuffer * message) +void CHIPDeviceStatusDelegateBridge::OnMessage(chip::System::PacketBufferHandle message) { NSLog(@"DeviceStatusDelegate received message from the device"); size_t data_len = message->DataLength(); // convert to NSData NSMutableData * dataBuffer = [[NSMutableData alloc] initWithBytes:message->Start() length:data_len]; - message = message->Next(); + message.FreeHead(); - while (message != NULL) { + while (!message.IsNull()) { data_len = message->DataLength(); [dataBuffer appendBytes:message->Start() length:data_len]; - message = message->Next(); + message.FreeHead(); } - chip::System::PacketBuffer::Free(message); - id strongDelegate = mDelegate; if (strongDelegate && mQueue) { dispatch_async(mQueue, ^{ diff --git a/src/inet/InetLayer.cpp b/src/inet/InetLayer.cpp index f8173bf9fac5e4..c92d019fe78049 100644 --- a/src/inet/InetLayer.cpp +++ b/src/inet/InetLayer.cpp @@ -1066,9 +1066,12 @@ chip::System::Error InetLayer::HandleInetLayerEvent(chip::System::Object & aTarg static_cast(aTarget).HandleIncomingConnection(reinterpret_cast(aArgument)); break; - case kInetEvent_TCPDataReceived: - static_cast(aTarget).HandleDataReceived(reinterpret_cast(aArgument)); - break; + case kInetEvent_TCPDataReceived: { + chip::System::PacketBufferHandle buf; + buf.Adopt(reinterpret_cast(aArgument)); + static_cast(aTarget).HandleDataReceived(std::move(buf)); + } + break; case kInetEvent_TCPDataSent: static_cast(aTarget).HandleDataSent(static_cast(aArgument)); diff --git a/src/inet/RawEndPoint.cpp b/src/inet/RawEndPoint.cpp index 35c23604f65cdf..e8bb78b2af1091 100644 --- a/src/inet/RawEndPoint.cpp +++ b/src/inet/RawEndPoint.cpp @@ -948,6 +948,7 @@ INET_ERROR RawEndPoint::GetPCB(IPAddressType addrType) * This fn() may be executed concurrently with SetICMPFilter() * - this fn() runs in the LwIP thread (and the lock has already been taken) * - SetICMPFilter() runs in the Inet thread. + * Returns non-zero if and only if ownership of the pbuf has been taken. */ #if LWIP_VERSION_MAJOR > 1 || LWIP_VERSION_MINOR >= 5 u8_t RawEndPoint::LwIPReceiveRawMessage(void * arg, struct raw_pcb * pcb, struct pbuf * p, const ip_addr_t * addr) @@ -986,10 +987,9 @@ u8_t RawEndPoint::LwIPReceiveRawMessage(void * arg, struct raw_pcb * pcb, struct if (enqueue) { - System::PacketBufferHandle buf_ForNow; - buf_ForNow.Adopt(buf); - pktInfo = GetPacketInfo(buf_ForNow); - buf = buf_ForNow.Release_ForNow(); + System::PacketBufferHandle bufHandle; + bufHandle.Adopt(buf); + pktInfo = GetPacketInfo(bufHandle); if (pktInfo != NULL) { @@ -1016,8 +1016,12 @@ u8_t RawEndPoint::LwIPReceiveRawMessage(void * arg, struct raw_pcb * pcb, struct pktInfo->DestPort = 0; } + buf = bufHandle.Release_ForNow(); if (lSystemLayer.PostEvent(*ep, kInetEvent_RawDataReceived, (uintptr_t) buf) != INET_NO_ERROR) + { + // If PostEvent() failed, it has not taken ownership of the buffer, so we need to free it ourselves. PacketBuffer::Free(buf); + } } return enqueue; diff --git a/src/inet/TCPEndPoint.cpp b/src/inet/TCPEndPoint.cpp index 9849181e4ab986..e458aa3359d46c 100644 --- a/src/inet/TCPEndPoint.cpp +++ b/src/inet/TCPEndPoint.cpp @@ -704,7 +704,7 @@ INET_ERROR TCPEndPoint::Send(PacketBuffer * data, bool push) if (mSendQueue == nullptr) mSendQueue = data; else - mSendQueue->AddToEnd(data); + mSendQueue->AddToEnd_ForNow(data); #if CHIP_SYSTEM_CONFIG_USE_LWIP @@ -1019,12 +1019,12 @@ INET_ERROR TCPEndPoint::AckReceive(uint16_t len) return res; } -INET_ERROR TCPEndPoint::PutBackReceivedData(PacketBuffer * data) +INET_ERROR TCPEndPoint::PutBackReceivedData(System::PacketBufferHandle data) { if (!IsConnected()) return INET_ERROR_INCORRECT_STATE; - mRcvQueue = data; + mRcvQueue = std::move(data); return INET_NO_ERROR; } @@ -1038,7 +1038,7 @@ uint32_t TCPEndPoint::PendingSendLength() uint32_t TCPEndPoint::PendingReceiveLength() { - if (mRcvQueue != nullptr) + if (!mRcvQueue.IsNull()) return mRcvQueue->TotalLength(); return 0; } @@ -1067,7 +1067,6 @@ INET_ERROR TCPEndPoint::Shutdown() INET_ERROR TCPEndPoint::Close() { // Clear the receive queue. - PacketBuffer::Free(mRcvQueue); mRcvQueue = nullptr; // Suppress closing callbacks, since the application explicitly called Close(). @@ -1326,7 +1325,7 @@ INET_ERROR TCPEndPoint::DriveSending() if (lenSent < bufLen) mSendQueue->ConsumeHead(lenSent); else - mSendQueue = PacketBuffer::FreeHead(mSendQueue); + mSendQueue = PacketBuffer::FreeHead_ForNow(mSendQueue); if (OnDataSent != nullptr) OnDataSent(this, lenSent); @@ -1392,18 +1391,14 @@ void TCPEndPoint::DriveReceiving() { // If there's data in the receive queue and the app is ready to receive it then call the app's callback // with the entire receive queue. - if (mRcvQueue != nullptr && ReceiveEnabled && OnDataReceived != nullptr) + if (!mRcvQueue.IsNull() && ReceiveEnabled && OnDataReceived != nullptr) { - PacketBuffer * rcvQueue = mRcvQueue; - mRcvQueue = nullptr; - System::PacketBufferHandle rcvQueue_ForNow; - rcvQueue_ForNow.Adopt(rcvQueue); - OnDataReceived(this, std::move(rcvQueue_ForNow)); + OnDataReceived(this, std::move(mRcvQueue)); } // If the connection is closing, and the receive queue is now empty, call DoClose() to complete // the process of closing the connection. - if (State == kState_Closing && mRcvQueue == nullptr) + if (State == kState_Closing && mRcvQueue.IsNull()) DoClose(INET_NO_ERROR, false); } @@ -1441,7 +1436,7 @@ INET_ERROR TCPEndPoint::DoClose(INET_ERROR err, bool suppressCallback) // AND there is data waiting to be processed on either the send or receive queues // ... THEN enter the Closing state, allowing the queued data to drain, // ... OTHERWISE go straight to the Closed state. - if (IsConnected() && err == INET_NO_ERROR && (mSendQueue != nullptr || mRcvQueue != nullptr)) + if (IsConnected() && err == INET_NO_ERROR && (mSendQueue != nullptr || !mRcvQueue.IsNull())) State = kState_Closing; else State = kState_Closed; @@ -1570,8 +1565,7 @@ INET_ERROR TCPEndPoint::DoClose(INET_ERROR err, bool suppressCallback) // Clear clear the send and receive queues. PacketBuffer::Free(mSendQueue); mSendQueue = nullptr; - PacketBuffer::Free(mRcvQueue); - mRcvQueue = nullptr; + mRcvQueue = nullptr; #if CHIP_SYSTEM_CONFIG_USE_LWIP mUnackedLength = 0; #endif // CHIP_SYSTEM_CONFIG_USE_LWIP @@ -1969,7 +1963,7 @@ void TCPEndPoint::HandleDataSent(uint16_t lenSent) } } -void TCPEndPoint::HandleDataReceived(PacketBuffer * buf) +void TCPEndPoint::HandleDataReceived(System::PacketBufferHandle buf) { // Only receive new data while in the Connected or SendShutdown states. if (State == kState_Connected || State == kState_SendShutdown) @@ -1979,13 +1973,15 @@ void TCPEndPoint::HandleDataReceived(PacketBuffer * buf) // If we received a data buffer, queue it on the receive queue. If there's already data in // the queue, compact the data into the head buffer. - if (buf != NULL) + if (!buf.IsNull()) { - if (mRcvQueue == NULL) - mRcvQueue = buf; + if (mRcvQueue.IsNull()) + { + mRcvQueue = std::move(buf); + } else { - mRcvQueue->AddToEnd(buf); + mRcvQueue->AddToEnd(std::move(buf)); mRcvQueue->CompactHead(); } } @@ -2012,8 +2008,6 @@ void TCPEndPoint::HandleDataReceived(PacketBuffer * buf) // Drive the received data into the app. DriveReceiving(); } - else - PacketBuffer::Free(buf); } void TCPEndPoint::HandleIncomingConnection(TCPEndPoint * conEP) @@ -2418,11 +2412,11 @@ void TCPEndPoint::ReceiveData() PacketBuffer * rcvBuf; bool isNewBuf = true; - if (mRcvQueue == nullptr) + if (mRcvQueue.IsNull()) rcvBuf = PacketBuffer::New(0).Release_ForNow(); else { - rcvBuf = mRcvQueue; + rcvBuf = mRcvQueue.Get_ForNow(); for (PacketBuffer * nextBuf = rcvBuf->Next(); nextBuf != nullptr; rcvBuf = nextBuf, nextBuf = nextBuf->Next()) ; @@ -2535,10 +2529,10 @@ void TCPEndPoint::ReceiveData() size_t newDataLength = rcvBuf->DataLength() + static_cast(rcvLen); VerifyOrDie(CanCastTo(newDataLength)); rcvBuf->SetDataLength(static_cast(newDataLength)); - if (mRcvQueue == nullptr) - mRcvQueue = rcvBuf; + if (mRcvQueue.IsNull()) + mRcvQueue.Adopt(rcvBuf); else - mRcvQueue->AddToEnd(rcvBuf); + mRcvQueue->AddToEnd_ForNow(rcvBuf); } else @@ -2546,7 +2540,7 @@ void TCPEndPoint::ReceiveData() VerifyOrDie(rcvLen > 0); size_t newDataLength = rcvBuf->DataLength() + static_cast(rcvLen); VerifyOrDie(CanCastTo(newDataLength)); - rcvBuf->SetDataLength(static_cast(newDataLength), mRcvQueue); + rcvBuf->SetDataLength(static_cast(newDataLength), mRcvQueue.Get_ForNow()); } } diff --git a/src/inet/TCPEndPoint.h b/src/inet/TCPEndPoint.h index 13db440251e396..929ec36d9025b5 100644 --- a/src/inet/TCPEndPoint.h +++ b/src/inet/TCPEndPoint.h @@ -323,7 +323,7 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis * portion remaining after the bytes acknowledged by a prior call to the * AckReceive(uint16_t len) method. */ - INET_ERROR PutBackReceivedData(chip::System::PacketBuffer * data); + INET_ERROR PutBackReceivedData(chip::System::PacketBufferHandle data); /** * @brief Extract the length of the data awaiting first transmit. @@ -564,7 +564,7 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis private: static chip::System::ObjectPool sPool; - chip::System::PacketBuffer * mRcvQueue; + chip::System::PacketBufferHandle mRcvQueue; chip::System::PacketBuffer * mSendQueue; #if INET_TCP_IDLE_CHECK_INTERVAL > 0 uint16_t mIdleTimeout; // in units of INET_TCP_IDLE_CHECK_INTERVAL; zero means no timeout @@ -647,7 +647,7 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis BufferOffset FindStartOfUnsent(); INET_ERROR GetPCB(IPAddressType addrType); void HandleDataSent(uint16_t len); - void HandleDataReceived(chip::System::PacketBuffer * buf); + void HandleDataReceived(chip::System::PacketBufferHandle buf); void HandleIncomingConnection(TCPEndPoint * pcb); void HandleError(INET_ERROR err); diff --git a/src/inet/UDPEndPoint.cpp b/src/inet/UDPEndPoint.cpp index 837fc112c35f92..bd05f6785a02fa 100644 --- a/src/inet/UDPEndPoint.cpp +++ b/src/inet/UDPEndPoint.cpp @@ -878,7 +878,6 @@ void UDPEndPoint::LwIPReceiveUDPMessage(void * arg, struct udp_pcb * pcb, struct System::PacketBufferHandle buf_ForNow; buf_ForNow.Adopt(buf); pktInfo = GetPacketInfo(buf_ForNow); - buf = buf_ForNow.Release_ForNow(); if (pktInfo != NULL) { #if LWIP_VERSION_MAJOR > 1 || LWIP_VERSION_MINOR >= 5 @@ -904,8 +903,12 @@ void UDPEndPoint::LwIPReceiveUDPMessage(void * arg, struct udp_pcb * pcb, struct pktInfo->DestPort = pcb->local_port; } + buf = buf_ForNow.Release_ForNow(); if (lSystemLayer.PostEvent(*ep, kInetEvent_UDPDataReceived, (uintptr_t) buf) != INET_NO_ERROR) + { + // If PostEvent() failed, it has not taken ownership of the buffer, so we need to free it ourselves. PacketBuffer::Free(buf); + } } #endif // CHIP_SYSTEM_CONFIG_USE_LWIP diff --git a/src/inet/tests/TestInetLayer.cpp b/src/inet/tests/TestInetLayer.cpp index 1ca62e2ce8b5d5..a75de76626686d 100644 --- a/src/inet/tests/TestInetLayer.cpp +++ b/src/inet/tests/TestInetLayer.cpp @@ -610,7 +610,7 @@ static void HandleTCPDataReceived(TCPEndPoint * aEndPoint, PacketBufferHandle aB if (aEndPoint->State != TCPEndPoint::kState_Connected) { - lStatus = aEndPoint->PutBackReceivedData(aBuffer.Release_ForNow()); + lStatus = aEndPoint->PutBackReceivedData(std::move(aBuffer)); INET_FAIL_ERROR(lStatus, "TCPEndPoint::PutBackReceivedData failed"); goto exit; } diff --git a/src/lib/core/CHIPTLVWriter.cpp b/src/lib/core/CHIPTLVWriter.cpp index 65c77b208b78ff..c4ca1f6885e7f2 100644 --- a/src/lib/core/CHIPTLVWriter.cpp +++ b/src/lib/core/CHIPTLVWriter.cpp @@ -1853,7 +1853,7 @@ CHIP_ERROR TLVWriter::GetNewPacketBuffer(TLVWriter & writer, uintptr_t & bufHand { newBuf = PacketBuffer::New(0).Release_ForNow(); if (newBuf != nullptr) - buf->AddToEnd(newBuf); + buf->AddToEnd_ForNow(newBuf); } if (newBuf != nullptr) diff --git a/src/messaging/ExchangeContext.cpp b/src/messaging/ExchangeContext.cpp index e34c8f03fd3bef..4390c062cba0ba 100644 --- a/src/messaging/ExchangeContext.cpp +++ b/src/messaging/ExchangeContext.cpp @@ -69,8 +69,8 @@ void ExchangeContext::SetResponseExpected(bool inResponseExpected) mFlags.Set(ExFlagValues::kFlagResponseExpected, inResponseExpected); } -CHIP_ERROR ExchangeContext::SendMessage(uint16_t protocolId, uint8_t msgType, PacketBuffer * msgBuf, const SendFlags & sendFlags, - void * msgCtxt) +CHIP_ERROR ExchangeContext::SendMessage(uint16_t protocolId, uint8_t msgType, PacketBufferHandle msgBuf, + const SendFlags & sendFlags, void * msgCtxt) { CHIP_ERROR err = CHIP_NO_ERROR; PayloadHeader payloadHeader; @@ -110,8 +110,7 @@ CHIP_ERROR ExchangeContext::SendMessage(uint16_t protocolId, uint8_t msgType, Pa payloadHeader.SetInitiator(IsInitiator()); - err = mExchangeMgr->GetSessionMgr()->SendMessage(payloadHeader, mPeerNodeId, msgBuf); - msgBuf = nullptr; + err = mExchangeMgr->GetSessionMgr()->SendMessage(payloadHeader, mPeerNodeId, std::move(msgBuf)); SuccessOrExit(err); exit: @@ -121,9 +120,11 @@ CHIP_ERROR ExchangeContext::SendMessage(uint16_t protocolId, uint8_t msgType, Pa SetResponseExpected(false); } - if (msgBuf != nullptr && !sendFlags.Has(SendMessageFlags::kSendFlag_RetainBuffer)) + if (sendFlags.Has(SendMessageFlags::kSendFlag_RetainBuffer)) { - PacketBuffer::Free(msgBuf); + // Nothing currently calls us with this flag. Ensure it stays that way until kSendFlag_RetainBuffer is removed + // in favour of callers Retain()ing buffers. + err = CHIP_ERROR_NOT_IMPLEMENTED; } // Release the reference to the exchange context acquired above. Under normal circumstances diff --git a/src/messaging/ExchangeContext.h b/src/messaging/ExchangeContext.h index ec77c49f0dcc07..bd85fe28a5fbd6 100644 --- a/src/messaging/ExchangeContext.h +++ b/src/messaging/ExchangeContext.h @@ -104,7 +104,7 @@ class DLL_EXPORT ExchangeContext : public ReferenceCountedSendMessage(0x0001, 0x0002, System::PacketBuffer::New().Release_ForNow(), - SendFlags(Messaging::SendMessageFlags::kSendFlag_None)); + ec1->SendMessage(0x0001, 0x0002, System::PacketBuffer::New(), SendFlags(Messaging::SendMessageFlags::kSendFlag_None)); NL_TEST_ASSERT(inSuite, !mockUnsolicitedAppDelegate.IsOnMessageReceivedCalled); // send a good packet - ec1->SendMessage(0x0001, 0x0001, System::PacketBuffer::New().Release_ForNow(), - SendFlags(Messaging::SendMessageFlags::kSendFlag_None)); + ec1->SendMessage(0x0001, 0x0001, System::PacketBuffer::New(), SendFlags(Messaging::SendMessageFlags::kSendFlag_None)); NL_TEST_ASSERT(inSuite, mockUnsolicitedAppDelegate.IsOnMessageReceivedCalled); } diff --git a/src/platform/Darwin/BlePlatformDelegate.h b/src/platform/Darwin/BlePlatformDelegate.h index 06539cbf004da7..80b3dfff2d6e73 100644 --- a/src/platform/Darwin/BlePlatformDelegate.h +++ b/src/platform/Darwin/BlePlatformDelegate.h @@ -21,7 +21,7 @@ #include using ::chip::Ble::ChipBleUUID; -using ::chip::System::PacketBuffer; +using ::chip::System::PacketBufferHandle; namespace chip { namespace DeviceLayer { @@ -35,11 +35,11 @@ class BlePlatformDelegateImpl : public Ble::BlePlatformDelegate virtual bool CloseConnection(BLE_CONNECTION_OBJECT connObj); virtual uint16_t GetMTU(BLE_CONNECTION_OBJECT connObj) const; virtual bool SendIndication(BLE_CONNECTION_OBJECT connObj, const ChipBleUUID * svcId, const ChipBleUUID * charId, - PacketBuffer * pBuf); + PacketBufferHandle pBuf); virtual bool SendWriteRequest(BLE_CONNECTION_OBJECT connObj, const ChipBleUUID * svcId, const ChipBleUUID * charId, - PacketBuffer * pBuf); + PacketBufferHandle pBuf); virtual bool SendReadRequest(BLE_CONNECTION_OBJECT connObj, const ChipBleUUID * svcId, const ChipBleUUID * charId, - PacketBuffer * pBuf); + PacketBufferHandle pBuf); virtual bool SendReadResponse(BLE_CONNECTION_OBJECT connObj, BLE_READ_REQUEST_CONTEXT requestContext, const Ble::ChipBleUUID * svcId, const ChipBleUUID * charId); }; diff --git a/src/platform/Darwin/BlePlatformDelegateImpl.mm b/src/platform/Darwin/BlePlatformDelegateImpl.mm index e8768a1425c9bd..50acb0768de271 100644 --- a/src/platform/Darwin/BlePlatformDelegateImpl.mm +++ b/src/platform/Darwin/BlePlatformDelegateImpl.mm @@ -103,16 +103,13 @@ uint16_t BlePlatformDelegateImpl::GetMTU(BLE_CONNECTION_OBJECT connObj) const { return 0; } bool BlePlatformDelegateImpl::SendIndication( - BLE_CONNECTION_OBJECT connObj, const ChipBleUUID * svcId, const ChipBleUUID * charId, PacketBuffer * pBuf) + BLE_CONNECTION_OBJECT connObj, const ChipBleUUID * svcId, const ChipBleUUID * charId, PacketBufferHandle pBuf) { - if (pBuf) { - PacketBuffer::Free(pBuf); - } return false; } bool BlePlatformDelegateImpl::SendWriteRequest( - BLE_CONNECTION_OBJECT connObj, const ChipBleUUID * svcId, const ChipBleUUID * charId, PacketBuffer * pBuf) + BLE_CONNECTION_OBJECT connObj, const ChipBleUUID * svcId, const ChipBleUUID * charId, PacketBufferHandle pBuf) { bool found = false; CBUUID * serviceId = nil; @@ -126,7 +123,7 @@ if (NULL != charId) { characteristicId = [CBUUID UUIDWithData:[NSData dataWithBytes:charId->bytes length:sizeof(charId->bytes)]]; } - if (NULL != pBuf) { + if (!pBuf.IsNull()) { data = [NSData dataWithBytes:pBuf->Start() length:pBuf->DataLength()]; } @@ -142,20 +139,15 @@ } } - if (pBuf) { - // Release delegate's reference to pBuf. pBuf will be freed when both platform delegate and Chip stack free their - // references to it. We release pBuf's reference here since its payload bytes were copied into a new NSData object - PacketBuffer::Free(pBuf); - } + // Going out of scope releases delegate's reference to pBuf. pBuf will be freed when both platform delegate and Chip + // stack free their references to it. We release pBuf's reference here since its payload bytes were copied into a new + // NSData object return found; } bool BlePlatformDelegateImpl::SendReadRequest( - BLE_CONNECTION_OBJECT connObj, const ChipBleUUID * svcId, const ChipBleUUID * charId, PacketBuffer * pBuf) + BLE_CONNECTION_OBJECT connObj, const ChipBleUUID * svcId, const ChipBleUUID * charId, PacketBufferHandle pBuf) { - if (pBuf) { - PacketBuffer::Free(pBuf); - } return false; } diff --git a/src/platform/EFR32/BLEManagerImpl.cpp b/src/platform/EFR32/BLEManagerImpl.cpp index dc9a32e3eb2fc9..5773672d9332c6 100644 --- a/src/platform/EFR32/BLEManagerImpl.cpp +++ b/src/platform/EFR32/BLEManagerImpl.cpp @@ -440,7 +440,7 @@ uint16_t BLEManagerImpl::GetMTU(BLE_CONNECTION_OBJECT conId) const } bool BLEManagerImpl::SendIndication(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId, - PacketBuffer * data) + PacketBufferHandle data) { CHIP_ERROR err = CHIP_NO_ERROR; CHIPoBLEConState * conState = GetConnectionState(conId); @@ -462,7 +462,6 @@ bool BLEManagerImpl::SendIndication(BLE_CONNECTION_OBJECT conId, const ChipBleUU err = MapBLEError(rsp->result); exit: - PacketBuffer::Free(data); if (err != CHIP_NO_ERROR) { ChipLogError(DeviceLayer, "BLEManagerImpl::SendIndication() failed: %s", ErrorStr(err)); @@ -472,14 +471,14 @@ bool BLEManagerImpl::SendIndication(BLE_CONNECTION_OBJECT conId, const ChipBleUU } bool BLEManagerImpl::SendWriteRequest(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId, - PacketBuffer * pBuf) + PacketBufferHandle pBuf) { ChipLogProgress(DeviceLayer, "BLEManagerImpl::SendWriteRequest() not supported"); return false; } bool BLEManagerImpl::SendReadRequest(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId, - PacketBuffer * pBuf) + PacketBufferHandle pBuf) { ChipLogProgress(DeviceLayer, "BLEManagerImpl::SendReadRequest() not supported"); return false; diff --git a/src/platform/EFR32/BLEManagerImpl.h b/src/platform/EFR32/BLEManagerImpl.h index c1e341d5622509..069f8fb679a096 100644 --- a/src/platform/EFR32/BLEManagerImpl.h +++ b/src/platform/EFR32/BLEManagerImpl.h @@ -70,11 +70,11 @@ class BLEManagerImpl final : public BLEManager, private BleLayer, private BlePla bool CloseConnection(BLE_CONNECTION_OBJECT conId) override; uint16_t GetMTU(BLE_CONNECTION_OBJECT conId) const override; bool SendIndication(BLE_CONNECTION_OBJECT conId, const Ble::ChipBleUUID * svcId, const Ble::ChipBleUUID * charId, - System::PacketBuffer * pBuf) override; + System::PacketBufferHandle pBuf) override; bool SendWriteRequest(BLE_CONNECTION_OBJECT conId, const Ble::ChipBleUUID * svcId, const Ble::ChipBleUUID * charId, - System::PacketBuffer * pBuf) override; + System::PacketBufferHandle pBuf) override; bool SendReadRequest(BLE_CONNECTION_OBJECT conId, const Ble::ChipBleUUID * svcId, const Ble::ChipBleUUID * charId, - System::PacketBuffer * pBuf) override; + System::PacketBufferHandle pBuf) override; bool SendReadResponse(BLE_CONNECTION_OBJECT conId, BLE_READ_REQUEST_CONTEXT requestContext, const Ble::ChipBleUUID * svcId, const Ble::ChipBleUUID * charId) override; diff --git a/src/platform/ESP32/BLEManagerImpl.h b/src/platform/ESP32/BLEManagerImpl.h index 5c0eba0023f0ba..ee579f6ec1aa4d 100644 --- a/src/platform/ESP32/BLEManagerImpl.h +++ b/src/platform/ESP32/BLEManagerImpl.h @@ -96,11 +96,11 @@ class BLEManagerImpl final : public BLEManager, bool CloseConnection(BLE_CONNECTION_OBJECT conId) override; uint16_t GetMTU(BLE_CONNECTION_OBJECT conId) const override; bool SendIndication(BLE_CONNECTION_OBJECT conId, const Ble::ChipBleUUID * svcId, const Ble::ChipBleUUID * charId, - System::PacketBuffer * pBuf) override; + System::PacketBufferHandle pBuf) override; bool SendWriteRequest(BLE_CONNECTION_OBJECT conId, const Ble::ChipBleUUID * svcId, const Ble::ChipBleUUID * charId, - System::PacketBuffer * pBuf) override; + System::PacketBufferHandle pBuf) override; bool SendReadRequest(BLE_CONNECTION_OBJECT conId, const Ble::ChipBleUUID * svcId, const Ble::ChipBleUUID * charId, - System::PacketBuffer * pBuf) override; + System::PacketBufferHandle pBuf) override; bool SendReadResponse(BLE_CONNECTION_OBJECT conId, BLE_READ_REQUEST_CONTEXT requestContext, const Ble::ChipBleUUID * svcId, const Ble::ChipBleUUID * charId) override; @@ -145,7 +145,7 @@ class BLEManagerImpl final : public BLEManager, struct CHIPoBLEConState { - System::PacketBuffer * PendingIndBuf; + System::PacketBufferHandle PendingIndBuf; uint16_t ConId; uint16_t MTU : 10; uint16_t Allocated : 1; diff --git a/src/platform/ESP32/bluedroid/BLEManagerImpl.cpp b/src/platform/ESP32/bluedroid/BLEManagerImpl.cpp index 0512af91f34351..414bbcc38cd24d 100644 --- a/src/platform/ESP32/bluedroid/BLEManagerImpl.cpp +++ b/src/platform/ESP32/bluedroid/BLEManagerImpl.cpp @@ -334,7 +334,7 @@ uint16_t BLEManagerImpl::GetMTU(BLE_CONNECTION_OBJECT conId) const } bool BLEManagerImpl::SendIndication(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId, - PacketBuffer * data) + PacketBufferHandle data) { CHIP_ERROR err = CHIP_NO_ERROR; CHIPoBLEConState * conState = GetConnectionState(conId); @@ -343,7 +343,7 @@ bool BLEManagerImpl::SendIndication(BLE_CONNECTION_OBJECT conId, const ChipBleUU VerifyOrExit(conState != NULL, err = CHIP_ERROR_INVALID_ARGUMENT); - VerifyOrExit(conState->PendingIndBuf == NULL, err = CHIP_ERROR_INCORRECT_STATE); + VerifyOrExit(conState->PendingIndBuf.IsNull(), err = CHIP_ERROR_INCORRECT_STATE); err = esp_ble_gatts_send_indicate(mAppIf, conId, mTXCharAttrHandle, data->DataLength(), data->Start(), false); if (err != CHIP_NO_ERROR) @@ -354,28 +354,26 @@ bool BLEManagerImpl::SendIndication(BLE_CONNECTION_OBJECT conId, const ChipBleUU // Save a reference to the buffer until we get a indication from the ESP BLE layer that it // has been sent. - conState->PendingIndBuf = data; - data = NULL; + conState->PendingIndBuf = std::move(data); exit: if (err != CHIP_NO_ERROR) { ChipLogError(DeviceLayer, "BLEManagerImpl::SendIndication() failed: %s", ErrorStr(err)); - PacketBuffer::Free(data); return false; } return true; } bool BLEManagerImpl::SendWriteRequest(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId, - PacketBuffer * pBuf) + PacketBufferHandle pBuf) { ChipLogError(DeviceLayer, "BLEManagerImpl::SendWriteRequest() not supported"); return false; } bool BLEManagerImpl::SendReadRequest(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId, - PacketBuffer * pBuf) + PacketBufferHandle pBuf) { ChipLogError(DeviceLayer, "BLEManagerImpl::SendReadRequest() not supported"); return false; @@ -1069,8 +1067,7 @@ void BLEManagerImpl::HandleTXCharConfirm(CHIPoBLEConState * conState, esp_ble_ga param->conf.status); // If there is a pending indication buffer for the connection, release it now. - PacketBuffer::Free(conState->PendingIndBuf); - conState->PendingIndBuf = NULL; + conState->PendingIndBuf = nullptr; // If the confirmation was successful... if (param->conf.status == ESP_GATT_OK) @@ -1149,9 +1146,12 @@ BLEManagerImpl::CHIPoBLEConState * BLEManagerImpl::GetConnectionState(uint16_t c { if (freeIndex < kMaxConnections) { - memset(&mCons[freeIndex], 0, sizeof(CHIPoBLEConState)); - mCons[freeIndex].Allocated = 1; - mCons[freeIndex].ConId = conId; + mCons[freeIndex].PendingIndBuf = nullptr; + mCons[freeIndex].ConId = conId; + mCons[freeIndex].MTU = 0; + mCons[freeIndex].Allocated = 1; + mCons[freeIndex].Subscribed = 0; + mCons[freeIndex].Unused = 0; return &mCons[freeIndex]; } @@ -1167,8 +1167,8 @@ bool BLEManagerImpl::ReleaseConnectionState(uint16_t conId) { if (mCons[i].Allocated && mCons[i].ConId == conId) { - PacketBuffer::Free(mCons[i].PendingIndBuf); - mCons[i].Allocated = 0; + mCons[i].PendingIndBuf = nullptr; + mCons[i].Allocated = 0; return true; } } diff --git a/src/platform/ESP32/nimble/BLEManagerImpl.cpp b/src/platform/ESP32/nimble/BLEManagerImpl.cpp index 3c8ca5def44340..5ebfe16c931b29 100644 --- a/src/platform/ESP32/nimble/BLEManagerImpl.cpp +++ b/src/platform/ESP32/nimble/BLEManagerImpl.cpp @@ -325,7 +325,7 @@ uint16_t BLEManagerImpl::GetMTU(BLE_CONNECTION_OBJECT conId) const } bool BLEManagerImpl::SendIndication(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId, - PacketBuffer * data) + PacketBufferHandle data) { CHIP_ERROR err = CHIP_NO_ERROR; struct os_mbuf * om; @@ -353,21 +353,20 @@ bool BLEManagerImpl::SendIndication(BLE_CONNECTION_OBJECT conId, const ChipBleUU if (err != CHIP_NO_ERROR) { ChipLogError(DeviceLayer, "BLEManagerImpl::SendIndication() failed: %s", ErrorStr(err)); - PacketBuffer::Free(data); return false; } return true; } bool BLEManagerImpl::SendWriteRequest(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId, - PacketBuffer * pBuf) + PacketBufferHandle pBuf) { ChipLogError(DeviceLayer, "BLEManagerImpl::SendWriteRequest() not supported"); return false; } bool BLEManagerImpl::SendReadRequest(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId, - PacketBuffer * pBuf) + PacketBufferHandle pBuf) { ChipLogError(DeviceLayer, "BLEManagerImpl::SendReadRequest() not supported"); return false; diff --git a/src/platform/K32W/BLEManagerImpl.h b/src/platform/K32W/BLEManagerImpl.h index 2fbee62a87142c..e99aaae629b8f5 100644 --- a/src/platform/K32W/BLEManagerImpl.h +++ b/src/platform/K32W/BLEManagerImpl.h @@ -77,11 +77,11 @@ class BLEManagerImpl final : public BLEManager, bool CloseConnection(BLE_CONNECTION_OBJECT conId) override; uint16_t GetMTU(BLE_CONNECTION_OBJECT conId) const override; bool SendIndication(BLE_CONNECTION_OBJECT conId, const Ble::ChipBleUUID * svcId, const Ble::ChipBleUUID * charId, - System::PacketBuffer * pBuf) override; + System::PacketBufferHandle pBuf) override; bool SendWriteRequest(BLE_CONNECTION_OBJECT conId, const Ble::ChipBleUUID * svcId, const Ble::ChipBleUUID * charId, - System::PacketBuffer * pBuf) override; + System::PacketBufferHandle pBuf) override; bool SendReadRequest(BLE_CONNECTION_OBJECT conId, const Ble::ChipBleUUID * svcId, const Ble::ChipBleUUID * charId, - System::PacketBuffer * pBuf) override; + System::PacketBufferHandle pBuf) override; bool SendReadResponse(BLE_CONNECTION_OBJECT conId, BLE_READ_REQUEST_CONTEXT requestContext, const Ble::ChipBleUUID * svcId, const Ble::ChipBleUUID * charId) override; diff --git a/src/platform/Linux/BLEManagerImpl.cpp b/src/platform/Linux/BLEManagerImpl.cpp index 4cf41605e6aae6..e11f4085d15bcd 100644 --- a/src/platform/Linux/BLEManagerImpl.cpp +++ b/src/platform/Linux/BLEManagerImpl.cpp @@ -343,20 +343,20 @@ bool BLEManagerImpl::CloseConnection(BLE_CONNECTION_OBJECT conId) } bool BLEManagerImpl::SendIndication(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const Ble::ChipBleUUID * charId, - chip::System::PacketBuffer * pBuf) + chip::System::PacketBufferHandle pBuf) { - return SendBluezIndication(conId, pBuf); + return SendBluezIndication(conId, std::move(pBuf)); } bool BLEManagerImpl::SendWriteRequest(BLE_CONNECTION_OBJECT conId, const Ble::ChipBleUUID * svcId, const Ble::ChipBleUUID * charId, - chip::System::PacketBuffer * pBuf) + chip::System::PacketBufferHandle pBuf) { ChipLogError(Ble, "SendWriteRequest: Not implemented"); return true; } bool BLEManagerImpl::SendReadRequest(BLE_CONNECTION_OBJECT conId, const Ble::ChipBleUUID * svcId, const Ble::ChipBleUUID * charId, - chip::System::PacketBuffer * pBuf) + chip::System::PacketBufferHandle pBuf) { ChipLogError(Ble, "SendReadRequest: Not implemented"); return true; diff --git a/src/platform/Linux/BLEManagerImpl.h b/src/platform/Linux/BLEManagerImpl.h index e3d66af952238d..296748fb05648d 100644 --- a/src/platform/Linux/BLEManagerImpl.h +++ b/src/platform/Linux/BLEManagerImpl.h @@ -134,11 +134,11 @@ class BLEManagerImpl final : public BLEManager, bool CloseConnection(BLE_CONNECTION_OBJECT conId) override; uint16_t GetMTU(BLE_CONNECTION_OBJECT conId) const override; bool SendIndication(BLE_CONNECTION_OBJECT conId, const Ble::ChipBleUUID * svcId, const Ble::ChipBleUUID * charId, - System::PacketBuffer * pBuf) override; + System::PacketBufferHandle pBuf) override; bool SendWriteRequest(BLE_CONNECTION_OBJECT conId, const Ble::ChipBleUUID * svcId, const Ble::ChipBleUUID * charId, - System::PacketBuffer * pBuf) override; + System::PacketBufferHandle pBuf) override; bool SendReadRequest(BLE_CONNECTION_OBJECT conId, const Ble::ChipBleUUID * svcId, const Ble::ChipBleUUID * charId, - System::PacketBuffer * pBuf) override; + System::PacketBufferHandle pBuf) override; bool SendReadResponse(BLE_CONNECTION_OBJECT conId, BLE_READ_REQUEST_CONTEXT requestContext, const Ble::ChipBleUUID * svcId, const Ble::ChipBleUUID * charId) override; diff --git a/src/platform/Linux/CHIPBluezHelper.cpp b/src/platform/Linux/CHIPBluezHelper.cpp index dca1995851f4b8..8d786e7e83ac10 100644 --- a/src/platform/Linux/CHIPBluezHelper.cpp +++ b/src/platform/Linux/CHIPBluezHelper.cpp @@ -1445,7 +1445,7 @@ static gboolean BluezC2Indicate(void * apClosure) return G_SOURCE_REMOVE; } -bool SendBluezIndication(BLE_CONNECTION_OBJECT apConn, chip::System::PacketBuffer * apBuf) +bool SendBluezIndication(BLE_CONNECTION_OBJECT apConn, chip::System::PacketBufferHandle apBuf) { ConnectionDataBundle * closure; const char * msg = nullptr; @@ -1453,7 +1453,7 @@ bool SendBluezIndication(BLE_CONNECTION_OBJECT apConn, chip::System::PacketBuffe uint8_t * buffer = nullptr; size_t len = 0; - VerifyOrExit(apBuf != nullptr, ChipLogError(DeviceLayer, "apBuf is NULL in %s", __func__)); + VerifyOrExit(!apBuf.IsNull(), ChipLogError(DeviceLayer, "apBuf is NULL in %s", __func__)); buffer = apBuf->Start(); len = apBuf->DataLength(); @@ -1470,11 +1470,6 @@ bool SendBluezIndication(BLE_CONNECTION_OBJECT apConn, chip::System::PacketBuffe ChipLogError(Ble, msg); } - if (nullptr != apBuf) - { - chip::System::PacketBuffer::Free(apBuf); - } - return success; } diff --git a/src/platform/Linux/CHIPBluezHelper.h b/src/platform/Linux/CHIPBluezHelper.h index 090853997ca5d8..03ab1b2d825e6e 100644 --- a/src/platform/Linux/CHIPBluezHelper.h +++ b/src/platform/Linux/CHIPBluezHelper.h @@ -200,7 +200,7 @@ struct ConnectionDataBundle CHIP_ERROR InitBluezBleLayer(bool aIsCentral, char * apBleAddr, BLEAdvConfig & aBleAdvConfig, void *& apEndpoint); bool BluezRunOnBluezThread(int (*aCallback)(void *), void * apClosure); -bool SendBluezIndication(BLE_CONNECTION_OBJECT apConn, chip::System::PacketBuffer * apBuf); +bool SendBluezIndication(BLE_CONNECTION_OBJECT apConn, chip::System::PacketBufferHandle apBuf); bool CloseBluezConnection(BLE_CONNECTION_OBJECT apConn); CHIP_ERROR StartBluezAdv(BluezEndpoint * apEndpoint); CHIP_ERROR StopBluezAdv(BluezEndpoint * apEndpoint); diff --git a/src/platform/Zephyr/GenericBLEManagerImpl_Zephyr.cpp b/src/platform/Zephyr/GenericBLEManagerImpl_Zephyr.cpp index df7514c3362c7b..eb515eb1302bde 100644 --- a/src/platform/Zephyr/GenericBLEManagerImpl_Zephyr.cpp +++ b/src/platform/Zephyr/GenericBLEManagerImpl_Zephyr.cpp @@ -609,7 +609,7 @@ bool GenericBLEManagerImpl_Zephyr::UnsubscribeCharacteristic(BLE_CONN template bool GenericBLEManagerImpl_Zephyr::SendIndication(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, - const ChipBleUUID * charId, PacketBuffer * pBuf) + const ChipBleUUID * charId, PacketBufferHandle pBuf) { CHIP_ERROR err = CHIP_NO_ERROR; uint8_t index = bt_conn_index(conId); @@ -634,14 +634,12 @@ bool GenericBLEManagerImpl_Zephyr::SendIndication(BLE_CONNECTION_OBJE ChipLogError(DeviceLayer, "GenericBLEManagerImpl_Zephyr::SendIndication() failed: %s", ErrorStr(err)); } - PacketBuffer::Free(pBuf); - return err == CHIP_NO_ERROR; } template bool GenericBLEManagerImpl_Zephyr::SendWriteRequest(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, - const ChipBleUUID * charId, PacketBuffer * pBuf) + const ChipBleUUID * charId, PacketBufferHandle pBuf) { ChipLogError(DeviceLayer, "%s: NOT IMPLEMENTED", __PRETTY_FUNCTION__); return true; @@ -649,7 +647,7 @@ bool GenericBLEManagerImpl_Zephyr::SendWriteRequest(BLE_CONNECTION_OB template bool GenericBLEManagerImpl_Zephyr::SendReadRequest(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, - const ChipBleUUID * charId, PacketBuffer * pBuf) + const ChipBleUUID * charId, PacketBufferHandle pBuf) { ChipLogError(DeviceLayer, "%s: NOT IMPLEMENTED", __PRETTY_FUNCTION__); return true; diff --git a/src/platform/Zephyr/GenericBLEManagerImpl_Zephyr.h b/src/platform/Zephyr/GenericBLEManagerImpl_Zephyr.h index a068e67a142cd0..a3cb16fe738129 100644 --- a/src/platform/Zephyr/GenericBLEManagerImpl_Zephyr.h +++ b/src/platform/Zephyr/GenericBLEManagerImpl_Zephyr.h @@ -71,9 +71,12 @@ class GenericBLEManagerImpl_Zephyr : public BLEManager, bool UnsubscribeCharacteristic(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId); bool CloseConnection(BLE_CONNECTION_OBJECT conId); uint16_t GetMTU(BLE_CONNECTION_OBJECT conId) const; - bool SendIndication(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId, PacketBuffer * pBuf); - bool SendWriteRequest(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId, PacketBuffer * pBuf); - bool SendReadRequest(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId, PacketBuffer * pBuf); + bool SendIndication(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId, + PacketBufferHandle pBuf); + bool SendWriteRequest(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId, + PacketBufferHandle pBuf); + bool SendReadRequest(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId, + PacketBufferHandle pBuf); bool SendReadResponse(BLE_CONNECTION_OBJECT conId, BLE_READ_REQUEST_CONTEXT requestContext, const ChipBleUUID * svcId, const ChipBleUUID * charId); diff --git a/src/platform/qpg6100/BLEManagerImpl.cpp b/src/platform/qpg6100/BLEManagerImpl.cpp index f9b23300e6534d..c1e9cd2911996b 100644 --- a/src/platform/qpg6100/BLEManagerImpl.cpp +++ b/src/platform/qpg6100/BLEManagerImpl.cpp @@ -224,7 +224,7 @@ uint16_t BLEManagerImpl::GetMTU(BLE_CONNECTION_OBJECT conId) const } bool BLEManagerImpl::SendIndication(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId, - PacketBuffer * data) + PacketBufferHandle data) { CHIP_ERROR err = CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE; bool isRxHandle; @@ -240,7 +240,6 @@ bool BLEManagerImpl::SendIndication(BLE_CONNECTION_OBJECT conId, const ChipBleUU qvCHIP_TxData(conId, cId, dataLen, data->Start()); exit: - PacketBuffer::Free(data); if (err != CHIP_NO_ERROR) { ChipLogError(DeviceLayer, "BLEManagerImpl::SendIndication() failed: %s", ErrorStr(err)); @@ -250,14 +249,14 @@ bool BLEManagerImpl::SendIndication(BLE_CONNECTION_OBJECT conId, const ChipBleUU } bool BLEManagerImpl::SendWriteRequest(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId, - PacketBuffer * pBuf) + PacketBufferHandle pBuf) { ChipLogProgress(DeviceLayer, "BLEManagerImpl::SendWriteRequest() not supported"); return false; } bool BLEManagerImpl::SendReadRequest(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId, - PacketBuffer * pBuf) + PacketBufferHandle pBuf) { ChipLogProgress(DeviceLayer, "BLEManagerImpl::SendReadRequest() not supported"); return false; diff --git a/src/platform/qpg6100/BLEManagerImpl.h b/src/platform/qpg6100/BLEManagerImpl.h index 00a3f656b7037e..f24875e3561368 100644 --- a/src/platform/qpg6100/BLEManagerImpl.h +++ b/src/platform/qpg6100/BLEManagerImpl.h @@ -66,11 +66,11 @@ class BLEManagerImpl final : public BLEManager, private BleLayer, private BlePla bool CloseConnection(BLE_CONNECTION_OBJECT conId) override; uint16_t GetMTU(BLE_CONNECTION_OBJECT conId) const override; bool SendIndication(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId, - PacketBuffer * pBuf) override; + PacketBufferHandle pBuf) override; bool SendWriteRequest(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId, - PacketBuffer * pBuf) override; + PacketBufferHandle pBuf) override; bool SendReadRequest(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId, - PacketBuffer * pBuf) override; + PacketBufferHandle pBuf) override; bool SendReadResponse(BLE_CONNECTION_OBJECT conId, BLE_READ_REQUEST_CONTEXT requestContext, const ChipBleUUID * svcId, const ChipBleUUID * charId) override; diff --git a/src/protocols/echo/EchoClient.cpp b/src/protocols/echo/EchoClient.cpp index 3221f22e5786e2..a6389f41699139 100644 --- a/src/protocols/echo/EchoClient.cpp +++ b/src/protocols/echo/EchoClient.cpp @@ -75,7 +75,7 @@ CHIP_ERROR EchoClient::SendEchoRequest(System::PacketBufferHandle payload) CHIP_ERROR err = CHIP_NO_ERROR; // Send an Echo Request message. Discard the exchange context if the send fails. - err = mExchangeCtx->SendMessage(kProtocol_Echo, kEchoMessageType_EchoRequest, payload.Release_ForNow(), + err = mExchangeCtx->SendMessage(kProtocol_Echo, kEchoMessageType_EchoRequest, std::move(payload), Messaging::SendFlags(Messaging::SendMessageFlags::kSendFlag_None)); if (err != CHIP_NO_ERROR) diff --git a/src/protocols/echo/EchoServer.cpp b/src/protocols/echo/EchoServer.cpp index 29eed31ba19931..dedc22b9d1981a 100644 --- a/src/protocols/echo/EchoServer.cpp +++ b/src/protocols/echo/EchoServer.cpp @@ -78,7 +78,7 @@ void EchoServer::OnMessageReceived(Messaging::ExchangeContext * ec, const Packet response->EnsureReservedSize(CHIP_SYSTEM_CONFIG_HEADER_RESERVE_SIZE); // Send an Echo Response back to the sender. - ec->SendMessage(kProtocol_Echo, kEchoMessageType_EchoResponse, response.Release_ForNow(), + ec->SendMessage(kProtocol_Echo, kEchoMessageType_EchoResponse, std::move(response), Messaging::SendFlags(Messaging::SendMessageFlags::kSendFlag_None)); // Discard the exchange context. diff --git a/src/system/SystemPacketBuffer.cpp b/src/system/SystemPacketBuffer.cpp index 30d7556b2f3de3..dcc50ae701ee1a 100644 --- a/src/system/SystemPacketBuffer.cpp +++ b/src/system/SystemPacketBuffer.cpp @@ -236,7 +236,7 @@ uint16_t PacketBuffer::ReservedSize() const * * @param[in] aPacket - the packet buffer to be added to the end of the current chain. */ -void PacketBuffer::AddToEnd(PacketBuffer * aPacket) +void PacketBuffer::AddToEnd_ForNow(PacketBuffer * aPacket) { #if CHIP_SYSTEM_CONFIG_USE_LWIP pbuf_cat(this, aPacket); @@ -257,13 +257,18 @@ void PacketBuffer::AddToEnd(PacketBuffer * aPacket) #endif // !CHIP_SYSTEM_CONFIG_USE_LWIP } +void PacketBuffer::AddToEnd(PacketBufferHandle aPacket) +{ + AddToEnd_ForNow(aPacket.Release_ForNow()); +} + /** * Detach the current buffer from its chain and return a pointer to the remaining buffers. The current buffer must be the head of * the chain. * * @return the tail of the current buffer chain or NULL if the current buffer is the only buffer in the chain. */ -PacketBuffer * PacketBuffer::DetachTail() +PacketBuffer * PacketBuffer::DetachTail_ForNow() { PacketBuffer & lReturn = *static_cast(this->next); @@ -707,5 +712,19 @@ PacketBuffer * PacketBuffer::BuildFreeList() #endif // !CHIP_SYSTEM_CONFIG_USE_LWIP && CHIP_SYSTEM_CONFIG_PACKETBUFFER_MAXALLOC +PacketBufferHandle PacketBufferHandle::PopHead() +{ + PacketBuffer * head = mBuffer; + + // This takes ownership from the `next` link. + mBuffer = static_cast(mBuffer->next); + + head->next = nullptr; + head->tot_len = head->len; + + // The returned handle takes ownership from this. + return PacketBufferHandle(head); +} + } // namespace System } // namespace chip diff --git a/src/system/SystemPacketBuffer.h b/src/system/SystemPacketBuffer.h index 89e390bdad8789..32fc9d945c0501 100644 --- a/src/system/SystemPacketBuffer.h +++ b/src/system/SystemPacketBuffer.h @@ -121,8 +121,12 @@ class DLL_EXPORT PacketBuffer : private pbuf PacketBuffer * Next() const; - void AddToEnd(PacketBuffer * aPacket); - PacketBuffer * DetachTail(); + // The raw PacketBuffer version of AddToEnd() will be removed when conversion to PacketBufferHandle is complete. + void AddToEnd_ForNow(PacketBuffer * aPacket); + // The PacketBufferHandle's ownership is transferred to the `next` link at the end of the current chain. + void AddToEnd(PacketBufferHandle aPacket); + // The raw PacketBuffer version of DetachTail() will be removed when conversion to PacketBufferHandle is complete. + PacketBuffer * DetachTail_ForNow(); void CompactHead(); PacketBuffer * Consume(uint16_t aConsumeLength); void ConsumeHead(uint16_t aConsumeLength); @@ -140,7 +144,8 @@ class DLL_EXPORT PacketBuffer : private pbuf static PacketBuffer * RightSize(PacketBuffer * aPacket); static void Free(PacketBuffer * aPacket); - static PacketBuffer * FreeHead(PacketBuffer * aPacket); + // To be removed when conversion to PacketBufferHandle is complete: + static PacketBuffer * FreeHead_ForNow(PacketBuffer * aPacket) { return FreeHead(aPacket); } private: #if !CHIP_SYSTEM_CONFIG_USE_LWIP && CHIP_SYSTEM_CONFIG_PACKETBUFFER_MAXALLOC @@ -149,6 +154,7 @@ class DLL_EXPORT PacketBuffer : private pbuf static PacketBuffer * BuildFreeList(); #endif // !CHIP_SYSTEM_CONFIG_USE_LWIP && CHIP_SYSTEM_CONFIG_PACKETBUFFER_MAXALLOC + static PacketBuffer * FreeHead(PacketBuffer * aPacket); void Clear(); friend class PacketBufferHandle; }; @@ -311,11 +317,30 @@ class DLL_EXPORT PacketBufferHandle // This is intended to be used only to call functions that have not yet been converted to take a PacketBufferHandle; // a permanent version may be created if/when the need is clear. Most uses will be converted to take a // `const PacketBufferHandle &`. - PacketBuffer * Get_ForNow() { return mBuffer; } - const PacketBuffer * Get_ForNow() const { return mBuffer; } + PacketBuffer * Get_ForNow() const { return mBuffer; } bool IsNull() const { return mBuffer == nullptr; } + /** + * Detach and return the head of a buffer chain while updating this handle to point to the remaining buffers. + * The current buffer must be the head of the chain. + * + * @return the detached buffer formerly at the head of the buffer chain. + */ + PacketBufferHandle PopHead(); + + /** + * Free the first buffer in a chain. + * + * @note When the buffer chain is referenced by multiple callers, `FreeHead()` will detach the head, but will not forcibly + * deallocate the head buffer. + */ + void FreeHead() + { + // `PacketBuffer::FreeHead()` frees the current head; this takes ownership from the `next` link. + mBuffer = PacketBuffer::FreeHead(mBuffer); + } + private: PacketBufferHandle(const PacketBufferHandle &) = delete; PacketBufferHandle & operator=(const PacketBufferHandle &) = delete; diff --git a/src/system/tests/TestSystemPacketBuffer.cpp b/src/system/tests/TestSystemPacketBuffer.cpp index 288c9df1b48e64..27b53f583ee0e4 100644 --- a/src/system/tests/TestSystemPacketBuffer.cpp +++ b/src/system/tests/TestSystemPacketBuffer.cpp @@ -495,7 +495,7 @@ void CheckAddToEnd(nlTestSuite * inSuite, void * inContext) buffer_2 = PrepareTestBuffer(theSecondContext); buffer_3 = PrepareTestBuffer(theThirdContext); - buffer_1->AddToEnd(buffer_2); + buffer_1->AddToEnd_ForNow(buffer_2); NL_TEST_ASSERT(inSuite, theFirstContext->buf->tot_len == (theFirstContext->init_len + theSecondContext->init_len)); NL_TEST_ASSERT(inSuite, theFirstContext->buf->next == theSecondContext->buf); @@ -503,7 +503,7 @@ void CheckAddToEnd(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, theThirdContext->buf->next == nullptr); - buffer_1->AddToEnd(buffer_3); + buffer_1->AddToEnd_ForNow(buffer_3); NL_TEST_ASSERT(inSuite, theFirstContext->buf->tot_len == @@ -552,7 +552,7 @@ void CheckDetachTail(nlTestSuite * inSuite, void * inContext) theFirstContext->buf->tot_len = static_cast(theFirstContext->buf->tot_len + theSecondContext->init_len); } - returned = buffer_1->DetachTail(); + returned = buffer_1->DetachTail_ForNow(); NL_TEST_ASSERT(inSuite, theFirstContext->buf->next == nullptr); NL_TEST_ASSERT(inSuite, theFirstContext->buf->tot_len == theFirstContext->init_len); @@ -1113,7 +1113,7 @@ void CheckFreeHead(nlTestSuite * inSuite, void * inContext) theFirstContext->buf->next = theSecondContext->buf; - returned = PacketBuffer::FreeHead(buffer_1); + returned = PacketBuffer::FreeHead_ForNow(buffer_1); NL_TEST_ASSERT(inSuite, returned == buffer_2); diff --git a/src/transport/BLE.cpp b/src/transport/BLE.cpp index d07449938e8c59..ee0c8fb614efc4 100644 --- a/src/transport/BLE.cpp +++ b/src/transport/BLE.cpp @@ -154,7 +154,7 @@ CHIP_ERROR BLE::SendMessage(const PacketHeader & header, const Transport::PeerAd VerifyOrExit(headerSize == actualEncodedHeaderSize, err = CHIP_ERROR_INTERNAL); - err = mBleEndPoint->Send(msgBuf.Release_ForNow()); + err = mBleEndPoint->Send(std::move(msgBuf)); SuccessOrExit(err); exit: diff --git a/src/transport/NetworkProvisioning.cpp b/src/transport/NetworkProvisioning.cpp index 8417b857cb1d38..09a096d133d8f5 100644 --- a/src/transport/NetworkProvisioning.cpp +++ b/src/transport/NetworkProvisioning.cpp @@ -53,7 +53,7 @@ NetworkProvisioning::~NetworkProvisioning() #endif } -CHIP_ERROR NetworkProvisioning::HandleNetworkProvisioningMessage(uint8_t msgType, System::PacketBuffer * msgBuf) +CHIP_ERROR NetworkProvisioning::HandleNetworkProvisioningMessage(uint8_t msgType, const System::PacketBufferHandle & msgBuf) { CHIP_ERROR err = CHIP_NO_ERROR; @@ -247,7 +247,7 @@ CHIP_ERROR NetworkProvisioning::SendThreadCredentials(const DeviceLayer::Interna } #ifdef CHIP_ENABLE_OPENTHREAD -CHIP_ERROR NetworkProvisioning::DecodeThreadAssociationRequest(System::PacketBuffer * msgBuf) +CHIP_ERROR NetworkProvisioning::DecodeThreadAssociationRequest(const System::PacketBufferHandle & msgBuf) { CHIP_ERROR err = CHIP_NO_ERROR; DeviceLayer::Internal::DeviceNetworkInfo networkInfo = {}; @@ -321,7 +321,7 @@ CHIP_ERROR NetworkProvisioning::DecodeThreadAssociationRequest(System::PacketBuf return err; } #else // CHIP_ENABLE_OPENTHREAD -CHIP_ERROR NetworkProvisioning::DecodeThreadAssociationRequest(System::PacketBuffer *) +CHIP_ERROR NetworkProvisioning::DecodeThreadAssociationRequest(const System::PacketBufferHandle &) { return CHIP_ERROR_INVALID_MESSAGE_TYPE; } diff --git a/src/transport/NetworkProvisioning.h b/src/transport/NetworkProvisioning.h index aaed77055bb5ab..6da6acb3cf99e8 100644 --- a/src/transport/NetworkProvisioning.h +++ b/src/transport/NetworkProvisioning.h @@ -87,7 +87,7 @@ class DLL_EXPORT NetworkProvisioning CHIP_ERROR SendNetworkCredentials(const char * ssid, const char * passwd); CHIP_ERROR SendThreadCredentials(const DeviceLayer::Internal::DeviceNetworkInfo & threadData); - CHIP_ERROR HandleNetworkProvisioningMessage(uint8_t msgType, System::PacketBuffer * msgBuf); + CHIP_ERROR HandleNetworkProvisioningMessage(uint8_t msgType, const System::PacketBufferHandle & msgBuf); /** * @brief @@ -116,7 +116,7 @@ class DLL_EXPORT NetworkProvisioning CHIP_ERROR EncodeString(const char * str, BufBound & bbuf); CHIP_ERROR DecodeString(const uint8_t * input, size_t input_len, BufBound & bbuf, size_t & consumed); - CHIP_ERROR DecodeThreadAssociationRequest(System::PacketBuffer * msgBuf); + CHIP_ERROR DecodeThreadAssociationRequest(const System::PacketBufferHandle & msgBuf); #if CONFIG_DEVICE_LAYER static void ConnectivityHandler(const DeviceLayer::ChipDeviceEvent * event, intptr_t arg); diff --git a/src/transport/RendezvousSession.cpp b/src/transport/RendezvousSession.cpp index 33594687359bfc..7e1265ffea5dae 100644 --- a/src/transport/RendezvousSession.cpp +++ b/src/transport/RendezvousSession.cpp @@ -401,8 +401,7 @@ CHIP_ERROR RendezvousSession::HandleSecureMessage(const PacketHeader & packetHea if (payloadHeader.GetProtocolID() == Protocols::kProtocol_NetworkProvisioning) { - ReturnErrorOnFailure( - mNetworkProvision.HandleNetworkProvisioningMessage(payloadHeader.GetMessageType(), msgBuf.Get_ForNow())); + ReturnErrorOnFailure(mNetworkProvision.HandleNetworkProvisioningMessage(payloadHeader.GetMessageType(), msgBuf)); } return CHIP_NO_ERROR; diff --git a/src/transport/SecurePairingSession.cpp b/src/transport/SecurePairingSession.cpp index 5a9faef2add70f..59c114d4ba6df4 100644 --- a/src/transport/SecurePairingSession.cpp +++ b/src/transport/SecurePairingSession.cpp @@ -279,7 +279,7 @@ CHIP_ERROR SecurePairingSession::DeriveSecureSession(const uint8_t * info, size_ return err; } -CHIP_ERROR SecurePairingSession::HandleCompute_pA(const PacketHeader & header, System::PacketBuffer * msg) +CHIP_ERROR SecurePairingSession::HandleCompute_pA(const PacketHeader & header, const System::PacketBufferHandle & msg) { CHIP_ERROR err = CHIP_NO_ERROR; @@ -341,7 +341,7 @@ CHIP_ERROR SecurePairingSession::HandleCompute_pA(const PacketHeader & header, S return err; } -CHIP_ERROR SecurePairingSession::HandleCompute_pB_cB(const PacketHeader & header, System::PacketBuffer * msg) +CHIP_ERROR SecurePairingSession::HandleCompute_pB_cB(const PacketHeader & header, const System::PacketBufferHandle & msg) { CHIP_ERROR err = CHIP_NO_ERROR; @@ -400,7 +400,7 @@ CHIP_ERROR SecurePairingSession::HandleCompute_pB_cB(const PacketHeader & header return err; } -CHIP_ERROR SecurePairingSession::HandleCompute_cA(const PacketHeader & header, System::PacketBuffer * msg) +CHIP_ERROR SecurePairingSession::HandleCompute_cA(const PacketHeader & header, const System::PacketBufferHandle & msg) { CHIP_ERROR err = CHIP_NO_ERROR; const uint8_t * hash = msg->Start(); @@ -450,15 +450,15 @@ CHIP_ERROR SecurePairingSession::HandlePeerMessage(const PacketHeader & packetHe switch (static_cast(payloadHeader.GetMessageType())) { case Spake2pMsgType::kSpake2pCompute_pA: - err = HandleCompute_pA(packetHeader, msg.Get_ForNow()); + err = HandleCompute_pA(packetHeader, msg); break; case Spake2pMsgType::kSpake2pCompute_pB_cB: - err = HandleCompute_pB_cB(packetHeader, msg.Get_ForNow()); + err = HandleCompute_pB_cB(packetHeader, msg); break; case Spake2pMsgType::kSpake2pCompute_cA: - err = HandleCompute_cA(packetHeader, msg.Get_ForNow()); + err = HandleCompute_cA(packetHeader, msg); break; default: diff --git a/src/transport/SecurePairingSession.h b/src/transport/SecurePairingSession.h index 232709f65fed28..7693c2d94ecf9d 100644 --- a/src/transport/SecurePairingSession.h +++ b/src/transport/SecurePairingSession.h @@ -212,9 +212,9 @@ class DLL_EXPORT SecurePairingSession CHIP_ERROR Init(uint32_t setupCode, uint32_t pbkdf2IterCount, const uint8_t * salt, size_t saltLen, Optional myNodeId, uint16_t myKeyId, SecurePairingSessionDelegate * delegate); - CHIP_ERROR HandleCompute_pA(const PacketHeader & header, System::PacketBuffer * msg); - CHIP_ERROR HandleCompute_pB_cB(const PacketHeader & header, System::PacketBuffer * msg); - CHIP_ERROR HandleCompute_cA(const PacketHeader & header, System::PacketBuffer * msg); + CHIP_ERROR HandleCompute_pA(const PacketHeader & header, const System::PacketBufferHandle & msg); + CHIP_ERROR HandleCompute_pB_cB(const PacketHeader & header, const System::PacketBufferHandle & msg); + CHIP_ERROR HandleCompute_cA(const PacketHeader & header, const System::PacketBufferHandle & msg); CHIP_ERROR AttachHeaderAndSend(uint8_t msgType, System::PacketBuffer * msgBuf); diff --git a/src/transport/SecureSessionMgr.cpp b/src/transport/SecureSessionMgr.cpp index e9805bcdbd0f2f..f79cc2049a1766 100644 --- a/src/transport/SecureSessionMgr.cpp +++ b/src/transport/SecureSessionMgr.cpp @@ -85,22 +85,20 @@ CHIP_ERROR SecureSessionMgr::Init(NodeId localNodeId, System::Layer * systemLaye return err; } -CHIP_ERROR SecureSessionMgr::SendMessage(NodeId peerNodeId, System::PacketBuffer * msgBuf) +CHIP_ERROR SecureSessionMgr::SendMessage(NodeId peerNodeId, System::PacketBufferHandle msgBuf) { PayloadHeader payloadHeader; - return SendMessage(payloadHeader, peerNodeId, msgBuf); + return SendMessage(payloadHeader, peerNodeId, std::move(msgBuf)); } -CHIP_ERROR SecureSessionMgr::SendMessage(PayloadHeader & payloadHeader, NodeId peerNodeId, System::PacketBuffer * msgIn) +CHIP_ERROR SecureSessionMgr::SendMessage(PayloadHeader & payloadHeader, NodeId peerNodeId, System::PacketBufferHandle msgBuf) { - System::PacketBufferHandle msgBuf; CHIP_ERROR err = CHIP_NO_ERROR; PeerConnectionState * state = mPeerConnections.FindPeerConnectionState(peerNodeId, nullptr); VerifyOrExit(mState == State::kInitialized, err = CHIP_ERROR_INCORRECT_STATE); - msgBuf.Adopt(msgIn); VerifyOrExit(!msgBuf.IsNull(), err = CHIP_ERROR_INVALID_ARGUMENT); VerifyOrExit(msgBuf->Next() == nullptr, err = CHIP_ERROR_INVALID_MESSAGE_LENGTH); VerifyOrExit(msgBuf->TotalLength() < kMax_SecureSDU_Length, err = CHIP_ERROR_INVALID_MESSAGE_LENGTH); diff --git a/src/transport/SecureSessionMgr.h b/src/transport/SecureSessionMgr.h index 56d319338a0fef..ec7400e083be89 100644 --- a/src/transport/SecureSessionMgr.h +++ b/src/transport/SecureSessionMgr.h @@ -117,13 +117,9 @@ class DLL_EXPORT SecureSessionMgr : public Mdns::ResolveDelegate, public Transpo /** * @brief * Send a message to a currently connected peer - * - * @details - * This method calls chip::System::PacketBuffer::Free on - * behalf of the caller regardless of the return status. */ - CHIP_ERROR SendMessage(NodeId peerNodeId, System::PacketBuffer * msgBuf); - CHIP_ERROR SendMessage(PayloadHeader & payloadHeader, NodeId peerNodeId, System::PacketBuffer * msgBuf); + CHIP_ERROR SendMessage(NodeId peerNodeId, System::PacketBufferHandle msgBuf); + CHIP_ERROR SendMessage(PayloadHeader & payloadHeader, NodeId peerNodeId, System::PacketBufferHandle msgBuf); SecureSessionMgr(); ~SecureSessionMgr() override; diff --git a/src/transport/raw/TCP.cpp b/src/transport/raw/TCP.cpp index debc351e93e548..6fb6c0603fd34a 100644 --- a/src/transport/raw/TCP.cpp +++ b/src/transport/raw/TCP.cpp @@ -341,7 +341,7 @@ CHIP_ERROR TCPBase::ProcessReceivedBuffer(Inet::TCPEndPoint * endPoint, const Pe // when a buffer is empty, it can be released back to the app if (buffer->DataLength() == 0) { - buffer.Adopt(buffer->DetachTail()); + buffer.FreeHead(); continue; } @@ -396,7 +396,7 @@ CHIP_ERROR TCPBase::ProcessReceivedBuffer(Inet::TCPEndPoint * endPoint, const Pe if (!buffer.IsNull()) { // Incomplete processing will be retried - endPoint->PutBackReceivedData(buffer.Release_ForNow()); + endPoint->PutBackReceivedData(std::move(buffer)); } return err; diff --git a/src/transport/tests/TestSecureSessionMgr.cpp b/src/transport/tests/TestSecureSessionMgr.cpp index e068e3d6572e8f..7c72b78156e8e9 100644 --- a/src/transport/tests/TestSecureSessionMgr.cpp +++ b/src/transport/tests/TestSecureSessionMgr.cpp @@ -150,7 +150,7 @@ void CheckMessageTest(nlTestSuite * inSuite, void * inContext) // Should be able to send a message to itself by just calling send. callback.ReceiveHandlerCallCount = 0; - err = secureSessionMgr.SendMessage(kDestinationNodeId, buffer.Release_ForNow()); + err = secureSessionMgr.SendMessage(kDestinationNodeId, std::move(buffer)); NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); ctx.DriveIOUntil(1000 /* ms */, []() { return callback.ReceiveHandlerCallCount != 0; });