Use Loop::Break/Continue instead of bool in visitor of ForEachActiveO…

…bject (#12420)

src/app/CASESessionManager.cpp

@@ -125,9 +125,9 @@ OperationalDeviceProxy * CASESessionManager::FindSession(SessionHandle session)
         if (activeSession->MatchesSession(session))
         {
             foundSession = activeSession;
-            return false;
+            return Loop::Break;
         }
-        return true;
+        return Loop::Continue;
     });
 
     return foundSession;
@@ -140,9 +140,9 @@ OperationalDeviceProxy * CASESessionManager::FindExistingSession(NodeId id)
         if (activeSession->GetDeviceId() == id)
         {
             foundSession = activeSession;
-            return false;
+            return Loop::Break;
         }
-        return true;
+        return Loop::Continue;
     });
 
     return foundSession;

src/app/InteractionModelEngine.cpp

@@ -81,7 +81,7 @@ void InteractionModelEngine::Shutdown()
     // Increase magic number to invalidate all Handle-s.
     mMagic++;
 
-    mCommandHandlerObjs.ForEachActiveObject([this](CommandHandler * obj) -> bool {
+    mCommandHandlerObjs.ForEachActiveObject([this](CommandHandler * obj) -> Loop {
         // Modifying the pool during iteration is generally frowned upon.
         // This is almost safe since mCommandHandlerObjs is a BitMapObjectPool which won't malfunction when modifying the inner
         // record while during traversal. But this behavior is not guranteed, so we should fix this by implementing DeallocateAll.
@@ -92,16 +92,16 @@ void InteractionModelEngine::Shutdown()
         // TODO(@kghost, #10332) Implement DeallocateAll and replace this.
 
         mCommandHandlerObjs.Deallocate(obj);
-        return true;
+        return Loop::Continue;
     });
 
-    mTimedHandlers.ForEachActiveObject([this](TimedHandler * obj) -> bool {
+    mTimedHandlers.ForEachActiveObject([this](TimedHandler * obj) -> Loop {
         // This calls back into us and deallocates |obj|.  As above, this is not
         // really guaranteed, and we should do something better here (like
         // ignoring the calls to OnTimedInteractionFailed and then doing a
         // DeallocateAll.
         mpExchangeMgr->CloseAllContextsForDelegate(obj);
-        return true;
+        return Loop::Continue;
     });
 
     for (auto & readClient : mReadClients)

src/channel/Manager.cpp

@@ -29,9 +29,9 @@ ChannelHandle ChannelManager::EstablishChannel(const ChannelBuilder & builder, C
         if (context->MatchesBuilder(builder))
         {
             channelContext = context;
-            return false;
+            return Loop::Break;
         }
-        return true;
+        return Loop::Continue;
     });
 
     if (channelContext == nullptr)

src/channel/Manager.h

@@ -57,7 +57,7 @@ class DLL_EXPORT ChannelManager : public SessionCreationDelegate
         mChannelHandles.ForEachActiveObject([&](ChannelContextHandleAssociation * association) {
             if (association->mChannelContext == channel)
                 event(association->mChannelDelegate);
-            return true;
+            return Loop::Continue;
         });
     }
 
@@ -67,9 +67,9 @@ class DLL_EXPORT ChannelManager : public SessionCreationDelegate
             if (context->MatchesSession(session, mExchangeManager->GetSessionManager()))
             {
                 context->OnNewConnection(session);
-                return false;
+                return Loop::Break;
             }
-            return true;
+            return Loop::Continue;
         });
     }
 

src/controller/CHIPDeviceController.cpp

@@ -708,9 +708,9 @@ CommissioneeDeviceProxy * DeviceCommissioner::FindCommissioneeDevice(SessionHand
         if (deviceProxy->MatchesSession(session))
         {
             foundDevice = deviceProxy;
-            return false;
+            return Loop::Break;
         }
-        return true;
+        return Loop::Continue;
     });
 
     return foundDevice;
@@ -723,9 +723,9 @@ CommissioneeDeviceProxy * DeviceCommissioner::FindCommissioneeDevice(NodeId id)
         if (deviceProxy->GetDeviceId() == id)
         {
             foundDevice = deviceProxy;
-            return false;
+            return Loop::Break;
         }
