From 65059ebbebc7267c7e811c1a27f9b5fe02471dfc Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Fri, 19 Dec 2025 09:31:45 -0800 Subject: [PATCH] [tmf] add overloads of `SendMessage` accepting `OwnedPtr` (#12217) This change introduces new overloads for `Coap::SendMessage()` that accept an `OwnedPtr`, transferring ownership of the message to the CoAP layer upon being called. The modules `BorderAgent` and `Commissioner` are updated to use this new method. The use of `OwnedPtr` simplifies the message allocation and cleanup. This removes the need for manual clean up calls(e.g., `FreeMessageOnError()`) and makes the code safer. --- src/core/coap/coap.cpp | 29 ++++++++++++++ src/core/coap/coap.hpp | 39 +++++++++++++++++++ src/core/meshcop/border_agent.cpp | 13 ++----- src/core/meshcop/commissioner.cpp | 63 ++++++++++++++----------------- 4 files changed, 101 insertions(+), 43 deletions(-) diff --git a/src/core/coap/coap.cpp b/src/core/coap/coap.cpp index 34f7391c3..f9d9eb537 100644 --- a/src/core/coap/coap.cpp +++ b/src/core/coap/coap.cpp @@ -372,11 +372,40 @@ Error CoapBase::SendMessage(Message &aMessage, #endif } +Error CoapBase::SendMessage(OwnedPtr aMessage, + const Ip6::MessageInfo &aMessageInfo, + ResponseHandler aHandler, + void *aContext) +{ + Error error; + + OT_ASSERT(aMessage != nullptr); + + SuccessOrExit(error = SendMessage(*aMessage, aMessageInfo, aHandler, aContext)); + aMessage.Release(); + +exit: + return error; +} + Error CoapBase::SendMessage(Message &aMessage, const Ip6::MessageInfo &aMessageInfo) { return SendMessage(aMessage, aMessageInfo, nullptr, nullptr); } +Error CoapBase::SendMessage(OwnedPtr aMessage, const Ip6::MessageInfo &aMessageInfo) +{ + Error error; + + OT_ASSERT(aMessage != nullptr); + + SuccessOrExit(error = SendMessage(*aMessage, aMessageInfo)); + aMessage.Release(); + +exit: + return error; +} + Error CoapBase::SendReset(Message &aRequest, const Ip6::MessageInfo &aMessageInfo) { return SendEmptyMessage(kTypeReset, aRequest, aMessageInfo); diff --git a/src/core/coap/coap.hpp b/src/core/coap/coap.hpp index 92cb9ae5b..09ea38c39 100644 --- a/src/core/coap/coap.hpp +++ b/src/core/coap/coap.hpp @@ -41,6 +41,7 @@ #include "common/locator.hpp" #include "common/message.hpp" #include "common/non_copyable.hpp" +#include "common/owned_ptr.hpp" #include "common/timer.hpp" #include "net/ip6.hpp" #include "net/netif.hpp" @@ -596,6 +597,7 @@ public: * @retval kErrorNoBufs Insufficient buffers available to send the CoAP message. */ Error SendMessage(Message &aMessage, const Ip6::MessageInfo &aMessageInfo, const TxParameters &aTxParameters); + /** * Sends a CoAP message with default transmission parameters. * @@ -614,6 +616,27 @@ public: ResponseHandler aHandler, void *aContext); + /** + * Sends a CoAP message with default transmission parameters. + * + * If Message ID was not set in the header (equal to 0), this method will assign unique Message ID to the message. + * + * This flavor of `SendMessage()` accepts an `OwnedPtr` and therefore takes ownership of the passed-in + * message. + * + * @param[in] aMessage An `OwnedPtr` to the message to send. + * @param[in] aMessageInfo A reference to the message info associated with @p aMessage. + * @param[in] aHandler A function pointer that shall be called on response reception or time-out. + * @param[in] aContext A pointer to arbitrary context information. + * + * @retval kErrorNone Successfully sent CoAP message. + * @retval kErrorNoBufs Insufficient buffers available to send the CoAP response. + */ + Error SendMessage(OwnedPtr aMessage, + const Ip6::MessageInfo &aMessageInfo, + ResponseHandler aHandler, + void *aContext); + /** * Sends a CoAP message with default transmission parameters. * @@ -627,6 +650,22 @@ public: */ Error SendMessage(Message &aMessage, const Ip6::MessageInfo &aMessageInfo); + /** + * Sends a CoAP message with default transmission parameters. + * + * If Message ID was not set in the header (equal to 0), this method will assign unique Message ID to the message. + * + * This flavor of `SendMessage()` accepts an `OwnedPtr` and therefore takes ownership of the passed-in + * message. + * + * @param[in] aMessage An `OwnedPtr` to the message to send. + * @param[in] aMessageInfo A reference to the message info associated with @p aMessage. + * + * @retval kErrorNone Successfully sent CoAP message. + * @retval kErrorNoBufs Insufficient buffers available to send the CoAP response. + */ + Error SendMessage(OwnedPtr aMessage, const Ip6::MessageInfo &aMessageInfo); + /** * Sends a CoAP reset message. * diff --git a/src/core/meshcop/border_agent.cpp b/src/core/meshcop/border_agent.cpp index a37e9187c..e8ddf7c23 100644 --- a/src/core/meshcop/border_agent.cpp +++ b/src/core/meshcop/border_agent.cpp @@ -497,8 +497,7 @@ Error Manager::EvictActiveCommissioner(void) messageInfo.SetSockAddrToRlocPeerAddrToLeaderAloc(); messageInfo.SetSockPortToTmf(); - SuccessOrExit(error = Get().SendMessage(*message, messageInfo)); - message.Release(); + error = Get().SendMessage(message.PassOwnership(), messageInfo); exit: return error; @@ -695,10 +694,8 @@ Error Manager::CoapDtlsSession::ForwardToLeader(const Coap::Message &aMessage messageInfo.SetSockAddrToRlocPeerAddrToLeaderAloc(); messageInfo.SetSockPortToTmf(); - // On success the message ownership is transferred. - SuccessOrExit(error = Get().SendMessage(*message, messageInfo, HandleLeaderResponseToFwdTmf, - forwardContext.Get())); - message.Release(); + SuccessOrExit(error = Get().SendMessage(message.PassOwnership(), messageInfo, + HandleLeaderResponseToFwdTmf, forwardContext.Get())); // Release the ownership of `forwardContext` since `SendMessage()` // will own it. We take back ownership when the callback @@ -982,9 +979,7 @@ void Manager::CoapDtlsSession::HandleTmfRelayTx(Coap::Message &aMessage) messageInfo.SetSockAddrToRlocPeerAddrTo(joinerRouterRloc); messageInfo.SetSockPortToTmf(); - // On success the message ownership is transferred. - SuccessOrExit(error = Get().SendMessage(*message, messageInfo)); - message.Release(); + SuccessOrExit(error = Get().SendMessage(message.PassOwnership(), messageInfo)); LogInfo("Forward %s to joiner router 0x%04x", UriToString(), joinerRouterRloc); diff --git a/src/core/meshcop/commissioner.cpp b/src/core/meshcop/commissioner.cpp index f9f3100c7..f4deb4d8c 100644 --- a/src/core/meshcop/commissioner.cpp +++ b/src/core/meshcop/commissioner.cpp @@ -603,12 +603,12 @@ void Commissioner::HandleJoinerExpirationTimer(void) Error Commissioner::SendMgmtCommissionerGetRequest(const uint8_t *aTlvs, uint8_t aLength) { - Error error = kErrorNone; - Coap::Message *message; - Tmf::MessageInfo messageInfo(GetInstance()); - Tlv tlv; + Error error = kErrorNone; + OwnedPtr message; + Tmf::MessageInfo messageInfo(GetInstance()); + Tlv tlv; - message = Get().NewPriorityConfirmablePostMessage(kUriCommissionerGet); + message.Reset(Get().NewPriorityConfirmablePostMessage(kUriCommissionerGet)); VerifyOrExit(message != nullptr, error = kErrorNoBufs); if (aLength > 0) @@ -620,13 +620,12 @@ Error Commissioner::SendMgmtCommissionerGetRequest(const uint8_t *aTlvs, uint8_t } messageInfo.SetSockAddrToRlocPeerAddrToLeaderAloc(); - SuccessOrExit(error = Get().SendMessage(*message, messageInfo, + SuccessOrExit(error = Get().SendMessage(message.PassOwnership(), messageInfo, Commissioner::HandleMgmtCommissionerGetResponse, this)); LogInfo("Sent %s to leader", UriToString()); exit: - FreeMessageOnError(message, error); return error; } @@ -643,11 +642,11 @@ Error Commissioner::SendMgmtCommissionerSetRequest(const CommissioningDataset &a const uint8_t *aTlvs, uint8_t aLength) { - Error error = kErrorNone; - Coap::Message *message; - Tmf::MessageInfo messageInfo(GetInstance()); + Error error = kErrorNone; + OwnedPtr message; + Tmf::MessageInfo messageInfo(GetInstance()); - message = Get().NewPriorityConfirmablePostMessage(kUriCommissionerSet); + message.Reset(Get().NewPriorityConfirmablePostMessage(kUriCommissionerSet)); VerifyOrExit(message != nullptr, error = kErrorNoBufs); if (aDataset.IsLocatorSet()) @@ -678,13 +677,12 @@ Error Commissioner::SendMgmtCommissionerSetRequest(const CommissioningDataset &a } messageInfo.SetSockAddrToRlocPeerAddrToLeaderAloc(); - SuccessOrExit(error = Get().SendMessage(*message, messageInfo, + SuccessOrExit(error = Get().SendMessage(message.PassOwnership(), messageInfo, Commissioner::HandleMgmtCommissionerSetResponse, this)); LogInfo("Sent %s to leader", UriToString()); exit: - FreeMessageOnError(message, error); return error; } @@ -706,25 +704,24 @@ exit: Error Commissioner::SendPetition(void) { - Error error = kErrorNone; - Coap::Message *message = nullptr; - Tmf::MessageInfo messageInfo(GetInstance()); + Error error = kErrorNone; + OwnedPtr message; + Tmf::MessageInfo messageInfo(GetInstance()); mTransmitAttempts++; - message = Get().NewPriorityConfirmablePostMessage(kUriLeaderPetition); + message.Reset(Get().NewPriorityConfirmablePostMessage(kUriLeaderPetition)); VerifyOrExit(message != nullptr, error = kErrorNoBufs); SuccessOrExit(error = Tlv::Append(*message, mCommissionerId)); messageInfo.SetSockAddrToRlocPeerAddrToLeaderAloc(); - SuccessOrExit( - error = Get().SendMessage(*message, messageInfo, Commissioner::HandleLeaderPetitionResponse, this)); + SuccessOrExit(error = Get().SendMessage(message.PassOwnership(), messageInfo, + Commissioner::HandleLeaderPetitionResponse, this)); LogInfo("Sent %s", UriToString()); exit: - FreeMessageOnError(message, error); return error; } @@ -779,11 +776,11 @@ void Commissioner::SendKeepAlive(void) { SendKeepAlive(mSessionId); } void Commissioner::SendKeepAlive(uint16_t aSessionId) { - Error error = kErrorNone; - Coap::Message *message = nullptr; - Tmf::MessageInfo messageInfo(GetInstance()); + Error error = kErrorNone; + OwnedPtr message; + Tmf::MessageInfo messageInfo(GetInstance()); - message = Get().NewPriorityConfirmablePostMessage(kUriLeaderKeepAlive); + message.Reset(Get().NewPriorityConfirmablePostMessage(kUriLeaderKeepAlive)); VerifyOrExit(message != nullptr, error = kErrorNoBufs); SuccessOrExit( @@ -792,13 +789,12 @@ void Commissioner::SendKeepAlive(uint16_t aSessionId) SuccessOrExit(error = Tlv::Append(*message, aSessionId)); messageInfo.SetSockAddrToRlocPeerAddrToLeaderAloc(); - SuccessOrExit(error = Get().SendMessage(*message, messageInfo, + SuccessOrExit(error = Get().SendMessage(message.PassOwnership(), messageInfo, Commissioner::HandleLeaderKeepAliveResponse, this)); LogInfo("Sent %s", UriToString()); exit: - FreeMessageOnError(message, error); LogWarnOnError(error, "send keep alive"); } @@ -996,15 +992,15 @@ Error Commissioner::SendRelayTransmit(Message &aMessage, const Ip6::MessageInfo { OT_UNUSED_VARIABLE(aMessageInfo); - Error error = kErrorNone; - ExtendedTlv tlv; - Coap::Message *message; - Tmf::MessageInfo messageInfo(GetInstance()); - Kek kek; + Error error = kErrorNone; + ExtendedTlv tlv; + OwnedPtr message; + Tmf::MessageInfo messageInfo(GetInstance()); + Kek kek; Get().ExtractKek(kek); - message = Get().NewPriorityNonConfirmablePostMessage(kUriRelayTx); + message.Reset(Get().NewPriorityNonConfirmablePostMessage(kUriRelayTx)); VerifyOrExit(message != nullptr, error = kErrorNoBufs); SuccessOrExit(error = Tlv::Append(*message, mJoinerPort)); @@ -1023,12 +1019,11 @@ Error Commissioner::SendRelayTransmit(Message &aMessage, const Ip6::MessageInfo messageInfo.SetSockAddrToRlocPeerAddrTo(mJoinerRloc); - SuccessOrExit(error = Get().SendMessage(*message, messageInfo)); + SuccessOrExit(error = Get().SendMessage(message.PassOwnership(), messageInfo)); aMessage.Free(); exit: - FreeMessageOnError(message, error); return error; }