Skip to content

Commit

Permalink
[owning-list] add RemoveAndFreeAllMatching() to OwningList class (o…
Browse files Browse the repository at this point in the history
…penthread#10210)

This commit introduces a new method, `RemoveAndFreeAllMatching()`, to
the `OwningList<Type>` class. This method removes and frees all
entries in the list that match a given indicator. This is used to
simplify the native mDNS implementation. The unit test
`test_linked_list` is also updated to validate the newly added helper
method.
  • Loading branch information
abtink authored May 8, 2024
1 parent 00f7c31 commit 226f239
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 36 deletions.
23 changes: 23 additions & 0 deletions src/core/common/owning_list.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,29 @@ template <typename Type> class OwningList : public LinkedList<Type>
{
LinkedList<Type>::RemoveAllMatching(aIndicator, aRemovedList);
}

/**
* Removes and frees all entries in the list matching a given entry indicator.
*
* The template type `Indicator` specifies the type of @p aIndicator object which is used to match against entries
* in the list. To check that an entry matches the given indicator, the `Matches()` method is invoked on each
* `Type` entry in the list. The `Matches()` method should be provided by `Type` class accordingly:
*
* bool Type::Matches(const Indicator &aIndicator) const
*
* @param[in] aIndicator An entry indicator to match against entries in the list.
*
* @retval TRUE At least one matching entry was removed.
* @retval FALSE No matching entry was found.
*
*/
template <typename Indicator> bool RemoveAndFreeAllMatching(const Indicator &aIndicator)
{
OwningList removedList;

RemoveAllMatching(aIndicator, removedList);
return !removedList.IsEmpty();
}
};

} // namespace ot
Expand Down
55 changes: 19 additions & 36 deletions src/core/net/mdns.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -313,11 +313,8 @@ void Core::HandleEntryTimer(void)

void Core::RemoveEmptyEntries(void)
{
OwningList<HostEntry> removedHosts;
OwningList<ServiceEntry> removedServices;

mHostEntries.RemoveAllMatching(Entry::kRemoving, removedHosts);
mServiceEntries.RemoveAllMatching(Entry::kRemoving, removedServices);
mHostEntries.RemoveAndFreeAllMatching(Entry::kRemoving);
mServiceEntries.RemoveAndFreeAllMatching(Entry::kRemoving);
}

void Core::HandleEntryTask(void)
Expand Down Expand Up @@ -1874,9 +1871,7 @@ void Core::ServiceEntry::ClearService(void)

