[core] add generic three-way comparison function (#8050)

This commit adds a generic `ThreeWayCompare()` function. It also
renames header to `num_utils.hpp` from `min_max.hpp`.

src/core/BUILD.gn

@@ -421,11 +421,11 @@ openthread_core_files = [
   "common/logging.hpp",
   "common/message.cpp",
   "common/message.hpp",
-  "common/min_max.hpp",
   "common/new.hpp",
   "common/non_copyable.hpp",
   "common/notifier.cpp",
   "common/notifier.hpp",
+  "common/num_utils.hpp",
   "common/numeric_limits.hpp",
   "common/owned_ptr.hpp",
   "common/owning_list.hpp",

src/core/Makefile.am

@@ -457,10 +457,10 @@ HEADERS_COMMON                                  = \
     common/log.hpp                                \
     common/logging.hpp                            \
     common/message.hpp                            \
-    common/min_max.hpp                            \
     common/new.hpp                                \
     common/non_copyable.hpp                       \
     common/notifier.hpp                           \
+    common/num_utils.hpp                          \
     common/numeric_limits.hpp                     \
     common/owned_ptr.hpp                          \
     common/owning_list.hpp                        \

src/core/backbone_router/bbr_manager.cpp

@@ -40,7 +40,7 @@
 #include "common/instance.hpp"
 #include "common/locator_getters.hpp"
 #include "common/log.hpp"
-#include "common/min_max.hpp"
+#include "common/num_utils.hpp"
 #include "common/random.hpp"
 #include "thread/mle_types.hpp"
 #include "thread/thread_netif.hpp"

src/core/backbone_router/ndproxy_table.cpp

@@ -38,7 +38,7 @@
 #include "common/array.hpp"
 #include "common/locator_getters.hpp"
 #include "common/log.hpp"
-#include "common/min_max.hpp"
+#include "common/num_utils.hpp"
 
 namespace ot {
 

src/core/border_router/routing_manager.cpp

@@ -46,7 +46,7 @@
 #include "common/instance.hpp"
 #include "common/locator_getters.hpp"
 #include "common/log.hpp"
-#include "common/min_max.hpp"
+#include "common/num_utils.hpp"
 #include "common/random.hpp"
 #include "common/settings.hpp"
 #include "meshcop/extended_panid.hpp"

src/core/common/data.hpp

@@ -44,7 +44,7 @@
 #include "common/const_cast.hpp"
 #include "common/equatable.hpp"
 #include "common/error.hpp"
-#include "common/min_max.hpp"
+#include "common/num_utils.hpp"
 #include "common/type_traits.hpp"
 
 namespace ot {

src/core/common/log.cpp

@@ -39,7 +39,7 @@
 
 #include "common/code_utils.hpp"
 #include "common/instance.hpp"
-#include "common/min_max.hpp"
+#include "common/num_utils.hpp"
 #include "common/string.hpp"
 
 /*

src/core/common/num_utils.hpp

@@ -28,11 +28,11 @@
 
 /**
  * @file
- *   This file includes definitions for generic min, max and clamp functions.
+ *   This file includes definitions for generic number utility functions (min, max, clamp).
  */
 
-#ifndef MIN_MAX_HPP_
-#define MIN_MAX_HPP_
+#ifndef NUM_UTILS_HPP_
+#define NUM_UTILS_HPP_
 
 #include "common/numeric_limits.hpp"
 #include "common/type_traits.hpp"
@@ -40,7 +40,7 @@
 namespace ot {
 
 /**
- * This template method returns the minimum of two given values.
+ * This template function returns the minimum of two given values.
  *
  * Uses `operator<` to compare the values.
  *
@@ -58,7 +58,7 @@ template <typename Type> Type Min(Type aFirst, Type aSecond)
 }
 
 /**
- * This template method returns the maximum of two given values.
+ * This template function returns the maximum of two given values.
  *
  * Uses `operator<` to compare the values.
  *
@@ -76,7 +76,7 @@ template <typename Type> Type Max(Type aFirst, Type aSecond)
 }
 
 /**
- * This template method returns clamped version of a given value to a given closed range [min, max].
+ * This template function returns clamped version of a given value to a given closed range [min, max].
  *
  * Uses `operator<` to compare the values. The behavior is undefined if the value of @p aMin is greater than @p aMax.
  *
@@ -97,7 +97,7 @@ template <typename Type> Type Clamp(Type aValue, Type aMin, Type aMax)
 }
 
 /**
- * This template method returns a clamped version of given integer to a `uint8_t`.
+ * This template function returns a clamped version of given integer to a `uint8_t`.
  *
  * If @p aValue is greater than max value of a `uint8_t`, the max value is returned.
  *
@@ -118,7 +118,7 @@ template <typename UintType> uint8_t ClampToUint8(UintType aValue)
 }
 
 /**
- * This template method returns a clamped version of given integer to a `uint16_t`.
+ * This template function returns a clamped version of given integer to a `uint16_t`.
  *
  * If @p aValue is greater than max value of a `uint16_t`, the max value is returned.
  *
@@ -137,6 +137,40 @@ template <typename UintType> uint16_t ClampToUint16(UintType aValue)
     return static_cast<uint16_t>(Min(aValue, static_cast<UintType>(NumericLimits<uint16_t>::kMax)));
 }
 
+/**
+ * This template function performs a three-way comparison between two values.
+ *
+ * @tparam Type   The value type.
+ *
+ * @param[in] aFirst  The first value.
+ * @param[in] aSecond The second value.
+ *
+ * @retval 1    If @p aFirst >  @p aSecond.
+ * @retval 0    If @p aFirst == @p aSecond.
+ * @retval -1   If @p aFirst <  @p aSecond.
+ *
+ */
+template <typename Type> int ThreeWayCompare(Type aFirst, Type aSecond)
+{
+    return (aFirst == aSecond) ? 0 : ((aFirst > aSecond) ? 1 : -1);
+}
+
+/**
+ * This is template specialization of three-way comparison between two boolean values.
+ *
+ * @param[in] aFirst  The first boolean value.
+ * @param[in] aSecond The second boolean value.
+ *
+ * @retval 1    If @p aFirst is true and @p aSecond is false (true > false).
+ * @retval 0    If both @p aFirst and @p aSecond are true, or both are false (they are equal).
+ * @retval -1   If @p aFirst is false and @p aSecond is true (false < true).
+ *
+ */
+template <> inline int ThreeWayCompare(bool aFirst, bool aSecond)
+{
+    return (aFirst == aSecond) ? 0 : (aFirst ? 1 : -1);
+}
+
 } // namespace ot
 
-#endif // MIN_MAX_HPP_
+#endif // NUM_UTILS_HPP_

src/core/common/string.hpp

@@ -43,6 +43,7 @@
 #include "common/binary_search.hpp"
 #include "common/code_utils.hpp"
 #include "common/error.hpp"
+#include "common/num_utils.hpp"
 
 namespace ot {
 
@@ -405,7 +406,7 @@ public:
         const char *mString; ///< The associated string.
 
     private:
-        int Compare(uint16_t aKey) const { return (aKey == mKey) ? 0 : ((aKey > mKey) ? 1 : -1); }
+        int Compare(uint16_t aKey) const { return ThreeWayCompare(aKey, mKey); }
 
         constexpr static bool AreInOrder(const Entry &aFirst, const Entry &aSecond)
         {

src/core/mac/data_poll_sender.cpp

@@ -38,7 +38,7 @@
 #include "common/locator_getters.hpp"
 #include "common/log.hpp"
 #include "common/message.hpp"
-#include "common/min_max.hpp"
+#include "common/num_utils.hpp"
 #include "net/ip6.hpp"
 #include "net/netif.hpp"
 #include "thread/mesh_forwarder.hpp"

src/core/mac/sub_mac.cpp

@@ -42,7 +42,7 @@
 #include "common/instance.hpp"
 #include "common/locator_getters.hpp"
 #include "common/log.hpp"
-#include "common/min_max.hpp"
+#include "common/num_utils.hpp"
 #include "common/random.hpp"
 #include "common/time.hpp"
 #include "mac/mac_frame.hpp"

src/core/meshcop/meshcop_tlvs.cpp

@@ -35,7 +35,7 @@
 
 #include "common/const_cast.hpp"
 #include "common/debug.hpp"
-#include "common/min_max.hpp"
+#include "common/num_utils.hpp"
 #include "common/string.hpp"
 #include "meshcop/meshcop.hpp"
 

src/core/meshcop/timestamp.cpp

@@ -34,6 +34,7 @@
 #include "timestamp.hpp"
 
 #include "common/code_utils.hpp"
+#include "common/num_utils.hpp"
 
 namespace ot {
 namespace MeshCoP {
@@ -80,39 +81,15 @@ exit:
 
 int Timestamp::Compare(const Timestamp &aFirst, const Timestamp &aSecond)
 {
-    int      rval;
-    uint64_t firstSeconds;
-    uint64_t secondSeconds;
-    uint16_t firstTicks;
-    uint16_t secondTicks;
-    bool     firstAuthoritative;
-    bool     secondAuthoritative;
+    int rval;
 
-    firstSeconds  = aFirst.GetSeconds();
-    secondSeconds = aSecond.GetSeconds();
+    rval = ThreeWayCompare(aFirst.GetSeconds(), aSecond.GetSeconds());
+    VerifyOrExit(rval == 0);
 
-    if (firstSeconds != secondSeconds)
-    {
-        ExitNow(rval = (firstSeconds > secondSeconds) ? 1 : -1);
-    }
+    rval = ThreeWayCompare(aFirst.GetTicks(), aSecond.GetTicks());
+    VerifyOrExit(rval == 0);
 
-    firstTicks  = aFirst.GetTicks();
-    secondTicks = aSecond.GetTicks();
-
-    if (firstTicks != secondTicks)
-    {
-        ExitNow(rval = (firstTicks > secondTicks) ? 1 : -1);
-    }
-
-    firstAuthoritative  = aFirst.GetAuthoritative();
-    secondAuthoritative = aSecond.GetAuthoritative();
-
-    if (firstAuthoritative != secondAuthoritative)
-    {
-        ExitNow(rval = firstAuthoritative ? 1 : -1);
-    }
-
-    rval = 0;
+    rval = ThreeWayCompare(aFirst.GetAuthoritative(), aSecond.GetAuthoritative());
 
 exit:
     return rval;

src/core/net/dns_dso.cpp

@@ -37,7 +37,7 @@
 #include "common/instance.hpp"
 #include "common/locator_getters.hpp"
 #include "common/log.hpp"
-#include "common/min_max.hpp"
+#include "common/num_utils.hpp"
 #include "common/random.hpp"
 
 /**

src/core/net/dns_dso.hpp

@@ -42,8 +42,8 @@
 #include "common/linked_list.hpp"
 #include "common/locator.hpp"
 #include "common/message.hpp"
-#include "common/min_max.hpp"
 #include "common/non_copyable.hpp"
+#include "common/num_utils.hpp"
 #include "common/timer.hpp"
 #include "net/dns_types.hpp"
 #include "net/socket.hpp"

src/core/net/dns_types.cpp

@@ -36,7 +36,7 @@
 #include "common/code_utils.hpp"
 #include "common/debug.hpp"
 #include "common/instance.hpp"
-#include "common/min_max.hpp"
+#include "common/num_utils.hpp"
 #include "common/random.hpp"
 #include "common/string.hpp"
 

src/core/net/ip6_address.cpp

@@ -40,7 +40,7 @@
 #include "common/code_utils.hpp"
 #include "common/encoding.hpp"
 #include "common/instance.hpp"
-#include "common/min_max.hpp"
+#include "common/num_utils.hpp"
 #include "common/numeric_limits.hpp"
 #include "common/random.hpp"
 #include "net/ip4_types.hpp"

src/core/net/srp_client.cpp

@@ -35,7 +35,7 @@
 #include "common/debug.hpp"
 #include "common/instance.hpp"
 #include "common/locator_getters.hpp"
-#include "common/min_max.hpp"
+#include "common/num_utils.hpp"
 #include "common/random.hpp"
 #include "common/settings.hpp"
 #include "common/string.hpp"

src/core/net/srp_server.cpp

@@ -40,8 +40,8 @@
 #include "common/instance.hpp"
 #include "common/locator_getters.hpp"
 #include "common/log.hpp"
-#include "common/min_max.hpp"
 #include "common/new.hpp"
+#include "common/num_utils.hpp"
 #include "common/random.hpp"
 #include "net/dns_types.hpp"
 #include "thread/thread_netif.hpp"

src/core/net/srp_server.hpp

@@ -63,9 +63,9 @@
 #include "common/heap_string.hpp"
 #include "common/linked_list.hpp"
 #include "common/locator.hpp"
-#include "common/min_max.hpp"
 #include "common/non_copyable.hpp"
 #include "common/notifier.hpp"
+#include "common/num_utils.hpp"
 #include "common/numeric_limits.hpp"
 #include "common/retain_ptr.hpp"
 #include "common/timer.hpp"

src/core/net/tcp6.cpp

@@ -43,7 +43,7 @@
 #include "common/instance.hpp"
 #include "common/locator_getters.hpp"
 #include "common/log.hpp"
-#include "common/min_max.hpp"
+#include "common/num_utils.hpp"
 #include "common/random.hpp"
 #include "net/checksum.hpp"
 #include "net/ip6.hpp"

src/core/thread/link_quality.cpp

@@ -38,7 +38,7 @@
 #include "common/code_utils.hpp"
 #include "common/instance.hpp"
 #include "common/locator_getters.hpp"
-#include "common/min_max.hpp"
+#include "common/num_utils.hpp"
 
 namespace ot {
 

src/core/thread/mesh_forwarder_ftd.cpp

@@ -36,7 +36,7 @@
 #if OPENTHREAD_FTD
 
 #include "common/locator_getters.hpp"
-#include "common/min_max.hpp"
+#include "common/num_utils.hpp"
 #include "meshcop/meshcop.hpp"
 #include "net/ip6.hpp"
 #include "net/tcp6.hpp"

src/core/thread/mle.cpp

@@ -43,7 +43,7 @@
 #include "common/encoding.hpp"
 #include "common/instance.hpp"
 #include "common/locator_getters.hpp"
-#include "common/min_max.hpp"
+#include "common/num_utils.hpp"
 #include "common/random.hpp"
 #include "common/serial_number.hpp"
 #include "common/settings.hpp"
@@ -2988,57 +2988,40 @@ bool Mle::IsBetterParent(uint16_t                aRloc16,
                          uint16_t                aVersion,
                          const Mac::CslAccuracy &aCslAccuracy)
 {
-    bool        rval                       = false;
+    int         rval;
     LinkQuality candidateTwoWayLinkQuality = mParentCandidate.GetTwoWayLinkQuality();
 
     // Mesh Impacting Criteria
-    if (aLinkQuality != candidateTwoWayLinkQuality)
-    {
-        ExitNow(rval = (aLinkQuality > candidateTwoWayLinkQuality));
-    }
+    rval = ThreeWayCompare(aLinkQuality, candidateTwoWayLinkQuality);
+    VerifyOrExit(rval == 0);
 
-    if (IsActiveRouter(aRloc16) != IsActiveRouter(mParentCandidate.GetRloc16()))
-    {
-        ExitNow(rval = IsActiveRouter(aRloc16));
-    }
+    rval = ThreeWayCompare(IsActiveRouter(aRloc16), IsActiveRouter(mParentCandidate.GetRloc16()));
+    VerifyOrExit(rval == 0);
 
-    if (aConnectivityTlv.GetParentPriority() != mParentPriority)
-    {
-        ExitNow(rval = (aConnectivityTlv.GetParentPriority() > mParentPriority));
-    }
+    rval = ThreeWayCompare(aConnectivityTlv.GetParentPriority(), mParentPriority);
+    VerifyOrExit(rval == 0);
 
     // Prefer the parent with highest quality links (Link Quality 3 field in Connectivity TLV) to neighbors
-    if (aConnectivityTlv.GetLinkQuality3() != mParentLinkQuality3)
-    {
-        ExitNow(rval = (aConnectivityTlv.GetLinkQuality3() > mParentLinkQuality3));
-    }
+    rval = ThreeWayCompare(aConnectivityTlv.GetLinkQuality3(), mParentLinkQuality3);
+    VerifyOrExit(rval == 0);
 
     // Thread 1.2 Specification 4.5.2.1.2 Child Impacting Criteria
-    if (aVersion != mParentCandidate.GetVersion())
-    {
-        ExitNow(rval = (aVersion > mParentCandidate.GetVersion()));
-    }
 
-    if (aConnectivityTlv.GetSedBufferSize() != mParentSedBufferSize)
-    {
-        ExitNow(rval = (aConnectivityTlv.GetSedBufferSize() > mParentSedBufferSize));
-    }
+    rval = ThreeWayCompare(aVersion, mParentCandidate.GetVersion());
+    VerifyOrExit(rval == 0);
 
-    if (aConnectivityTlv.GetSedDatagramCount() != mParentSedDatagramCount)
-    {
-        ExitNow(rval = (aConnectivityTlv.GetSedDatagramCount() > mParentSedDatagramCount));
-    }
+    rval = ThreeWayCompare(aConnectivityTlv.GetSedBufferSize(), mParentSedBufferSize);
+    VerifyOrExit(rval == 0);
+
+    rval = ThreeWayCompare(aConnectivityTlv.GetSedDatagramCount(), mParentSedDatagramCount);
+    VerifyOrExit(rval == 0);
 
     // Extra rules
-    if (aConnectivityTlv.GetLinkQuality2() != mParentLinkQuality2)
-    {
-        ExitNow(rval = (aConnectivityTlv.GetLinkQuality2() > mParentLinkQuality2));
-    }
+    rval = ThreeWayCompare(aConnectivityTlv.GetLinkQuality2(), mParentLinkQuality2);
+    VerifyOrExit(rval == 0);
 
-    if (aConnectivityTlv.GetLinkQuality1() != mParentLinkQuality1)
-    {
-        ExitNow(rval = (aConnectivityTlv.GetLinkQuality1() > mParentLinkQuality1));
-    }
+    rval = ThreeWayCompare(aConnectivityTlv.GetLinkQuality1(), mParentLinkQuality1);
+    VerifyOrExit(rval == 0);
 
 #if OPENTHREAD_CONFIG_MAC_CSL_RECEIVER_ENABLE
     // CSL metric
@@ -3047,19 +3030,18 @@ bool Mle::IsBetterParent(uint16_t                aRloc16,
         uint64_t cslMetric          = CalcParentCslMetric(aCslAccuracy);
         uint64_t candidateCslMetric = CalcParentCslMetric(mParentCandidate.GetCslAccuracy());
 
-        if (candidateCslMetric != cslMetric)
-        {
-            ExitNow(rval = (cslMetric < candidateCslMetric));
-        }
+        // Smaller metric is better.
+        rval = ThreeWayCompare(candidateCslMetric, cslMetric);
+        VerifyOrExit(rval == 0);
     }
 #else
     OT_UNUSED_VARIABLE(aCslAccuracy);
 #endif
 
-    rval = (aLinkMargin > mParentLinkMargin);
+    rval = ThreeWayCompare(aLinkMargin, mParentLinkMargin);
 
 exit:
-    return rval;
+    return (rval > 0);
 }
 
 void Mle::HandleParentResponse(RxInfo &aRxInfo)

src/core/thread/mle_router.cpp

@@ -40,6 +40,7 @@
 #include "common/encoding.hpp"
 #include "common/instance.hpp"
 #include "common/locator_getters.hpp"
+#include "common/num_utils.hpp"
 #include "common/random.hpp"
 #include "common/serial_number.hpp"
 #include "common/settings.hpp"
@@ -1184,20 +1185,14 @@ int MleRouter::ComparePartitions(bool              aSingletonA,
 {
     int rval = 0;
 
-    if (aLeaderDataA.GetWeighting() != aLeaderDataB.GetWeighting())
-    {
-        ExitNow(rval = aLeaderDataA.GetWeighting() > aLeaderDataB.GetWeighting() ? 1 : -1);
-    }
+    rval = ThreeWayCompare(aLeaderDataA.GetWeighting(), aLeaderDataB.GetWeighting());
+    VerifyOrExit(rval == 0);
 
-    if (aSingletonA != aSingletonB)
-    {
-        ExitNow(rval = aSingletonB ? 1 : -1);
-    }
+    // Not being a singleton is better.
+    rval = ThreeWayCompare(!aSingletonA, !aSingletonB);
+    VerifyOrExit(rval == 0);
 
-    if (aLeaderDataA.GetPartitionId() != aLeaderDataB.GetPartitionId())
-    {
-        ExitNow(rval = aLeaderDataA.GetPartitionId() > aLeaderDataB.GetPartitionId() ? 1 : -1);
-    }
+    rval = ThreeWayCompare(aLeaderDataA.GetPartitionId(), aLeaderDataB.GetPartitionId());
 
 exit:
     return rval;

src/core/thread/topology.cpp

@@ -38,7 +38,7 @@
 #include "common/debug.hpp"
 #include "common/instance.hpp"
 #include "common/locator_getters.hpp"
-#include "common/min_max.hpp"
+#include "common/num_utils.hpp"
 
 namespace ot {
 

src/core/utils/history_tracker.cpp

@@ -40,7 +40,7 @@
 #include "common/debug.hpp"
 #include "common/instance.hpp"
 #include "common/locator_getters.hpp"
-#include "common/min_max.hpp"
+#include "common/num_utils.hpp"
 #include "common/string.hpp"
 #include "common/timer.hpp"
 #include "net/ip6_headers.hpp"

src/core/utils/ping_sender.cpp

@@ -38,7 +38,7 @@
 #include "common/as_core_type.hpp"
 #include "common/encoding.hpp"
 #include "common/locator_getters.hpp"
-#include "common/min_max.hpp"
+#include "common/num_utils.hpp"
 #include "common/random.hpp"
 
 namespace ot {

tests/unit/test_serial_number.cpp

@@ -32,7 +32,7 @@
 
 #include "test_util.h"
 #include "common/code_utils.hpp"
-#include "common/min_max.hpp"
+#include "common/num_utils.hpp"
 #include "common/numeric_limits.hpp"
 #include "common/serial_number.hpp"
 
@@ -66,7 +66,7 @@ template <typename UintType> void TestSerialNumber(const char *aName)
     printf("TestSerialNumber<%s>() passed\n", aName);
 }
 
-void TestMinMaxClamp(void)
+void TestNumUtils(void)
 {
     uint16_t u16;
     uint32_t u32;
@@ -109,7 +109,16 @@ void TestMinMaxClamp(void)
     u32 = 0xfff0000;
     VerifyOrQuit(ClampToUint16(u32) == 0xffff);
 
-    printf("TestMinMaxClamp() passed\n");
+    VerifyOrQuit(ThreeWayCompare<uint8_t>(2, 2) == 0);
+    VerifyOrQuit(ThreeWayCompare<uint8_t>(2, 1) > 0);
+    VerifyOrQuit(ThreeWayCompare<uint8_t>(1, 2) < 0);
+
+    VerifyOrQuit(ThreeWayCompare<bool>(false, false) == 0);
+    VerifyOrQuit(ThreeWayCompare<bool>(true, true) == 0);
+    VerifyOrQuit(ThreeWayCompare<bool>(true, false) > 0);
+    VerifyOrQuit(ThreeWayCompare<bool>(false, true) < 0);
+
+    printf("TestNumUtils() passed\n");
 }
 
 } // namespace ot
@@ -120,7 +129,7 @@ int main(void)
     ot::TestSerialNumber<uint16_t>("uint16_t");
     ot::TestSerialNumber<uint32_t>("uint32_t");
     ot::TestSerialNumber<uint64_t>("uint64_t");
-    ot::TestMinMaxClamp();
+    ot::TestNumUtils();
     printf("\nAll tests passed.\n");
     return 0;
 }