From 0e2392bdc2a83e13b03267c2f2311a78c0936096 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Fri, 26 Nov 2021 22:30:26 -0800 Subject: [PATCH] [heap-allocatable] new method to allocate and init using constructor (#7201) This commit adds a new static method in `Heap::Allocatable` to allocate a new object and initialize it using the underlying `Type` constructor (passing all the parameters to the constructor). This is in addition to the existing mechanism to use an `Init()` method to initialize the allocated object. The `Init()` method returns an `Error` allowing the initialization itself to potentially fail. This commit also updates `Srp::Server` types `Host` and `UpdateMetadata` to use the new method. --- src/core/common/heap_allocatable.hpp | 26 +++++++++++++--- src/core/net/srp_server.cpp | 46 ++++++++++++---------------- src/core/net/srp_server.hpp | 14 ++++----- 3 files changed, 47 insertions(+), 39 deletions(-) diff --git a/src/core/common/heap_allocatable.hpp b/src/core/common/heap_allocatable.hpp index 66fab9a5b..1b8801631 100644 --- a/src/core/common/heap_allocatable.hpp +++ b/src/core/common/heap_allocatable.hpp @@ -47,7 +47,7 @@ namespace Heap { /** * This template class defines a `Heap::Allocatable` object. * - * `Heap::Allocatable` provides `New()` and `Free()` methods to allocate and free instances of `Type` on heap. + * `Heap::Allocatable` provides methods to allocate and free instances of `Type` on heap. * * Users of this class should follow CRTP-style inheritance, i.e., the `Type` class itself should inherit from * `Allocatable`. @@ -60,7 +60,25 @@ template class Allocatable { public: /** - * This static method allocates a new instance of `Type` on heap and initializes it. + * This static method allocates a new instance of `Type` on heap and initializes it using `Type` constructor. + * + * The `Type` class MUST have a constructor `Type(Args...)` which is invoked upon allocation of new `Type` to + * initialize it. + * + * @param[in] aArgs A set of arguments to pass to the `Type` constructor of the allocated `Type` instance. + * + * @returns A pointer to the newly allocated instance or `nullptr` if it fails to allocate. + * + */ + template static Type *Allocate(Args &&... aArgs) + { + void *buf = Heap::CAlloc(1, sizeof(Type)); + + return (buf != nullptr) ? new (buf) Type(static_cast(aArgs)...) : nullptr; + } + + /** + * This static method allocates a new instance of `Type` on heap and initializes it using `Type::Init()` method. * * The `Type` class MUST have a default constructor (with no arguments) which is invoked upon allocation of new * `Type` instance. It MUST also provide an `Error Init(Args...)` method to initialize the instance. If any `Error` @@ -71,7 +89,7 @@ public: * @returns A pointer to the newly allocated instance or `nullptr` if it fails to allocate or initialize. * */ - template static Type *New(Args &&... aArgs) + template static Type *AllocateAndInit(Args &&... aArgs) { void *buf = Heap::CAlloc(1, sizeof(Type)); Type *object = nullptr; @@ -93,7 +111,7 @@ public: /** * This method frees the `Type` instance. * - * The instance MUST be heap allocated using the `New()` method. + * The instance MUST be heap allocated using either `Allocate()` or `AllocateAndInit()`. * * The `Free()` method invokes the `Type` destructor before releasing the allocated heap buffer for the instance. * This ensures that any heap allocated member variables in `Type` are freed before the `Type` instance itself is diff --git a/src/core/net/srp_server.cpp b/src/core/net/srp_server.cpp index a23df2a68..db96e6dbd 100644 --- a/src/core/net/srp_server.cpp +++ b/src/core/net/srp_server.cpp @@ -679,7 +679,7 @@ void Server::ProcessDnsUpdate(Message &aMessage, MessageMetadata &aMetadata) // Per 2.3.2 of SRP draft 6, no prerequisites should be included in a SRP update. VerifyOrExit(aMetadata.mDnsHeader.GetPrerequisiteRecordCount() == 0, error = kErrorFailed); - host = Host::New(GetInstance(), aMetadata.mRxTime); + host = Host::Allocate(GetInstance(), aMetadata.mRxTime); VerifyOrExit(host != nullptr, error = kErrorNoBufs); SuccessOrExit(error = ProcessUpdateSection(*host, aMessage, aMetadata)); @@ -1155,7 +1155,7 @@ void Server::HandleUpdate(Host &aHost, const MessageMetadata &aMetadata) exit: if ((error == kErrorNone) && (mServiceUpdateHandler != nullptr)) { - UpdateMetadata *update = UpdateMetadata::New(GetInstance(), aHost, aMetadata); + UpdateMetadata *update = UpdateMetadata::Allocate(GetInstance(), aHost, aMetadata); mOutstandingUpdates.Push(*update); mOutstandingUpdatesTimer.FireAtIfEarlier(update->GetExpireTime()); @@ -1642,19 +1642,14 @@ exit: //--------------------------------------------------------------------------------------------------------------------- // Server::Host -Error Server::Host::Init(Instance &aInstance, TimeMilli aUpdateTime) +Server::Host::Host(Instance &aInstance, TimeMilli aUpdateTime) + : InstanceLocator(aInstance) + , mNext(nullptr) + , mLease(0) + , mKeyLease(0) + , mUpdateTime(aUpdateTime) { - InstanceLocatorInit::Init(aInstance); - mNext = nullptr; - mFullName.Free(); - mAddresses.Clear(); mKey.Clear(); - mLease = 0; - mKeyLease = 0; - mUpdateTime = aUpdateTime; - mServices.Clear(); - - return kErrorNone; } Server::Host::~Host(void) @@ -1746,11 +1741,11 @@ Server::Service *Server::Host::AddNewService(const char *aServiceName, if (desc == nullptr) { - desc.Reset(Service::Description::New(aInstanceName, *this)); + desc.Reset(Service::Description::AllocateAndInit(aInstanceName, *this)); VerifyOrExit(desc != nullptr); } - service = Service::New(aServiceName, *desc, aIsSubType, aUpdateTime); + service = Service::AllocateAndInit(aServiceName, *desc, aIsSubType, aUpdateTime); VerifyOrExit(service != nullptr); mServices.Push(*service); @@ -1924,23 +1919,20 @@ exit: //--------------------------------------------------------------------------------------------------------------------- // Server::UpdateMetadata -Error Server::UpdateMetadata::Init(Instance &aInstance, Host &aHost, const MessageMetadata &aMessageMetadata) +Server::UpdateMetadata::UpdateMetadata(Instance &aInstance, Host &aHost, const MessageMetadata &aMessageMetadata) + : InstanceLocator(aInstance) + , mNext(nullptr) + , mExpireTime(TimerMilli::GetNow() + kDefaultEventsHandlerTimeout) + , mDnsHeader(aMessageMetadata.mDnsHeader) + , mId(Get().AllocateId()) + , mLeaseConfig(aMessageMetadata.mLeaseConfig) + , mHost(aHost) + , mIsDirectRxFromClient(aMessageMetadata.IsDirectRxFromClient()) { - InstanceLocatorInit::Init(aInstance); - mNext = nullptr; - mExpireTime = TimerMilli::GetNow() + kDefaultEventsHandlerTimeout; - mDnsHeader = aMessageMetadata.mDnsHeader; - mId = Get().AllocateId(); - mLeaseConfig = aMessageMetadata.mLeaseConfig; - mHost = &aHost; - mIsDirectRxFromClient = aMessageMetadata.IsDirectRxFromClient(); - if (aMessageMetadata.mMessageInfo != nullptr) { mMessageInfo = *aMessageMetadata.mMessageInfo; } - - return kErrorNone; } } // namespace Srp diff --git a/src/core/net/srp_server.hpp b/src/core/net/srp_server.hpp index 49185b0a5..fa15cc617 100644 --- a/src/core/net/srp_server.hpp +++ b/src/core/net/srp_server.hpp @@ -393,7 +393,7 @@ public: * */ class Host : public otSrpServerHost, - public InstanceLocatorInit, + public InstanceLocator, public LinkedListEntry, private Heap::Allocatable, private NonCopyable @@ -514,10 +514,9 @@ public: private: static constexpr uint16_t kMaxAddresses = OPENTHREAD_CONFIG_SRP_SERVER_MAX_ADDRESSES_NUM; - Host(void) = default; + Host(Instance &aInstance, TimeMilli aUpdateTime); ~Host(void); - Error Init(Instance &aInstance, TimeMilli aUpdateTime); Error SetFullName(const char *aFullName); void SetKey(Dns::Ecdsa256KeyRecord &aKey); void SetLease(uint32_t aLease) { mLease = aLease; } @@ -786,7 +785,7 @@ private: // This class includes metadata for processing a SRP update (register, deregister) // and sending DNS response to the client. - class UpdateMetadata : public InstanceLocatorInit, + class UpdateMetadata : public InstanceLocator, public LinkedListEntry, public Heap::Allocatable { @@ -794,25 +793,24 @@ private: friend class Heap::Allocatable; public: - Error Init(Instance &aInstance, Host &aHost, const MessageMetadata &aMessageMetadata); TimeMilli GetExpireTime(void) const { return mExpireTime; } const Dns::UpdateHeader &GetDnsHeader(void) const { return mDnsHeader; } ServiceUpdateId GetId(void) const { return mId; } const LeaseConfig & GetLeaseConfig(void) const { return mLeaseConfig; } - Host & GetHost(void) { return *mHost; } + Host & GetHost(void) { return mHost; } const Ip6::MessageInfo & GetMessageInfo(void) const { return mMessageInfo; } bool IsDirectRxFromClient(void) const { return mIsDirectRxFromClient; } bool Matches(ServiceUpdateId aId) const { return mId == aId; } private: - UpdateMetadata(void) = default; + UpdateMetadata(Instance &aInstance, Host &aHost, const MessageMetadata &aMessageMetadata); UpdateMetadata * mNext; TimeMilli mExpireTime; Dns::UpdateHeader mDnsHeader; ServiceUpdateId mId; // The ID of this service update transaction. LeaseConfig mLeaseConfig; // Lease config to use when processing the message. - Host * mHost; // The `UpdateMetadata` has no ownership of this host. + Host & mHost; // The `UpdateMetadata` has no ownership of this host. Ip6::MessageInfo mMessageInfo; // Valid when `mIsDirectRxFromClient` is true. bool mIsDirectRxFromClient; };