From 753b30520786e5671a7a5cd94340f62890de1f36 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Wed, 20 Sep 2023 14:17:45 -0700 Subject: [PATCH] [border-agent] simplify `ForwardContext` allocation and ownership (#9444) This commit updates `ForwardContext` to be `Heap::Allocatable` and uses `OwnerPtr` to simplify the ownership (and freeing) of allocated `ForwardContext` objects. --- src/core/meshcop/border_agent.cpp | 58 +++++++++++++++---------------- src/core/meshcop/border_agent.hpp | 15 ++++---- 2 files changed, 37 insertions(+), 36 deletions(-) diff --git a/src/core/meshcop/border_agent.cpp b/src/core/meshcop/border_agent.cpp index 0cdfac27a..d38d78d8a 100644 --- a/src/core/meshcop/border_agent.cpp +++ b/src/core/meshcop/border_agent.cpp @@ -41,6 +41,7 @@ #include "common/instance.hpp" #include "common/locator_getters.hpp" #include "common/log.hpp" +#include "common/owned_ptr.hpp" #include "common/settings.hpp" #include "meshcop/meshcop.hpp" #include "meshcop/meshcop_tlvs.hpp" @@ -56,10 +57,10 @@ RegisterLogModule("BorderAgent"); //---------------------------------------------------------------------------------------------------------------------- // `BorderAgent::ForwardContext` -void BorderAgent::ForwardContext::Init(Instance &aInstance, - const Coap::Message &aMessage, - bool aPetition, - bool aSeparate) +Error BorderAgent::ForwardContext::Init(Instance &aInstance, + const Coap::Message &aMessage, + bool aPetition, + bool aSeparate) { InstanceLocatorInit::Init(aInstance); mMessageId = aMessage.GetMessageId(); @@ -68,9 +69,11 @@ void BorderAgent::ForwardContext::Init(Instance &aInstance, mType = aMessage.GetType(); mTokenLength = aMessage.GetTokenLength(); memcpy(mToken, aMessage.GetToken(), mTokenLength); + + return kErrorNone; } -Error BorderAgent::ForwardContext::ToHeader(Coap::Message &aMessage, uint8_t aCode) +Error BorderAgent::ForwardContext::ToHeader(Coap::Message &aMessage, uint8_t aCode) const { if ((mType == Coap::kTypeNonConfirmable) || mSeparate) { @@ -114,7 +117,7 @@ Coap::Message::Code BorderAgent::CoapCodeFromError(Error aError) return code; } -void BorderAgent::SendErrorMessage(ForwardContext &aForwardContext, Error aError) +void BorderAgent::SendErrorMessage(const ForwardContext &aForwardContext, Error aError) { Error error = kErrorNone; Coap::Message *message = nullptr; @@ -170,12 +173,14 @@ void BorderAgent::HandleCoapResponse(void *aContext, { OT_UNUSED_VARIABLE(aMessageInfo); - ForwardContext &forwardContext = *static_cast(aContext); + OwnedPtr forwardContext(static_cast(aContext)); - forwardContext.Get().HandleCoapResponse(forwardContext, AsCoapMessagePtr(aMessage), aResult); + forwardContext->Get().HandleCoapResponse(*forwardContext.Get(), AsCoapMessagePtr(aMessage), aResult); } -void BorderAgent::HandleCoapResponse(ForwardContext &aForwardContext, const Coap::Message *aResponse, Error aResult) +void BorderAgent::HandleCoapResponse(const ForwardContext &aForwardContext, + const Coap::Message *aResponse, + Error aResult) { Coap::Message *message = nullptr; Error error; @@ -223,8 +228,6 @@ exit: SendErrorMessage(aForwardContext, error); } - - Heap::Free(&aForwardContext); } BorderAgent::BorderAgent(Instance &aInstance) @@ -516,12 +519,12 @@ exit: Error BorderAgent::ForwardToLeader(const Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo, Uri aUri) { - Error error = kErrorNone; - ForwardContext *forwardContext = nullptr; - Tmf::MessageInfo messageInfo(GetInstance()); - Coap::Message *message = nullptr; - bool petition = false; - bool separate = false; + Error error = kErrorNone; + OwnedPtr forwardContext; + Tmf::MessageInfo messageInfo(GetInstance()); + Coap::Message *message = nullptr; + bool petition = false; + bool separate = false; VerifyOrExit(mState != kStateStopped); @@ -543,10 +546,8 @@ Error BorderAgent::ForwardToLeader(const Coap::Message &aMessage, const Ip6::Mes SuccessOrExit(error = Get().SendAck(aMessage, aMessageInfo)); } - forwardContext = static_cast(Heap::CAlloc(1, sizeof(ForwardContext))); - VerifyOrExit(forwardContext != nullptr, error = kErrorNoBufs); - - forwardContext->Init(GetInstance(), aMessage, petition, separate); + forwardContext.Reset(ForwardContext::AllocateAndInit(GetInstance(), aMessage, petition, separate)); + VerifyOrExit(!forwardContext.IsNull(), error = kErrorNoBufs); message = Get().NewPriorityConfirmablePostMessage(aUri); VerifyOrExit(message != nullptr, error = kErrorNoBufs); @@ -557,10 +558,14 @@ Error BorderAgent::ForwardToLeader(const Coap::Message &aMessage, const Ip6::Mes SuccessOrExit(error = messageInfo.SetSockAddrToRlocPeerAddrToLeaderAloc()); messageInfo.SetSockPortToTmf(); - SuccessOrExit(error = Get().SendMessage(*message, messageInfo, HandleCoapResponse, forwardContext)); + SuccessOrExit(error = + Get().SendMessage(*message, messageInfo, HandleCoapResponse, forwardContext.Get())); - // HandleCoapResponse is responsible to free this forward context. - forwardContext = nullptr; + // Release the ownership of `forwardContext` since `SendMessage()` + // will own it. We take back ownership from `HandleCoapResponse()` + // callback. + + forwardContext.Release(); LogInfo("Forwarded request to leader on %s", PathForUri(aUri)); @@ -569,11 +574,6 @@ exit: if (error != kErrorNone) { - if (forwardContext != nullptr) - { - Heap::Free(forwardContext); - } - FreeMessage(message); SendErrorMessage(aMessage, separate, error); } diff --git a/src/core/meshcop/border_agent.hpp b/src/core/meshcop/border_agent.hpp index fadd87202..dd96b450a 100644 --- a/src/core/meshcop/border_agent.hpp +++ b/src/core/meshcop/border_agent.hpp @@ -41,6 +41,7 @@ #include #include "common/as_core_type.hpp" +#include "common/heap_allocatable.hpp" #include "common/locator.hpp" #include "common/non_copyable.hpp" #include "common/notifier.hpp" @@ -159,13 +160,13 @@ private: static constexpr uint16_t kUdpPort = OPENTHREAD_CONFIG_BORDER_AGENT_UDP_PORT; static constexpr uint32_t kKeepAliveTimeout = 50 * 1000; // Timeout to reject a commissioner (in msec) - class ForwardContext : public InstanceLocatorInit + class ForwardContext : public InstanceLocatorInit, public Heap::Allocatable { public: - void Init(Instance &aInstance, const Coap::Message &aMessage, bool aPetition, bool aSeparate); + Error Init(Instance &aInstance, const Coap::Message &aMessage, bool aPetition, bool aSeparate); bool IsPetition(void) const { return mPetition; } uint16_t GetMessageId(void) const { return mMessageId; } - Error ToHeader(Coap::Message &aMessage, uint8_t aCode); + Error ToHeader(Coap::Message &aMessage, uint8_t aCode) const; private: uint16_t mMessageId; // The CoAP Message ID of the original request. @@ -180,7 +181,7 @@ private: Coap::Message::Code CoapCodeFromError(Error aError); Error SendMessage(Coap::Message &aMessage); - void SendErrorMessage(ForwardContext &aForwardContext, Error aError); + void SendErrorMessage(const ForwardContext &aForwardContext, Error aError); void SendErrorMessage(const Coap::Message &aRequest, bool aSeparate, Error aError); static void HandleConnected(bool aConnected, void *aContext); @@ -194,9 +195,9 @@ private: otMessage *aMessage, const otMessageInfo *aMessageInfo, Error aResult); - void HandleCoapResponse(ForwardContext &aForwardContext, const Coap::Message *aResponse, Error aResult); - Error ForwardToLeader(const Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo, Uri aUri); - Error ForwardToCommissioner(Coap::Message &aForwardMessage, const Message &aMessage); + void HandleCoapResponse(const ForwardContext &aForwardContext, const Coap::Message *aResponse, Error aResult); + Error ForwardToLeader(const Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo, Uri aUri); + Error ForwardToCommissioner(Coap::Message &aForwardMessage, const Message &aMessage); static bool HandleUdpReceive(void *aContext, const otMessage *aMessage, const otMessageInfo *aMessageInfo); bool HandleUdpReceive(const Message &aMessage, const Ip6::MessageInfo &aMessageInfo);