From 41e07366fac8b7ee1ffde999809ef1e690faecc2 Mon Sep 17 00:00:00 2001
From: Abtin Keshavarzian <abtink@google.com>
Date: Thu, 7 May 2026 07:30:14 -0700
Subject: [PATCH] [mlr] extract address registration success evaluation into
 helper (#13071)

This commit introduces a new static helper method,
`Manager::DidRegisterSuccessfully()`, to evaluate whether a specific
multicast address was successfully registered based on the MLR response
status and the list of failed addresses.

Previously, this evaluation logic was duplicated and inline within
`Manager::Finish()` using the expression:
`success = aSuccess || !aFailedAddresses.IsEmptyOrContains(addr)`.
This logic was not immediately intuitive and required reasoning through
the boolean conditions to understand the intended behavior.

Extracting this into a dedicated helper method improves code
readability and maintainability. It simplifies `Finish()` by
clearly separating the outcome evaluation from the actual state
transition logic (`kStateRegistering` to `kStateRegistered` or
`kStateToRegister`).

Additionally, the unused `AddressArray::IsEmptyOrContains()` method
has been removed.
---
 src/core/thread/mlr_manager.cpp | 37 +++++++++++++++++++++++++++++----
 src/core/thread/mlr_manager.hpp |  4 +++-
 2 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/src/core/thread/mlr_manager.cpp b/src/core/thread/mlr_manager.cpp
index 9064a5339..a40c20b6f 100644
--- a/src/core/thread/mlr_manager.cpp
+++ b/src/core/thread/mlr_manager.cpp
@@ -509,6 +509,35 @@ void Manager::SetMulticastAddressState(State aFromState, State aToState)
 #endif
 }
 
+bool Manager::DidRegisterSuccessfully(const Ip6::Address &aAddress, bool aSuccess, const AddressArray &aFailedAddresses)
+{
+    // If the operation succeeded, all address registrations were successful.
+    //
+    // If it failed and the failed address list is empty, all registrations
+    // failed.
+    //
+    // If it failed and a non-empty failed address list is provided, only the
+    // addresses in the list failed (if an address is not in the list, its
+    // registration was successful).
+
+    bool didRegister;
+
+    if (aSuccess)
+    {
+        didRegister = true;
+    }
+    else if (aFailedAddresses.IsEmpty())
+    {
+        didRegister = false;
+    }
+    else
+    {
+        didRegister = !aFailedAddresses.Contains(aAddress);
+    }
+
+    return didRegister;
+}
+
 void Manager::Finish(bool aSuccess, const AddressArray &aFailedAddresses)
 {
     OT_ASSERT(mPending);
@@ -520,9 +549,9 @@ void Manager::Finish(bool aSuccess, const AddressArray &aFailedAddresses)
     {
         if (addr.Matches(kStateRegistering))
         {
-            bool success = aSuccess || !aFailedAddresses.IsEmptyOrContains(addr.GetAddress());
+            bool didRegister = DidRegisterSuccessfully(addr.GetAddress(), aSuccess, aFailedAddresses);
 
-            addr.SetMlrState(success ? kStateRegistered : kStateToRegister);
+            addr.SetMlrState(didRegister ? kStateRegistered : kStateToRegister);
         }
     }
 #endif
@@ -538,9 +567,9 @@ void Manager::Finish(bool aSuccess, const AddressArray &aFailedAddresses)
 
             if (addrEntry.GetMlrState(child) == kStateRegistering)
             {
-                bool success = aSuccess || !aFailedAddresses.IsEmptyOrContains(addrEntry);
+                bool didRegister = DidRegisterSuccessfully(addrEntry, aSuccess, aFailedAddresses);
 
-                addrEntry.SetMlrState(success ? kStateRegistered : kStateToRegister, child);
+                addrEntry.SetMlrState(didRegister ? kStateRegistered : kStateToRegister, child);
             }
         }
     }
diff --git a/src/core/thread/mlr_manager.hpp b/src/core/thread/mlr_manager.hpp
index 9e98324f8..95af09bc5 100644
--- a/src/core/thread/mlr_manager.hpp
+++ b/src/core/thread/mlr_manager.hpp
@@ -146,7 +146,6 @@ private:
     class AddressArray : public Array<Ip6::Address, kMaxIp6Addresses>
     {
     public:
-        bool IsEmptyOrContains(const Ip6::Address &aAddress) const { return IsEmpty() || Contains(aAddress); }
         void AddUnique(const Ip6::Address &aAddress);
     };
 
@@ -161,6 +160,9 @@ private:
     DeclareTmfResponseHandlerIn(Manager, HandleResponse);
 
     static Error ParseResponse(Error aResult, Coap::Msg *aMsg, uint8_t &aStatus, AddressArray &aFailedAddresses);
+    static bool  DidRegisterSuccessfully(const Ip6::Address &aAddress,
+                                         bool                aSuccess,
+                                         const AddressArray &aFailedAddresses);
 
 #if OPENTHREAD_FTD && OPENTHREAD_CONFIG_COMMISSIONER_ENABLE
     DeclareTmfResponseHandlerIn(Manager, HandleRegisterResponse);