From 832e61c1c4eb5a4b8ee3394fb5cfcd7945ef8e25 Mon Sep 17 00:00:00 2001
From: Xiang Xiao <xiaoxiang@xiaomi.com>
Date: Mon, 24 Feb 2025 21:57:52 +0800
Subject: [PATCH] netutils/ptpd: Add the missing comparison in is_better_clock

The original implementation only checked if each field was less than,
but didn't check for greater than before proceeding to the next field.
This caused incorrect clock selection behavior in the PTP Best Master
Clock Algorithm (BMCA).

The fix expands each comparison to explicitly check both < and >
conditions, returning the appropriate result immediately. This ensures
proper precedence evaluation according to IEEE 1588 specification:
- gm_priority1
- gm_quality (class, accuracy, variance)
- gm_priority2
- gm_identity

Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Signed-off-by: dongjiuzhu1 <dongjiuzhu1@xiaomi.com>
---
 netutils/ptpd/ptpd.c | 75 +++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 67 insertions(+), 8 deletions(-)

diff --git a/netutils/ptpd/ptpd.c b/netutils/ptpd/ptpd.c
index 4ba8d2c86..2003e5a08 100644
--- a/netutils/ptpd/ptpd.c
+++ b/netutils/ptpd/ptpd.c
@@ -231,20 +231,79 @@ static void ptp_format_to_timespec(FAR const uint8_t *timestamp,
 static bool is_better_clock(FAR const struct ptp_announce_s *a,
                             FAR const struct ptp_announce_s *b)
 {
-  if  (a->gm_priority1 < b->gm_priority1     /* Main priority field */
-    || a->gm_quality[0] < b->gm_quality[0]   /* Clock class */
-    || a->gm_quality[1] < b->gm_quality[1]   /* Clock accuracy */
-    || a->gm_quality[2] < b->gm_quality[2]   /* Clock variance high byte */
-    || a->gm_quality[3] < b->gm_quality[3]   /* Clock variance low byte */
-    || a->gm_priority2 < b->gm_priority2     /* Sub priority field */
-    || memcmp(a->gm_identity, b->gm_identity, sizeof(a->gm_identity)) < 0)
+  /* Main priority field */
+
+  if (a->gm_priority1 < b->gm_priority1)
     {
       return true;
     }
-    else
+
+  if (a->gm_priority1 > b->gm_priority1)
     {
       return false;
     }
+
+  /* Clock class */
+
+  if (a->gm_quality[0] < b->gm_quality[0])
+    {
+      return true;
+    }
+
+  if (a->gm_quality[0] > b->gm_quality[0])
+    {
+      return false;
+    }
+
+  /* Clock accuracy */
+
+  if (a->gm_quality[1] < b->gm_quality[1])
+    {
+      return true;
+    }
+
+  if (a->gm_quality[1] > b->gm_quality[1])
+    {
+      return false;
+    }
+
+  /* Clock variance high byte */
+
+  if (a->gm_quality[2] < b->gm_quality[2])
+    {
+      return true;
+    }
+
+  if (a->gm_quality[2] > b->gm_quality[2])
+    {
+      return false;
+    }
+
+  /* Clock variance low byte */
+
+  if (a->gm_quality[3] < b->gm_quality[3])
+    {
+      return true;
+    }
+
+  if (a->gm_quality[3] > b->gm_quality[3])
+    {
+      return false;
+    }
+
+  /* Sub priority field */
+
+  if (a->gm_priority2 < b->gm_priority2)
+    {
+      return true;
+    }
+
+  if (a->gm_priority2 > b->gm_priority2)
+    {
+      return false;
+    }
+
+  return memcmp(a->gm_identity, b->gm_identity, sizeof(a->gm_identity)) < 0;
 }
 
 static int64_t timespec_to_ms(FAR const struct timespec *ts)