-        return true;
+        return Loop::Continue;
     });
 
     return foundDevice;

src/inet/InetLayer.h

@@ -50,7 +50,7 @@ class EndPointManager
 {
 public:
     using EndPoint        = EndPointType;
-    using EndPointVisitor = bool (*)(EndPoint *);
+    using EndPointVisitor = Loop (*)(EndPoint *);
 
     EndPointManager() {}
     virtual ~EndPointManager() {}
@@ -83,7 +83,7 @@ class EndPointManager
 
     virtual EndPoint * CreateEndPoint()                         = 0;
     virtual void DeleteEndPoint(EndPoint * endPoint)            = 0;
-    virtual bool ForEachEndPoint(const EndPointVisitor visitor) = 0;
+    virtual Loop ForEachEndPoint(const EndPointVisitor visitor) = 0;
 
 private:
     System::Layer * mSystemLayer;
@@ -101,9 +101,9 @@ class EndPointManagerImplPool : public EndPointManager<typename EndPointImpl::En
 
     EndPoint * CreateEndPoint() override { return sEndPointPool.CreateObject(*this); }
     void DeleteEndPoint(EndPoint * endPoint) override { sEndPointPool.ReleaseObject(static_cast<EndPointImpl *>(endPoint)); }
-    bool ForEachEndPoint(const typename Manager::EndPointVisitor visitor) override
+    Loop ForEachEndPoint(const typename Manager::EndPointVisitor visitor) override
     {
-        return sEndPointPool.ForEachActiveObject([&](EndPoint * endPoint) -> bool { return visitor(endPoint); });
+        return sEndPointPool.ForEachActiveObject([&](EndPoint * endPoint) -> Loop { return visitor(endPoint); });
     }
 
 private:

src/inet/TCPEndPoint.cpp

@@ -2522,11 +2522,11 @@ void TCPEndPoint::HandleIdleTimer(chip::System::Layer * aSystemLayer, void * aAp
     auto & endPointManager = *reinterpret_cast<EndPointManager<TCPEndPoint> *>(aAppState);
     bool lTimerRequired    = IsIdleTimerRunning(endPointManager);
 
-    endPointManager.ForEachEndPoint([](TCPEndPoint * lEndPoint) -> bool {
+    endPointManager.ForEachEndPoint([](TCPEndPoint * lEndPoint) -> Loop {
         if (!lEndPoint->IsConnected())
-            return true;
+            return Loop::Continue;
         if (lEndPoint->mIdleTimeout == 0)
-            return true;
+            return Loop::Continue;
 
         if (lEndPoint->mRemainingIdleTime == 0)
         {
@@ -2537,7 +2537,7 @@ void TCPEndPoint::HandleIdleTimer(chip::System::Layer * aSystemLayer, void * aAp
             --lEndPoint->mRemainingIdleTime;
         }
 
-        return true;
+        return Loop::Continue;
     });
 
     if (lTimerRequired)
@@ -2550,7 +2550,9 @@ void TCPEndPoint::HandleIdleTimer(chip::System::Layer * aSystemLayer, void * aAp
 bool TCPEndPoint::IsIdleTimerRunning(EndPointManager<TCPEndPoint> & endPointManager)
 {
     // See if there are any TCP connections with the idle timer check in use.
-    return !endPointManager.ForEachEndPoint([](TCPEndPoint * lEndPoint) { return (lEndPoint->mIdleTimeout == 0); });
+    return Loop::Break == endPointManager.ForEachEndPoint([](TCPEndPoint * lEndPoint) {
+        return (lEndPoint->mIdleTimeout == 0) ? Loop::Continue : Loop::Break;
+    });
 }
 
 #endif // INET_TCP_IDLE_CHECK_INTERVAL > 0

src/lib/support/Pool.cpp

@@ -84,7 +84,8 @@ size_t StaticAllocatorBitmap::IndexOf(void * element)
     return index;
 }
 
-bool StaticAllocatorBitmap::ForEachActiveObjectInner(void * context, bool lambda(void * context, void * object))
+using Lambda = Loop (*)(void *, void *);
+Loop StaticAllocatorBitmap::ForEachActiveObjectInner(void * context, Lambda lambda)
 {
     for (size_t word = 0; word * kBitChunkSize < Capacity(); ++word)
     {
@@ -94,12 +95,12 @@ bool StaticAllocatorBitmap::ForEachActiveObjectInner(void * context, bool lambda
         {
             if ((value & (kBit1 << offset)) != 0)
             {
-                if (!lambda(context, At(word * kBitChunkSize + offset)))
-                    return false;
+                if (lambda(context, At(word * kBitChunkSize + offset)) == Loop::Break)
+                    return Loop::Break;
             }
         }
     }
-    return true;
+    return Loop::Finish;
 }
 
 #if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
@@ -116,20 +117,20 @@ HeapObjectListNode * HeapObjectList::FindNode(void * object) const
     return nullptr;
 }
 
-using Lambda = bool (*)(void *, void *);
-bool HeapObjectList::ForEachNode(void * context, bool lambda(void * context, void * object))
+using Lambda = Loop (*)(void *, void *);
+Loop HeapObjectList::ForEachNode(void * context, Lambda lambda)
 {
     ++mIterationDepth;
-    bool result            = true;
+    Loop result            = Loop::Finish;
     bool anyReleased       = false;
     HeapObjectListNode * p = mNext;
     while (p != this)
     {
         if (p->mObject != nullptr)
         {
-            if (!lambda(context, p->mObject))
+            if (lambda(context, p->mObject) == Loop::Break)
             {
-                result = false;
+                result = Loop::Break;
                 break;
             }
         }

src/lib/support/Pool.h

@@ -32,6 +32,13 @@
 
 namespace chip {
 
+enum class Loop : uint8_t
+{
+    Continue,
+    Break,
+    Finish,
+};
+
 namespace internal {
 
 class Statistics
@@ -87,7 +94,7 @@ class StaticAllocatorBitmap : public internal::StaticAllocatorBase
     void * At(size_t index) { return static_cast<uint8_t *>(mElements) + mElementSize * index; }
     size_t IndexOf(void * element);
 
-    bool ForEachActiveObjectInner(void * context, bool lambda(void * context, void * object));
+    Loop ForEachActiveObjectInner(void * context, Loop lambda(void * context, void * object));
 
 private:
     void * mElements;
@@ -112,7 +119,7 @@ class LambdaProxy
 {
 public:
     LambdaProxy(Function && function) : mFunction(std::move(function)) {}
-    static bool Call(void * context, void * target)
+    static Loop Call(void * context, void * target)
     {
         return static_cast<LambdaProxy *>(context)->mFunction(static_cast<T *>(target));
     }
@@ -150,7 +157,7 @@ struct HeapObjectList : HeapObjectListNode
 
     HeapObjectListNode * FindNode(void * object) const;
 
-    bool ForEachNode(void * context, bool lambda(void * context, void * object));
+    Loop ForEachNode(void * context, Loop lambda(void * context, void * object));
 
     size_t mIterationDepth;
 };
@@ -178,8 +185,8 @@ struct HeapObjectList : HeapObjectListNode
  *
  * @fn ForEachActiveObject
  * @memberof ObjectPool
- * @param visitor   A function that takes a T* and returns true to continue iterating or false to stop iterating.
- * @returns false if a visitor call returned false, true otherwise.
+ * @param visitor   A function that takes a T* and returns Loop::Continue to continue iterating or Loop::Break to stop iterating.
+ * @returns Loop::Break if a visitor call returned Loop::Break, Loop::Finish otherwise.
  *
  * Iteration may be nested. ReleaseObject() can be called during iteration, on the current object or any other.
  * CreateObject() can be called, but it is undefined whether or not a newly created object will be visited.
@@ -226,25 +233,27 @@ class BitMapObjectPool : public internal::StaticAllocatorBitmap, public internal
      * @brief
      *   Run a functor for each active object in the pool
      *
-     *  @param     function The functor of type `bool (*)(T*)`, return false to break the iteration
-     *  @return    bool     Returns false if broke during iteration
+     *  @param     function The functor of type `Loop (*)(T*)`, return Loop::Break to break the iteration
+     *  @return    Loop     Returns Break or Finish according to the iteration
      *
      * caution
      *   this function is not thread-safe, make sure all usage of the
      *   pool is protected by a lock, or else avoid using this function
      */
     template <typename Function>
-    bool ForEachActiveObject(Function && function)
+    Loop ForEachActiveObject(Function && function)
     {
+        static_assert(std::is_same<Loop, decltype(function(std::declval<T *>()))>::value,
+                      "The function must take T* and return Loop");
         internal::LambdaProxy<T, Function> proxy(std::forward<Function>(function));
         return ForEachActiveObjectInner(&proxy, &internal::LambdaProxy<T, Function>::Call);
     }
 
 private:
-    static bool ReleaseObject(void * context, void * object)
+    static Loop ReleaseObject(void * context, void * object)
     {
         static_cast<BitMapObjectPool *>(context)->ReleaseObject(static_cast<T *>(object));
-        return true;
+        return Loop::Continue;
     }
 
     std::atomic<tBitChunkType> mUsage[(N + kBitChunkSize - 1) / kBitChunkSize];
@@ -314,22 +323,23 @@ class HeapObjectPool : public internal::Statistics, public internal::PoolCommon<
      * @brief
      *   Run a functor for each active object in the pool
      *
-     *  @param     function The functor of type `bool (*)(T*)`, return false to break the iteration
-     *  @return    bool     Returns false if broke during iteration
+     *  @param     function The functor of type `Loop (*)(T*)`, return Loop::Break to break the iteration
+     *  @return    Loop     Returns Break or Finish according to the iteration
      */
     template <typename Function>
-    bool ForEachActiveObject(Function && function)
+    Loop ForEachActiveObject(Function && function)
     {
-        // return ForEachNode([function](void *object) { return function(static_cast<T*>(object)); });
+        static_assert(std::is_same<Loop, decltype(function(std::declval<T *>()))>::value,
+                      "The function must take T* and return Loop");
         internal::LambdaProxy<T, Function> proxy(std::forward<Function>(function));
         return mObjects.ForEachNode(&proxy, &internal::LambdaProxy<T, Function>::Call);
     }
 
 private:
-    static bool ReleaseObject(void * context, void * object)
+    static Loop ReleaseObject(void * context, void * object)
     {
         static_cast<HeapObjectPool *>(context)->ReleaseObject(static_cast<T *>(object));
-        return true;
+        return Loop::Continue;
     }
 
     internal::HeapObjectList mObjects;

src/lib/support/PoolWrapper.h

@@ -38,16 +38,18 @@ class PoolInterface
     virtual void ResetObject(U * element, ConstructorArguments &&... args) = 0;
 
     template <typename Function>
-    bool ForEachActiveObject(Function && function)
+    Loop ForEachActiveObject(Function && function)
     {
-        auto proxy = [&](U * target) -> bool { return function(target); };
+        static_assert(std::is_same<Loop, decltype(function(std::declval<U *>()))>::value,
+                      "The function must take T* and return Loop");
+        auto proxy = [&](U * target) -> Loop { return function(target); };
         return ForEachActiveObjectInner(
-            &proxy, [](void * context, U * target) -> bool { return (*static_cast<decltype(proxy) *>(context))(target); });
+            &proxy, [](void * context, U * target) -> Loop { return (*static_cast<decltype(proxy) *>(context))(target); });
     }
 
 protected:
-    using Lambda                                                         = bool (*)(void *, U *);
-    virtual bool ForEachActiveObjectInner(void * context, Lambda lambda) = 0;
+    using Lambda                                                         = Loop (*)(void *, U *);
+    virtual Loop ForEachActiveObjectInner(void * context, Lambda lambda) = 0;
 };
 
 template <class T, size_t N, typename Interface>
@@ -75,7 +77,7 @@ class PoolProxy<T, N, std::tuple<U, ConstructorArguments...>> : public PoolInter
     }
 
 protected:
-    virtual bool ForEachActiveObjectInner(void * context,
+    virtual Loop ForEachActiveObjectInner(void * context,
                                           typename PoolInterface<U, ConstructorArguments...>::Lambda lambda) override
     {
         return Impl().ForEachActiveObject([&](T * target) { return lambda(context, static_cast<U *>(target)); });