void Core::ServiceEntry::ScheduleToRemoveIfEmpty(void)
{
OwningList<SubType> removedSubTypes;

mSubTypes.RemoveAllMatching(EmptyChecker(), removedSubTypes);
mSubTypes.RemoveAndFreeAllMatching(EmptyChecker());

if (IsEmpty())
{
Expand Down Expand Up @@ -2149,8 +2144,6 @@ void Core::ServiceEntry::PrepareResponseRecords(TxMessage &aResponse, TimeMilli

void Core::ServiceEntry::UpdateRecordsState(const TxMessage &aResponse)
{
OwningList<SubType> removedSubTypes;

Entry::UpdateRecordsState(aResponse);

mPtrRecord.UpdateStateAfterAnswer(aResponse);
Expand All @@ -2162,7 +2155,7 @@ void Core::ServiceEntry::UpdateRecordsState(const TxMessage &aResponse)
subType.mPtrRecord.UpdateStateAfterAnswer(aResponse);
}

mSubTypes.RemoveAllMatching(EmptyChecker(), removedSubTypes);
mSubTypes.RemoveAndFreeAllMatching(EmptyChecker());

if (IsEmpty())
{
Expand Down Expand Up @@ -4142,11 +4135,10 @@ void Core::TxMessageHistory::CalculateHash(const Message &aMessage, Hash &aHash)

void Core::TxMessageHistory::HandleTimer(void)
{
TimeMilli now = TimerMilli::GetNow();
TimeMilli nextTime = now.GetDistantFuture();
OwningList<HashEntry> expiredEntries;
TimeMilli now = TimerMilli::GetNow();
TimeMilli nextTime = now.GetDistantFuture();

mHashEntries.RemoveAllMatching(ExpireChecker(now), expiredEntries);
mHashEntries.RemoveAndFreeAllMatching(ExpireChecker(now));

for (const HashEntry &entry : mHashEntries)
{
Expand Down Expand Up @@ -4268,21 +4260,16 @@ void Core::AddPassiveIp6AddrCache(const char *aHostName)

void Core::HandleCacheTimer(void)
{
CacheTimerContext context(GetInstance());
ExpireChecker expireChecker(context.GetNow());
OwningList<BrowseCache> expiredBrowseList;
OwningList<SrvCache> expiredSrvList;
OwningList<TxtCache> expiredTxtList;
OwningList<Ip6AddrCache> expiredIp6AddrList;
OwningList<Ip4AddrCache> expiredIp4AddrList;
CacheTimerContext context(GetInstance());
ExpireChecker expireChecker(context.GetNow());

// First remove all expired entries.

mBrowseCacheList.RemoveAllMatching(expireChecker, expiredBrowseList);
mSrvCacheList.RemoveAllMatching(expireChecker, expiredSrvList);
mTxtCacheList.RemoveAllMatching(expireChecker, expiredTxtList);
mIp6AddrCacheList.RemoveAllMatching(expireChecker, expiredIp6AddrList);
mIp4AddrCacheList.RemoveAllMatching(expireChecker, expiredIp4AddrList);
mBrowseCacheList.RemoveAndFreeAllMatching(expireChecker);
mSrvCacheList.RemoveAndFreeAllMatching(expireChecker);
mTxtCacheList.RemoveAndFreeAllMatching(expireChecker);
mIp6AddrCacheList.RemoveAndFreeAllMatching(expireChecker);
mIp4AddrCacheList.RemoveAndFreeAllMatching(expireChecker);

// Process cache types in a specific order to optimize name
// compression when constructing query messages.
Expand Down Expand Up @@ -4720,9 +4707,7 @@ void Core::CacheEntry::Remove(const ResultCallback &aCallback)

void Core::CacheEntry::ClearEmptyCallbacks(void)
{
CallbackList emptyCallbacks;

mCallbacks.RemoveAllMatching(EmptyChecker(), emptyCallbacks);
mCallbacks.RemoveAndFreeAllMatching(EmptyChecker());

if (mCallbacks.IsEmpty())
{
Expand Down Expand Up @@ -5765,13 +5750,13 @@ void Core::AddrCache::DetermineRecordFireTime(void)

void Core::AddrCache::ProcessExpiredRecords(TimeMilli aNow)
{
OwningList<AddrEntry> expiredEntries;
Heap::Array<AddressAndTtl> addrArray;
AddressResult result;
bool didRemoveAny;

mCommittedEntries.RemoveAllMatching(ExpireChecker(aNow), expiredEntries);
didRemoveAny = mCommittedEntries.RemoveAndFreeAllMatching(ExpireChecker(aNow));

VerifyOrExit(!expiredEntries.IsEmpty());
VerifyOrExit(didRemoveAny);

ConstructResult(result, addrArray);
InvokeCallbacks(result);
Expand Down Expand Up @@ -5963,9 +5948,7 @@ void Core::AddrCache::CommitNewResponseEntries(void)
}
else
{
OwningList<AddrEntry> removedEntries;

mCommittedEntries.RemoveAllMatching(EmptyChecker(), removedEntries);
mCommittedEntries.RemoveAndFreeAllMatching(EmptyChecker());
}

while (!mNewEntries.IsEmpty())
Expand Down
37 changes: 37 additions & 0 deletions tests/unit/test_linked_list.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,7 @@ void TestOwningList(void)
OwningList<Entry> list;
OwningList<Entry> removedList;
OwnedPtr<Entry> ptr;
bool didRemove;

printf("TestOwningList\n");

Expand Down Expand Up @@ -433,6 +434,42 @@ void TestOwningList(void)
VerifyOrQuit(list.IsEmpty());
VerifyLinkedListContent(&removedList, &c, &d, &f, nullptr);
VerifyOrQuit(!c.WasFreed());

// Test `RemoveAndFreeAllMatching()`

a.ResetTestFlags();
b.ResetTestFlags();
c.ResetTestFlags();
d.ResetTestFlags();
e.ResetTestFlags();
f.ResetTestFlags();
list.Push(a);
list.Push(b);
list.Push(c);
list.Push(d);
list.Push(e);
list.Push(f);
VerifyLinkedListContent(&list, &f, &e, &d, &c, &b, &a, nullptr);

didRemove = list.RemoveAndFreeAllMatching(kAlphaType);
VerifyLinkedListContent(&list, &f, &d, &c, nullptr);
VerifyOrQuit(didRemove);
VerifyOrQuit(a.WasFreed());
VerifyOrQuit(b.WasFreed());
VerifyOrQuit(e.WasFreed());
VerifyOrQuit(!c.WasFreed());

didRemove = list.RemoveAndFreeAllMatching(kAlphaType);
VerifyOrQuit(!didRemove);
VerifyLinkedListContent(&list, &f, &d, &c, nullptr);
VerifyOrQuit(!c.WasFreed());

didRemove = list.RemoveAndFreeAllMatching(kBetaType);
VerifyOrQuit(list.IsEmpty());
VerifyOrQuit(didRemove);
VerifyOrQuit(c.WasFreed());
VerifyOrQuit(d.WasFreed());
VerifyOrQuit(f.WasFreed());
}

} // namespace ot
Expand Down

0 comments on commit 226f239

Please sign in to comment.