From 4e8836cbece3359751e528dfe2c3d2c799614767 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Wed, 15 Jul 2020 23:07:16 -0700 Subject: [PATCH] [child-table] enable use of range-based "for" loop (#5239) This commit updates `ChildTable` class. It mainly adds support for using range-based `for` loops to iterate over all child entries (matching a given state filter). It also simplifies the `Iterator` implementation (removing the now unused behavior to start the iteration from a specific child in the table). The unit test `test_child_table` is also updated to add tests for the newly added behavior. --- src/core/mac/data_poll_handler.cpp | 13 +- src/core/thread/address_resolver.cpp | 8 +- src/core/thread/child_table.cpp | 29 +---- src/core/thread/child_table.hpp | 118 ++++++++++++++----- src/core/thread/indirect_sender.cpp | 12 +- src/core/thread/key_manager.cpp | 8 +- src/core/thread/mesh_forwarder.cpp | 4 +- src/core/thread/mesh_forwarder_ftd.cpp | 14 +-- src/core/thread/mle_router.cpp | 57 ++++----- src/core/thread/network_diagnostic.cpp | 4 +- src/core/thread/src_match_controller.cpp | 8 +- src/core/utils/child_supervision.cpp | 4 +- tests/unit/test_child_table.cpp | 144 +++++++++++------------ 13 files changed, 210 insertions(+), 213 deletions(-) diff --git a/src/core/mac/data_poll_handler.cpp b/src/core/mac/data_poll_handler.cpp index 8676504bc..032b0bf26 100644 --- a/src/core/mac/data_poll_handler.cpp +++ b/src/core/mac/data_poll_handler.cpp @@ -79,9 +79,8 @@ DataPollHandler::DataPollHandler(Instance &aInstance) void DataPollHandler::Clear(void) { - for (ChildTable::Iterator iter(GetInstance(), Child::kInStateAnyExceptInvalid); !iter.IsDone(); iter++) + for (Child &child : Get().Iterate(Child::kInStateAnyExceptInvalid)) { - Child &child = *iter.GetChild(); child.SetDataPollPending(false); child.SetFrameReplacePending(false); child.SetFramePurgePending(false); @@ -289,20 +288,18 @@ exit: void DataPollHandler::ProcessPendingPolls(void) { - for (ChildTable::Iterator iter(GetInstance(), Child::kInStateValidOrRestoring); !iter.IsDone(); iter++) + for (Child &child : Get().Iterate(Child::kInStateValidOrRestoring)) { - Child *child = iter.GetChild(); - - if (!child->IsDataPollPending()) + if (!child.IsDataPollPending()) { continue; } // Find the child with earliest poll receive time. - if ((mIndirectTxChild == nullptr) || (child->GetLastHeard() < mIndirectTxChild->GetLastHeard())) + if ((mIndirectTxChild == nullptr) || (child.GetLastHeard() < mIndirectTxChild->GetLastHeard())) { - mIndirectTxChild = child; + mIndirectTxChild = &child; } } diff --git a/src/core/thread/address_resolver.cpp b/src/core/thread/address_resolver.cpp index c61f84a7e..7d17bcea0 100644 --- a/src/core/thread/address_resolver.cpp +++ b/src/core/thread/address_resolver.cpp @@ -723,10 +723,8 @@ void AddressResolver::HandleAddressError(Coap::Message &aMessage, const Ip6::Mes meshLocalIid.ConvertToExtAddress(extAddr); - for (ChildTable::Iterator iter(GetInstance(), Child::kInStateValid); !iter.IsDone(); iter++) + for (Child &child : Get().Iterate(Child::kInStateValid)) { - Child &child = *iter.GetChild(); - if (child.IsFullThreadDevice()) { continue; @@ -780,10 +778,8 @@ void AddressResolver::HandleAddressQuery(Coap::Message &aMessage, const Ip6::Mes ExitNow(); } - for (ChildTable::Iterator iter(GetInstance(), Child::kInStateValid); !iter.IsDone(); iter++) + for (Child &child : Get().Iterate(Child::kInStateValid)) { - Child &child = *iter.GetChild(); - if (child.IsFullThreadDevice() || child.GetLinkFailures() >= Mle::kFailedChildTransmissions) { continue; diff --git a/src/core/thread/child_table.cpp b/src/core/thread/child_table.cpp index b54b20ba3..b859a9429 100644 --- a/src/core/thread/child_table.cpp +++ b/src/core/thread/child_table.cpp @@ -44,16 +44,6 @@ namespace ot { ChildTable::Iterator::Iterator(Instance &aInstance, Child::StateFilter aFilter) : InstanceLocator(aInstance) , mFilter(aFilter) - , mStart(nullptr) - , mChild(nullptr) -{ - Reset(); -} - -ChildTable::Iterator::Iterator(Instance &aInstance, Child::StateFilter aFilter, Child *aStartingChild) - : InstanceLocator(aInstance) - , mFilter(aFilter) - , mStart(aStartingChild) , mChild(nullptr) { Reset(); @@ -61,12 +51,7 @@ ChildTable::Iterator::Iterator(Instance &aInstance, Child::StateFilter aFilter, void ChildTable::Iterator::Reset(void) { - if (mStart == nullptr) - { - mStart = &Get().mChildren[0]; - } - - mChild = mStart; + mChild = &Get().mChildren[0]; if (!mChild->MatchesFilter(mFilter)) { @@ -76,22 +61,12 @@ void ChildTable::Iterator::Reset(void) void ChildTable::Iterator::Advance(void) { - ChildTable &childTable = Get(); - Child * listStart = &childTable.mChildren[0]; - Child * listEnd = &childTable.mChildren[childTable.mMaxChildrenAllowed]; - VerifyOrExit(mChild != nullptr, OT_NOOP); do { mChild++; - - if (mChild >= listEnd) - { - mChild = listStart; - } - - VerifyOrExit(mChild != mStart, mChild = nullptr); + VerifyOrExit(mChild < &Get().mChildren[Get().mMaxChildrenAllowed], mChild = nullptr); } while (!mChild->MatchesFilter(mFilter)); exit: diff --git a/src/core/thread/child_table.hpp b/src/core/thread/child_table.hpp index ae5c3d046..87300376f 100644 --- a/src/core/thread/child_table.hpp +++ b/src/core/thread/child_table.hpp @@ -49,6 +49,8 @@ namespace ot { */ class ChildTable : public InstanceLocator { + class IteratorBuilder; + public: /** * This class represents an iterator for iterating through the child entries in the child table. @@ -56,9 +58,11 @@ public: */ class Iterator : public InstanceLocator { + friend class IteratorBuilder; + public: /** - * This constructor initializes an `Iterator` instance to start from beginning of the child table. + * This constructor initializes an `Iterator` instance. * * @param[in] aInstance A reference to the OpenThread instance. * @param[in] aFilter A child state filter. @@ -66,23 +70,6 @@ public: */ Iterator(Instance &aInstance, Child::StateFilter aFilter); - /** - * This constructor initializes an `Iterator` instance to start from a given child. - * - * This constructor allows the iterator to start from a given `Child` entry. The iterator will start from the - * given child and will go through all entries in the child table (matching the filter) till it gets back to - * the starting `Child` entry. - * - * If the given starting `Child` pointer is `nullptr`, then the iterator starts from beginning of the child - * table. - * - * @param[in] aInstance A reference to the OpenThread instance. - * @param[in] aFilter A child state filter. - * @param[in] aStartingChild A pointer to a child. If non-nullptr, the iterator starts from the given entry. - * - */ - Iterator(Instance &aInstance, Child::StateFilter aFilter, Child *aStartingChild); - /** * This method resets the iterator to start over. * @@ -99,16 +86,6 @@ public: */ bool IsDone(void) const { return (mChild == nullptr); } - /** - * This method advances the iterator. - * - * The iterator is moved to point to the next `Child` entry matching the given state filter in the constructor. - * If there are no more `Child` entries matching the given filter, the iterator becomes empty (i.e., - * `GetChild()` returns `nullptr` and `IsDone()` returns `true`). - * - */ - void Advance(void); - /** * This method overloads `++` operator (pre-increment) to advance the iterator. * @@ -137,9 +114,61 @@ public: */ Child *GetChild(void) { return mChild; } + /** + * This method overloads the `*` dereference operator and gets a reference to `Child` entry to which the + * iterator is currently pointing. + * + * This method MUST be used when the iterator is not empty/finished (i.e., `IsDone()` returns `false`). + * + * @returns A reference to the `Child` entry currently pointed by the iterator. + * + */ + Child &operator*(void) { return *mChild; } + + /** + * This method overloads the `->` dereference operator and gets a pointer to `Child` entry to which the iterator + * is currently pointing. + * + * @returns A pointer to the `Child` entry associated with the iterator, or `nullptr` if iterator is empty/done. + * + */ + Child *operator->(void) { return mChild; } + + /** + * This method overloads operator `==` to evaluate whether or not two `Iterator` instances point to the same + * child entry. + * + * @param[in] aOther The other `Iterator` to compare with. + * + * @retval TRUE If the two `Iterator` objects point to the same child entry or both are done. + * @retval FALSE If the two `Iterator` objects do not point to the same child entry. + * + */ + bool operator==(const Iterator &aOther) const { return mChild == aOther.mChild; } + + /** + * This method overloads operator `!=` to evaluate whether or not two `Iterator` instances point to the same + * child entry. + * + * @param[in] aOther The other `Iterator` to compare with. + * + * @retval TRUE If the two `Iterator` objects do not point to the same child entry. + * @retval FALSE If the two `Iterator` objects point to the same child entry or both are done. + * + */ + bool operator!=(const Iterator &aOther) const { return mChild != aOther.mChild; } + private: + Iterator(Instance &aInstance) + : InstanceLocator(aInstance) + , mFilter(Child::StateFilter::kInStateValid) + , mChild(nullptr) + { + } + + void Advance(void); + Child::StateFilter mFilter; - Child * mStart; Child * mChild; }; @@ -276,12 +305,43 @@ public: */ otError SetMaxChildrenAllowed(uint16_t aMaxChildren); + /** + * This method enables range-based `for` loop iteration over all child entries in the child table matching a given + * state filter. + * + * This method should be used as follows: + * + * for (Child &child : aChildTable.Iterate(aFilter)) { ... } + * + * @param[in] aFilter A child state filter. + * + * @returns An IteratorBuilder instance. + * + */ + IteratorBuilder Iterate(Child::StateFilter aFilter) { return IteratorBuilder(GetInstance(), aFilter); } + private: enum { kMaxChildren = OPENTHREAD_CONFIG_MLE_MAX_CHILDREN, }; + class IteratorBuilder : public InstanceLocator + { + public: + IteratorBuilder(Instance &aInstance, Child::StateFilter aFilter) + : InstanceLocator(aInstance) + , mFilter(aFilter) + { + } + + Iterator begin(void) { return Iterator(GetInstance(), mFilter); } + Iterator end(void) { return Iterator(GetInstance()); } + + private: + Child::StateFilter mFilter; + }; + uint16_t mMaxChildrenAllowed; Child mChildren[kMaxChildren]; }; diff --git a/src/core/thread/indirect_sender.cpp b/src/core/thread/indirect_sender.cpp index df3ff5035..4e006e12d 100644 --- a/src/core/thread/indirect_sender.cpp +++ b/src/core/thread/indirect_sender.cpp @@ -72,10 +72,10 @@ void IndirectSender::Stop(void) { VerifyOrExit(mEnabled, OT_NOOP); - for (ChildTable::Iterator iter(GetInstance(), Child::kInStateAnyExceptInvalid); !iter.IsDone(); iter++) + for (Child &child : Get().Iterate(Child::kInStateAnyExceptInvalid)) { - iter.GetChild()->SetIndirectMessage(nullptr); - mSourceMatchController.ResetMessageCount(*iter.GetChild()); + child.SetIndirectMessage(nullptr); + mSourceMatchController.ResetMessageCount(child); } mDataPollHandler.Clear(); @@ -556,14 +556,14 @@ exit: void IndirectSender::ClearMessagesForRemovedChildren(void) { - for (ChildTable::Iterator iter(GetInstance(), Child::kInStateAnyExceptValidOrRestoring); !iter.IsDone(); iter++) + for (Child &child : Get().Iterate(Child::kInStateAnyExceptValidOrRestoring)) { - if (iter.GetChild()->GetIndirectMessageCount() == 0) + if (child.GetIndirectMessageCount() == 0) { continue; } - ClearAllMessagesForSleepyChild(*iter.GetChild()); + ClearAllMessagesForSleepyChild(child); } } diff --git a/src/core/thread/key_manager.cpp b/src/core/thread/key_manager.cpp index 3941cbc6a..bcb0c6aee 100644 --- a/src/core/thread/key_manager.cpp +++ b/src/core/thread/key_manager.cpp @@ -130,11 +130,11 @@ otError KeyManager::SetMasterKey(const MasterKey &aKey) } // reset child frame counters - for (ChildTable::Iterator iter(GetInstance(), Child::kInStateAnyExceptInvalid); !iter.IsDone(); iter++) + for (Child &child : Get().Iterate(Child::kInStateAnyExceptInvalid)) { - iter.GetChild()->SetKeySequence(0); - iter.GetChild()->SetLinkFrameCounter(0); - iter.GetChild()->SetMleFrameCounter(0); + child.SetKeySequence(0); + child.SetLinkFrameCounter(0); + child.SetMleFrameCounter(0); } #endif diff --git a/src/core/thread/mesh_forwarder.cpp b/src/core/thread/mesh_forwarder.cpp index 6c183dfad..b15ff3b14 100644 --- a/src/core/thread/mesh_forwarder.cpp +++ b/src/core/thread/mesh_forwarder.cpp @@ -138,9 +138,9 @@ void MeshForwarder::RemoveMessage(Message &aMessage) if (queue == &mSendQueue) { #if OPENTHREAD_FTD - for (ChildTable::Iterator iter(GetInstance(), Child::kInStateAnyExceptInvalid); !iter.IsDone(); iter++) + for (Child &child : Get().Iterate(Child::kInStateAnyExceptInvalid)) { - IgnoreError(mIndirectSender.RemoveMessageFromSleepyChild(aMessage, *iter.GetChild())); + IgnoreError(mIndirectSender.RemoveMessageFromSleepyChild(aMessage, child)); } #endif diff --git a/src/core/thread/mesh_forwarder_ftd.cpp b/src/core/thread/mesh_forwarder_ftd.cpp index 3110445bb..c88c41311 100644 --- a/src/core/thread/mesh_forwarder_ftd.cpp +++ b/src/core/thread/mesh_forwarder_ftd.cpp @@ -80,11 +80,8 @@ otError MeshForwarder::SendMessage(Message &aMessage) ip6Header.GetDestination() == mle.GetRealmLocalAllThreadNodesAddress()) { // destined for all sleepy children - for (ChildTable::Iterator iter(GetInstance(), Child::kInStateValidOrRestoring); !iter.IsDone(); - iter++) + for (Child &child : Get().Iterate(Child::kInStateValidOrRestoring)) { - Child &child = *iter.GetChild(); - if (!child.IsRxOnWhenIdle()) { mIndirectSender.AddMessageForSleepyChild(aMessage, child); @@ -94,11 +91,8 @@ otError MeshForwarder::SendMessage(Message &aMessage) else { // destined for some sleepy children which subscribed the multicast address. - for (ChildTable::Iterator iter(GetInstance(), Child::kInStateValidOrRestoring); !iter.IsDone(); - iter++) + for (Child &child : Get().Iterate(Child::kInStateValidOrRestoring)) { - Child &child = *iter.GetChild(); - if (mle.IsSleepyChildSubscribed(ip6Header.GetDestination(), child)) { mIndirectSender.AddMessageForSleepyChild(aMessage, child); @@ -329,9 +323,9 @@ void MeshForwarder::RemoveDataResponseMessages(void) if (!(ip6Header.GetDestination().IsMulticast())) { - for (ChildTable::Iterator iter(GetInstance(), Child::kInStateAnyExceptInvalid); !iter.IsDone(); iter++) + for (Child &child : Get().Iterate(Child::kInStateAnyExceptInvalid)) { - IgnoreError(mIndirectSender.RemoveMessageFromSleepyChild(*message, *iter.GetChild())); + IgnoreError(mIndirectSender.RemoveMessageFromSleepyChild(*message, child)); } } diff --git a/src/core/thread/mle_router.cpp b/src/core/thread/mle_router.cpp index 4ae545c72..d87896b37 100644 --- a/src/core/thread/mle_router.cpp +++ b/src/core/thread/mle_router.cpp @@ -330,11 +330,11 @@ void MleRouter::SetStateRouter(uint16_t aRloc16) Get().SetBeaconEnabled(true); // remove children that do not have matching RLOC16 - for (ChildTable::Iterator iter(GetInstance(), Child::kInStateValidOrRestoring); !iter.IsDone(); iter++) + for (Child &child : Get().Iterate(Child::kInStateValidOrRestoring)) { - if (RouterIdFromRloc16(iter.GetChild()->GetRloc16()) != mRouterId) + if (RouterIdFromRloc16(child.GetRloc16()) != mRouterId) { - RemoveNeighbor(*iter.GetChild()); + RemoveNeighbor(child); } } } @@ -370,11 +370,11 @@ void MleRouter::SetStateLeader(uint16_t aRloc16) Get().Clear(); // remove children that do not have matching RLOC16 - for (ChildTable::Iterator iter(GetInstance(), Child::kInStateValidOrRestoring); !iter.IsDone(); iter++) + for (Child &child : Get().Iterate(Child::kInStateValidOrRestoring)) { - if (RouterIdFromRloc16(iter.GetChild()->GetRloc16()) != mRouterId) + if (RouterIdFromRloc16(child.GetRloc16()) != mRouterId) { - RemoveNeighbor(*iter.GetChild()); + RemoveNeighbor(child); } } @@ -1835,9 +1835,8 @@ void MleRouter::HandleStateUpdateTimer(void) } // update children state - for (ChildTable::Iterator iter(GetInstance(), Child::kInStateAnyExceptInvalid); !iter.IsDone(); iter++) + for (Child &child : Get().Iterate(Child::kInStateAnyExceptInvalid)) { - Child & child = *iter.GetChild(); uint32_t timeout = 0; switch (child.GetState()) @@ -2124,14 +2123,14 @@ otError MleRouter::UpdateChildAddresses(const Message &aMessage, uint16_t aOffse // table is timed out and then trying to register its globally unique // IPv6 address as the new child. - for (ChildTable::Iterator iter(GetInstance(), Child::kInStateValidOrRestoring); !iter.IsDone(); iter++) + for (Child &child : Get().Iterate(Child::kInStateValidOrRestoring)) { - if (iter.GetChild() == &aChild) + if (&child == &aChild) { continue; } - IgnoreError(iter.GetChild()->RemoveIp6Address(address)); + IgnoreError(child.RemoveIp6Address(address)); } // Clear EID-to-RLOC cache for the unicast address registered by the child. @@ -2728,9 +2727,8 @@ void MleRouter::SynchronizeChildNetworkData(void) { VerifyOrExit(IsRouterOrLeader(), OT_NOOP); - for (ChildTable::Iterator iter(GetInstance(), Child::kInStateValid); !iter.IsDone(); iter++) + for (Child &child : Get().Iterate(Child::kInStateValid)) { - Child & child = *iter.GetChild(); uint8_t version; if (child.IsRxOnWhenIdle()) @@ -3452,7 +3450,6 @@ Neighbor *MleRouter::GetNeighbor(const Mac::Address &aAddress) Neighbor *MleRouter::GetNeighbor(const Ip6::Address &aAddress) { Lowpan::Context context; - Child * child; Neighbor * rval = nullptr; if (aAddress.IsLinkLocal()) @@ -3468,19 +3465,17 @@ Neighbor *MleRouter::GetNeighbor(const Ip6::Address &aAddress) context.mContextId = 0xff; } - for (ChildTable::Iterator iter(GetInstance(), Child::kInStateValidOrRestoring); !iter.IsDone(); iter++) + for (Child &child : Get().Iterate(Child::kInStateValidOrRestoring)) { - child = iter.GetChild(); - if ((context.mContextId == kMeshLocalPrefixContextId) && aAddress.GetIid().IsLocator() && - (aAddress.GetIid().GetLocator() == child->GetRloc16())) + (aAddress.GetIid().GetLocator() == child.GetRloc16())) { - ExitNow(rval = child); + ExitNow(rval = &child); } - if (child->HasIp6Address(aAddress)) + if (child.HasIp6Address(aAddress)) { - ExitNow(rval = child); + ExitNow(rval = &child); } } @@ -3752,9 +3747,9 @@ void MleRouter::RefreshStoredChildren(void) { SuccessOrExit(Get().DeleteChildInfo()); - for (ChildTable::Iterator iter(GetInstance(), Child::kInStateAnyExceptInvalid); !iter.IsDone(); iter++) + for (Child &child : Get().Iterate(Child::kInStateAnyExceptInvalid)) { - SuccessOrExit(StoreChild(*iter.GetChild())); + SuccessOrExit(StoreChild(child)); } exit: @@ -4110,9 +4105,9 @@ void MleRouter::HandleAddressSolicitResponse(Coap::Message * aMessage, IgnoreError(SendLinkRequest(nullptr)); // send child id responses - for (ChildTable::Iterator iter(GetInstance(), Child::kInStateChildIdRequest); !iter.IsDone(); iter++) + for (Child &child : Get().Iterate(Child::kInStateChildIdRequest)) { - IgnoreError(SendChildIdResponse(*iter.GetChild())); + IgnoreError(SendChildIdResponse(child)); } exit: @@ -4671,9 +4666,9 @@ bool MleRouter::HasChildren(void) void MleRouter::RemoveChildren(void) { - for (ChildTable::Iterator iter(GetInstance(), Child::kInStateValidOrRestoring); !iter.IsDone(); iter++) + for (Child &child : Get().Iterate(Child::kInStateValidOrRestoring)) { - RemoveNeighbor(*iter.GetChild()); + RemoveNeighbor(child); } } @@ -4711,10 +4706,8 @@ otError MleRouter::GetMaxChildTimeout(uint32_t &aTimeout) const VerifyOrExit(IsRouterOrLeader(), error = OT_ERROR_INVALID_STATE); - for (ChildTable::Iterator iter(GetInstance(), Child::kInStateValid); !iter.IsDone(); iter++) + for (Child &child : Get().Iterate(Child::kInStateValid)) { - Child &child = *iter.GetChild(); - if (child.IsFullThreadDevice()) { continue; @@ -4783,10 +4776,8 @@ bool MleRouter::HasSleepyChildrenSubscribed(const Ip6::Address &aAddress) { bool rval = false; - for (ChildTable::Iterator iter(GetInstance(), Child::kInStateValidOrRestoring); !iter.IsDone(); iter++) + for (Child &child : Get().Iterate(Child::kInStateValidOrRestoring)) { - Child &child = *iter.GetChild(); - if (child.IsRxOnWhenIdle()) { continue; diff --git a/src/core/thread/network_diagnostic.cpp b/src/core/thread/network_diagnostic.cpp index 2b778ce68..7c7fd494b 100644 --- a/src/core/thread/network_diagnostic.cpp +++ b/src/core/thread/network_diagnostic.cpp @@ -241,12 +241,10 @@ otError NetworkDiagnostic::AppendChildTable(Message &aMessage) SuccessOrExit(error = aMessage.Append(&tlv, sizeof(ChildTableTlv))); - for (ChildTable::Iterator iter(GetInstance(), Child::kInStateValid); !iter.IsDone(); iter++) + for (Child &child : Get().Iterate(Child::kInStateValid)) { VerifyOrExit(count--, OT_NOOP); - Child &child = *iter.GetChild(); - timeout = 0; while (static_cast(1 << timeout) < child.GetTimeout()) diff --git a/src/core/thread/src_match_controller.cpp b/src/core/thread/src_match_controller.cpp index 7ecd1de39..0dcc91c05 100644 --- a/src/core/thread/src_match_controller.cpp +++ b/src/core/thread/src_match_controller.cpp @@ -212,12 +212,12 @@ otError SourceMatchController::AddPendingEntries(void) { otError error = OT_ERROR_NONE; - for (ChildTable::Iterator iter(GetInstance(), Child::kInStateValidOrRestoring); !iter.IsDone(); iter++) + for (Child &child : Get().Iterate(Child::kInStateValidOrRestoring)) { - if (iter.GetChild()->IsIndirectSourceMatchPending()) + if (child.IsIndirectSourceMatchPending()) { - SuccessOrExit(error = AddAddress(*iter.GetChild())); - iter.GetChild()->SetIndirectSourceMatchPending(false); + SuccessOrExit(error = AddAddress(child)); + child.SetIndirectSourceMatchPending(false); } } diff --git a/src/core/utils/child_supervision.cpp b/src/core/utils/child_supervision.cpp index 16ce303ac..16c35d931 100644 --- a/src/core/utils/child_supervision.cpp +++ b/src/core/utils/child_supervision.cpp @@ -120,10 +120,8 @@ void ChildSupervisor::HandleTimer(void) { VerifyOrExit(mSupervisionInterval != 0, OT_NOOP); - for (ChildTable::Iterator iter(GetInstance(), Child::kInStateValid); !iter.IsDone(); iter++) + for (Child &child : Get().Iterate(Child::kInStateValid)) { - Child &child = *iter.GetChild(); - child.IncrementSecondsSinceLastSupervision(); if ((child.GetSecondsSinceLastSupervision() >= mSupervisionInterval) && !child.IsRxOnWhenIdle()) diff --git a/tests/unit/test_child_table.cpp b/tests/unit/test_child_table.cpp index 00da31c2e..ea658e438 100644 --- a/tests/unit/test_child_table.cpp +++ b/tests/unit/test_child_table.cpp @@ -144,97 +144,85 @@ void VerifyChildTableContent(ChildTable &aTable, uint16_t aChildListLength, cons VerifyOrQuit(ChildMatches(*child, aChildList[listIndex]), "FindChild(address) returned incorrect child"); } - // Verify `ChildTable::Iterator` behavior when starting from different child entries. + // Verify `ChildTable::Iterator` behavior. - for (uint16_t listIndex = 0; listIndex <= aChildListLength; listIndex++) { - Child *startingChild = nullptr; + ChildTable::Iterator iter(*sInstance, filter); + bool childObserved[kMaxChildren]; + uint16_t numChildren = 0; - if (listIndex < aChildListLength) + memset(childObserved, 0, sizeof(childObserved)); + + // Use the iterator and verify that each returned `Child` entry is in the expected list. + + for (; !iter.IsDone(); iter++) { - startingChild = aTable.FindChild(aChildList[listIndex].mRloc16, Child::kInStateAnyExceptInvalid); - VerifyOrQuit(startingChild != nullptr, "FindChild() failed"); - } + Child * child = iter.GetChild(); + Child & childRef = *iter; + bool didFind = false; + uint16_t childIndex; - // Test an iterator starting from `startingChild`. + VerifyOrQuit(child != nullptr, "iter.GetChild() failed"); + VerifyOrQuit(&childRef == child, "iter.operator*() failed"); + VerifyOrQuit(iter->GetRloc16() == child->GetRloc16(), "iter.operator->() failed"); - { - ChildTable::Iterator iter(*sInstance, filter, startingChild); - bool childObserved[kMaxChildren]; - uint16_t numChildren = 0; - - memset(childObserved, 0, sizeof(childObserved)); - - // Check if the first entry matches the `startingChild` - - if ((startingChild != nullptr) && StateMatchesFilter(startingChild->GetState(), filter)) - { - VerifyOrQuit(!iter.IsDone(), "iterator IsDone() failed"); - VerifyOrQuit(iter.GetChild() != nullptr, "iterator GetChild() failed"); - VerifyOrQuit(iter.GetChild() == startingChild, - "Iterator failed to start from the given child entry"); - - iter++; - iter.Reset(); - VerifyOrQuit(iter.GetChild() == startingChild, "iterator Reset() failed"); - } - - // Use the iterator and verify that each returned `Child` entry is in the expected list. - - for (; !iter.IsDone(); iter++) - { - Child * child = iter.GetChild(); - bool didFind = false; - uint16_t childIndex; - - VerifyOrQuit(child != nullptr, "iter.GetChild() failed"); - - childIndex = aTable.GetChildIndex(*child); - VerifyOrQuit(childIndex < aTable.GetMaxChildrenAllowed(), "Child Index is out of bound"); - VerifyOrQuit(aTable.GetChildAtIndex(childIndex) == child, "GetChildAtIndex() failed"); - - for (uint16_t index = 0; index < aChildListLength; index++) - { - if (ChildMatches(*iter.GetChild(), aChildList[index])) - { - childObserved[index] = true; - numChildren++; - didFind = true; - break; - } - } - - VerifyOrQuit(didFind, "ChildTable::Iterator returned an entry not in the expected list"); - } - - // Verify that when iterator is done, it points to `nullptr`. - - VerifyOrQuit(iter.GetChild() == nullptr, "iterator GetChild() failed"); - - iter++; - VerifyOrQuit(iter.IsDone(), "iterator Advance() (after iterator is done) failed"); - VerifyOrQuit(iter.GetChild() == nullptr, "iterator GetChild() failed"); - - // Verify that the number of children matches the number of entries we get from iterator. - - VerifyOrQuit(aTable.GetNumChildren(filter) == numChildren, "GetNumChildren() failed"); - VerifyOrQuit(aTable.HasChildren(filter) == (numChildren != 0), "HasChildren() failed"); - - // Verify that there is no missing or extra entry between the expected list - // and what was observed/returned by the iterator. + childIndex = aTable.GetChildIndex(*child); + VerifyOrQuit(childIndex < aTable.GetMaxChildrenAllowed(), "Child Index is out of bound"); + VerifyOrQuit(aTable.GetChildAtIndex(childIndex) == child, "GetChildAtIndex() failed"); for (uint16_t index = 0; index < aChildListLength; index++) { - if (StateMatchesFilter(aChildList[index].mState, filter)) + if (ChildMatches(*iter.GetChild(), aChildList[index])) { - VerifyOrQuit(childObserved[index], "iterator failed to return an expected entry"); - } - else - { - VerifyOrQuit(!childObserved[index], "iterator returned an extra unexpected entry"); + childObserved[index] = true; + numChildren++; + didFind = true; + break; } } + + VerifyOrQuit(didFind, "ChildTable::Iterator returned an entry not in the expected list"); } + + // Verify that when iterator is done, it points to `nullptr`. + + VerifyOrQuit(iter.GetChild() == nullptr, "iterator GetChild() failed"); + + iter++; + VerifyOrQuit(iter.IsDone(), "iterator Advance() (after iterator is done) failed"); + VerifyOrQuit(iter.GetChild() == nullptr, "iterator GetChild() failed"); + + // Verify that the number of children matches the number of entries we get from iterator. + + VerifyOrQuit(aTable.GetNumChildren(filter) == numChildren, "GetNumChildren() failed"); + VerifyOrQuit(aTable.HasChildren(filter) == (numChildren != 0), "HasChildren() failed"); + + // Verify that there is no missing or extra entry between the expected list + // and what was observed/returned by the iterator. + + for (uint16_t index = 0; index < aChildListLength; index++) + { + if (StateMatchesFilter(aChildList[index].mState, filter)) + { + VerifyOrQuit(childObserved[index], "iterator failed to return an expected entry"); + } + else + { + VerifyOrQuit(!childObserved[index], "iterator returned an extra unexpected entry"); + } + } + + // Verify the behavior of range-based `for` iteration. + + iter.Reset(); + + for (Child &child : aTable.Iterate(filter)) + { + VerifyOrQuit(&child == iter.GetChild(), "range-based for loop Iterate() failed"); + iter++; + } + + VerifyOrQuit(iter.IsDone(), "range-based for loop Iterate() did not return all entries"); } }