From dd39621968a1fab20435102872803abdec2f64ea Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Mon, 8 Nov 2021 14:47:30 -0800 Subject: [PATCH] [srp-server] simplify calls to `RemoveHost()` & `RemoveService()` (#7136) This commit adds `enum` types to use as arguments `aRetainName` and `aNotifyServiceHandler` in the calls to method `RemoveHost()` and `Host::RemoveService()`. --- src/core/net/srp_server.cpp | 28 +++++++++++++------------- src/core/net/srp_server.hpp | 40 ++++++++++++++++++++++++------------- 2 files changed, 40 insertions(+), 28 deletions(-) diff --git a/src/core/net/srp_server.cpp b/src/core/net/srp_server.cpp index 608e2a2fa..0e008ac71 100644 --- a/src/core/net/srp_server.cpp +++ b/src/core/net/srp_server.cpp @@ -263,7 +263,7 @@ void Server::AddHost(Host &aHost) IgnoreError(mHosts.Add(aHost)); } -void Server::RemoveHost(Host *aHost, bool aRetainName, bool aNotifyServiceHandler) +void Server::RemoveHost(Host *aHost, RetainName aRetainName, NotifyMode aNotifyServiceHandler) { VerifyOrExit(aHost != nullptr); @@ -400,16 +400,16 @@ void Server::CommitSrpUpdate(Error aError, if (aHost.GetKeyLease() == 0) { otLogInfoSrp("[server] remove key of host %s", aHost.GetFullName()); - RemoveHost(existingHost, /* aRetainName */ false, /* aNotifyServiceHandler */ false); + RemoveHost(existingHost, kDeleteName, kDoNotNotifyServiceHandler); } else if (existingHost != nullptr) { existingHost->SetKeyLease(aHost.GetKeyLease()); - RemoveHost(existingHost, /* aRetainName */ true, /* aNotifyServiceHandler */ false); + RemoveHost(existingHost, kRetainName, kDoNotNotifyServiceHandler); for (Service &service : existingHost->mServices) { - existingHost->RemoveService(&service, /* aRetainName */ true, /* aNotifyServiceHandler */ false); + existingHost->RemoveService(&service, kRetainName, kDoNotNotifyServiceHandler); } } } @@ -583,7 +583,7 @@ void Server::Stop(void) while (!mHosts.IsEmpty()) { - RemoveHost(mHosts.GetHead(), /* aRetainName */ false, /* aNotifyServiceHandler */ true); + RemoveHost(mHosts.GetHead(), kDeleteName, kNotifyServiceHandler); } // TODO: We should cancel any outstanding service updates, but current @@ -1307,7 +1307,7 @@ void Server::HandleLeaseTimer(void) otLogInfoSrp("[server] KEY LEASE of host %s expired", host->GetFullName()); // Removes the whole host and all services if the KEY RR expired. - RemoveHost(host, /* aRetainName */ false, /* aNotifyServiceHandler */ true); + RemoveHost(host, kDeleteName, kNotifyServiceHandler); } else if (host->IsDeleted()) { @@ -1327,7 +1327,7 @@ void Server::HandleLeaseTimer(void) if (service->GetKeyExpireTime() <= now) { service->Log(Service::kKeyLeaseExpired); - host->RemoveService(service, /* aRetainName */ false, /* aNotifyServiceHandler */ true); + host->RemoveService(service, kDeleteName, kNotifyServiceHandler); } else { @@ -1343,10 +1343,10 @@ void Server::HandleLeaseTimer(void) for (Service &service : host->mServices) { // Don't need to notify the service handler as `RemoveHost` at below will do. - host->RemoveService(&service, /* aRetainName */ true, /* aNotifyServiceHandler */ false); + host->RemoveService(&service, kRetainName, kDoNotNotifyServiceHandler); } - RemoveHost(host, /* aRetainName */ true, /* aNotifyServiceHandler */ true); + RemoveHost(host, kRetainName, kNotifyServiceHandler); earliestExpireTime = OT_MIN(earliestExpireTime, host->GetKeyExpireTime()); } @@ -1367,7 +1367,7 @@ void Server::HandleLeaseTimer(void) if (service->GetKeyExpireTime() <= now) { service->Log(Service::kKeyLeaseExpired); - host->RemoveService(service, /* aRetainName */ false, /* aNotifyServiceHandler */ true); + host->RemoveService(service, kDeleteName, kNotifyServiceHandler); } else if (service->mIsDeleted) { @@ -1379,7 +1379,7 @@ void Server::HandleLeaseTimer(void) service->Log(Service::kLeaseExpired); // The service is expired, delete it. - host->RemoveService(service, /* aRetainName */ true, /* aNotifyServiceHandler */ true); + host->RemoveService(service, kRetainName, kNotifyServiceHandler); earliestExpireTime = OT_MIN(earliestExpireTime, service->GetKeyExpireTime()); } else @@ -1804,7 +1804,7 @@ exit: return service; } -void Server::Host::RemoveService(Service *aService, bool aRetainName, bool aNotifyServiceHandler) +void Server::Host::RemoveService(Service *aService, RetainName aRetainName, NotifyMode aNotifyServiceHandler) { Server &server = Get(); @@ -1842,7 +1842,7 @@ void Server::Host::FreeAllServices(void) { while (!mServices.IsEmpty()) { - RemoveService(mServices.GetHead(), /* aRetainName */ false, /* aNotifyServiceHandler */ false); + RemoveService(mServices.GetHead(), kDeleteName, kDoNotNotifyServiceHandler); } } @@ -1902,7 +1902,7 @@ Error Server::Host::MergeServicesAndResourcesFrom(Host &aHost) if (service.mIsDeleted) { // `RemoveService()` does nothing if `exitsingService` is `nullptr`. - RemoveService(existingService, /* aRetainName */ true, /* aNotifyServiceHandler */ false); + RemoveService(existingService, kRetainName, kDoNotNotifyServiceHandler); continue; } diff --git a/src/core/net/srp_server.hpp b/src/core/net/srp_server.hpp index 7864ac7fa..201c5035b 100644 --- a/src/core/net/srp_server.hpp +++ b/src/core/net/srp_server.hpp @@ -98,6 +98,18 @@ class Server : public InstanceLocator, private NonCopyable friend class Host; friend class Dns::ServiceDiscovery::Server; + enum RetainName : bool + { + kDeleteName = false, + kRetainName = true, + }; + + enum NotifyMode : bool + { + kDoNotNotifyServiceHandler = false, + kNotifyServiceHandler = true, + }; + public: static constexpr uint16_t kUdpPortMin = OPENTHREAD_CONFIG_SRP_SERVER_UDP_PORT_MIN; ///< The reserved min port. static constexpr uint16_t kUdpPortMax = OPENTHREAD_CONFIG_SRP_SERVER_UDP_PORT_MAX; ///< The reserved max port. @@ -498,19 +510,19 @@ public: static Host *New(Instance &aInstance); explicit Host(Instance &aInstance); - void Free(void); - Error SetFullName(const char *aFullName); - void SetKey(Dns::Ecdsa256KeyRecord &aKey); - void SetLease(uint32_t aLease) { mLease = aLease; } - void SetKeyLease(uint32_t aKeyLease) { mKeyLease = aKeyLease; } - LinkedList & GetServices(void) { return mServices; } - Service * AddNewService(const char *aServiceName, const char *aInstanceName, bool aIsSubType); - void RemoveService(Service *aService, bool aRetainName, bool aNotifyServiceHandler); - void FreeAllServices(void); - void FreeUnusedServiceDescriptions(void); - void ClearResources(void); - Error MergeServicesAndResourcesFrom(Host &aHost); - Error AddIp6Address(const Ip6::Address &aIp6Address); + void Free(void); + Error SetFullName(const char *aFullName); + void SetKey(Dns::Ecdsa256KeyRecord &aKey); + void SetLease(uint32_t aLease) { mLease = aLease; } + void SetKeyLease(uint32_t aKeyLease) { mKeyLease = aKeyLease; } + LinkedList &GetServices(void) { return mServices; } + Service * AddNewService(const char *aServiceName, const char *aInstanceName, bool aIsSubType); + void RemoveService(Service *aService, RetainName aRetainName, NotifyMode aNotifyServiceHandler); + void FreeAllServices(void); + void FreeUnusedServiceDescriptions(void); + void ClearResources(void); + Error MergeServicesAndResourcesFrom(Host &aHost); + Error AddIp6Address(const Ip6::Address &aIp6Address); Service::Description * FindServiceDescription(const char *aInstanceName); const Service::Description *FindServiceDescription(const char *aInstanceName) const; Service * FindService(const char *aServiceName, const char *aInstanceName); @@ -847,7 +859,7 @@ private: void HandleUpdate(const Dns::UpdateHeader &aDnsHeader, Host &aHost, const Ip6::MessageInfo &aMessageInfo); void AddHost(Host &aHost); - void RemoveHost(Host *aHost, bool aRetainName, bool aNotifyServiceHandler); + void RemoveHost(Host *aHost, RetainName aRetainName, NotifyMode aNotifyServiceHandler); bool HasNameConflictsWith(Host &aHost) const; void SendResponse(const Dns::UpdateHeader & aHeader, Dns::UpdateHeader::Response aResponseCode,