Use safer System::Clock types in dnssd

src/lib/dnssd/Resolver_ImplMinimalMdns.cpp

@@ -513,14 +513,14 @@ CHIP_ERROR MinMdnsResolver::ScheduleResolveRetries()
     ReturnErrorCodeIf(mSystemLayer == nullptr, CHIP_ERROR_INCORRECT_STATE);
     mSystemLayer->CancelTimer(&ResolveRetryCallback, this);
 
-    Optional<uint32_t> delayMs = mActiveResolves.GetMsUntilNextExpectedResponse();
+    Optional<System::Clock::Timeout> delay = mActiveResolves.GetTimeUntilNextExpectedResponse();
 
-    if (!delayMs.HasValue())
+    if (!delay.HasValue())
     {
         return CHIP_NO_ERROR;
     }
 
-    return mSystemLayer->StartTimer(System::Clock::Milliseconds32(delayMs.Value()), &ResolveRetryCallback, this);
+    return mSystemLayer->StartTimer(delay.Value(), &ResolveRetryCallback, this);
 }
 
 void MinMdnsResolver::ResolveRetryCallback(System::Layer *, void * self)

src/lib/dnssd/minimal_mdns/ActiveResolveAttempts.cpp

@@ -26,6 +26,8 @@ using namespace chip;
 namespace mdns {
 namespace Minimal {
 
+constexpr chip::System::Clock::Timeout ActiveResolveAttempts::kMaxRetryDelay;
+
 void ActiveResolveAttempts::Reset()
 
 {
@@ -56,9 +58,9 @@ void ActiveResolveAttempts::MarkPending(const PeerId & peerId)
     // Strategy when picking the peer id to use:
     //   1 if a matching peer id is already found, use that one
     //   2 if an 'unused' entry is found, use that
-    //   3 otherwise expire the one with the largest nextRetryDelaySec
-    //     or if equal nextRetryDelaySec, pick the one with the oldest
-    //     queryDueTimeMs
+    //   3 otherwise expire the one with the largest nextRetryDelay
+    //     or if equal nextRetryDelay, pick the one with the oldest
+    //     queryDueTime
 
     RetryEntry * entryToUse = &mRetryQueue[0];
 
@@ -94,12 +96,11 @@ void ActiveResolveAttempts::MarkPending(const PeerId & peerId)
         //    - on same delay, use queryDueTime to determine the oldest request
         //      (the one with the smallest  due time was issued the longest time
         //       ago)
-        if (entry->nextRetryDelaySec > entryToUse->nextRetryDelaySec)
+        if (entry->nextRetryDelay > entryToUse->nextRetryDelay)
         {
             entryToUse = entry;
         }
-        else if ((entry->nextRetryDelaySec == entryToUse->nextRetryDelaySec) &&
-                 (entry->queryDueTimeMs < entryToUse->queryDueTimeMs))
+        else if ((entry->nextRetryDelay == entryToUse->nextRetryDelay) && (entry->queryDueTime < entryToUse->queryDueTime))
         {
             entryToUse = entry;
         }
@@ -117,16 +118,16 @@ void ActiveResolveAttempts::MarkPending(const PeerId & peerId)
         ChipLogError(Discovery, "Re-using pending resolve entry before reply was received.");
     }
 
-    entryToUse->peerId            = peerId;
-    entryToUse->queryDueTimeMs    = mClock->GetMonotonicMilliseconds();
-    entryToUse->nextRetryDelaySec = 1;
+    entryToUse->peerId         = peerId;
+    entryToUse->queryDueTime   = mClock->GetMonotonicTimestamp();
+    entryToUse->nextRetryDelay = System::Clock::Seconds16(1);
 }
 
-Optional<uint32_t> ActiveResolveAttempts::GetMsUntilNextExpectedResponse() const
+Optional<System::Clock::Timeout> ActiveResolveAttempts::GetTimeUntilNextExpectedResponse() const
 {
-    Optional<uint32_t> minDelay = Optional<uint32_t>::Missing();
+    Optional<System::Clock::Timeout> minDelay = Optional<System::Clock::Timeout>::Missing();
 
-    chip::System::Clock::MonotonicMilliseconds nowMs = mClock->GetMonotonicMilliseconds();
+    chip::System::Clock::Timestamp now = mClock->GetMonotonicTimestamp();
 
     for (auto & entry : mRetryQueue)
     {
@@ -135,13 +136,13 @@ Optional<uint32_t> ActiveResolveAttempts::GetMsUntilNextExpectedResponse() const
             continue;
         }
 
-        if (nowMs >= entry.queryDueTimeMs)
+        if (now >= entry.queryDueTime)
         {
             // found an entry that needs processing right now
-            return Optional<uint32_t>::Value(0);
+            return Optional<System::Clock::Timeout>::Value(0);
         }
 
-        uint32_t entryDelay = static_cast<int>(entry.queryDueTimeMs - nowMs);
+        System::Clock::Timeout entryDelay = entry.queryDueTime - now;
         if (!minDelay.HasValue() || (minDelay.Value() > entryDelay))
         {
             minDelay.SetValue(entryDelay);
@@ -153,7 +154,7 @@ Optional<uint32_t> ActiveResolveAttempts::GetMsUntilNextExpectedResponse() const
 
 Optional<PeerId> ActiveResolveAttempts::NextScheduledPeer()
 {
-    chip::System::Clock::MonotonicMilliseconds nowMs = mClock->GetMonotonicMilliseconds();
+    chip::System::Clock::Timestamp now = mClock->GetMonotonicTimestamp();
 
     for (auto & entry : mRetryQueue)
     {
@@ -162,20 +163,20 @@ Optional<PeerId> ActiveResolveAttempts::NextScheduledPeer()
             continue; // not a pending item
         }
 
-        if (entry.queryDueTimeMs > nowMs)
+        if (entry.queryDueTime > now)
         {
             continue; // not yet due
         }
 
-        if (entry.nextRetryDelaySec > kMaxRetryDelaySec)
+        if (entry.nextRetryDelay > kMaxRetryDelay)
         {
             ChipLogError(Discovery, "Timeout waiting for mDNS resolution.");
             entry.peerId.SetNodeId(kUndefinedNodeId);
             continue;
         }
 
-        entry.queryDueTimeMs = nowMs + entry.nextRetryDelaySec * 1000L;
-        entry.nextRetryDelaySec *= 2;
+        entry.queryDueTime = now + entry.nextRetryDelay;
+        entry.nextRetryDelay *= 2;
 
         return Optional<PeerId>::Value(entry.peerId);
     }

src/lib/dnssd/minimal_mdns/ActiveResolveAttempts.h

@@ -38,8 +38,8 @@ namespace Minimal {
 class ActiveResolveAttempts
 {
 public:
-    static constexpr size_t kRetryQueueSize     = 4;
-    static constexpr uint32_t kMaxRetryDelaySec = 16;
+    static constexpr size_t kRetryQueueSize                      = 4;
+    static constexpr chip::System::Clock::Timeout kMaxRetryDelay = chip::System::Clock::Seconds16(16);
 
     ActiveResolveAttempts(chip::System::Clock::ClockBase * clock) : mClock(clock) { Reset(); }
 
@@ -55,10 +55,10 @@ class ActiveResolveAttempts
     /// by NextScheduledPeer (potentially with others as well)
     void MarkPending(const chip::PeerId & peerId);
 
-    // Get minimum milliseconds until the next pending reply is required.
+    // Get minimum time until the next pending reply is required.
     //
     // Returns missing if no actively tracked elements exist.
-    chip::Optional<uint32_t> GetMsUntilNextExpectedResponse() const;
+    chip::Optional<chip::System::Clock::Timeout> GetTimeUntilNextExpectedResponse() const;
 
     // Get the peer Id that needs scheduling for a query
     //
@@ -79,7 +79,7 @@ class ActiveResolveAttempts
         chip::PeerId peerId;
 
         // When a reply is expected for this item
-        chip::System::Clock::MonotonicMilliseconds queryDueTimeMs;
+        chip::System::Clock::Timestamp queryDueTime;
 
         // Next expected delay for sending if reply is not reached by
         // 'queryDueTimeMs'
@@ -89,7 +89,7 @@ class ActiveResolveAttempts
         //      one second
         //    - the intervals between successive queries MUST increase by at
         //      least a factor of two
-        uint32_t nextRetryDelaySec = 1;
+        chip::System::Clock::Timeout nextRetryDelay = chip::System::Clock::Seconds16(1);
     };
 
     chip::System::Clock::ClockBase * mClock;

src/lib/dnssd/minimal_mdns/ResponseSender.cpp

@@ -89,7 +89,7 @@ CHIP_ERROR ResponseSender::Respond(uint32_t messageId, const QueryData & query,
 
     // send all 'Answer' replies
     {
-        const uint64_t kTimeNowMs = chip::System::SystemClock().GetMonotonicMilliseconds();
+        const chip::System::Clock::Timestamp kTimeNow = chip::System::SystemClock().GetMonotonicTimestamp();
 
         QueryReplyFilter queryReplyFilter(query);
         QueryResponderRecordFilter responseFilter;
@@ -102,8 +102,7 @@ CHIP_ERROR ResponseSender::Respond(uint32_t messageId, const QueryData & query,
             //
             // TODO: the 'last sent' value does NOT track the interface we used to send, so this may cause
             //       broadcasts on one interface to throttle broadcasts on another interface.
-            constexpr uint64_t kOneSecondMs = 1000;
-            responseFilter.SetIncludeOnlyMulticastBeforeMS(kTimeNowMs - kOneSecondMs);
+            responseFilter.SetIncludeOnlyMulticastBeforeMS(kTimeNow - chip::System::Clock::Seconds32(1));
         }
         for (size_t i = 0; i < kMaxQueryResponders; ++i)
         {
@@ -120,7 +119,7 @@ CHIP_ERROR ResponseSender::Respond(uint32_t messageId, const QueryData & query,
 
                 if (!mSendState.SendUnicast())
                 {
-                    it->lastMulticastTime = kTimeNowMs;
+                    it->lastMulticastTime = kTimeNow;
                 }
             }
         }

src/lib/dnssd/minimal_mdns/responders/QueryResponder.cpp

@@ -160,7 +160,7 @@ void QueryResponderBase::ClearBroadcastThrottle()
 {
     for (size_t i = 0; i < mResponderInfoSize; i++)
     {
-        mResponderInfos[i].lastMulticastTime = 0;
+        mResponderInfos[i].lastMulticastTime = chip::System::Clock::Zero;
     }
 }
 

src/lib/dnssd/minimal_mdns/responders/QueryResponder.h

@@ -29,9 +29,9 @@ namespace Minimal {
 /// Represents available data (replies) for mDNS queries.
 struct QueryResponderRecord
 {
-    Responder * responder      = nullptr; // what response/data is available
-    bool reportService         = false;   // report as a service when listing dnssd services
-    uint64_t lastMulticastTime = 0;       // last time this record was multicast
+    Responder * responder = nullptr;                       // what response/data is available
+    bool reportService    = false;                         // report as a service when listing dnssd services
+    chip::System::Clock::Timestamp lastMulticastTime{ 0 }; // last time this record was multicast
 };
 
 namespace Internal {
@@ -122,9 +122,9 @@ class QueryResponderRecordFilter
 
     /// Filter out anything that was multicast past ms.
     /// If ms is 0, no filtering is done
-    QueryResponderRecordFilter & SetIncludeOnlyMulticastBeforeMS(uint64_t ms)
+    QueryResponderRecordFilter & SetIncludeOnlyMulticastBeforeMS(chip::System::Clock::Timestamp time)
     {
-        mIncludeOnlyMulticastBeforeMS = ms;
+        mIncludeOnlyMulticastBefore = time;
         return *this;
     }
 
@@ -140,7 +140,7 @@ class QueryResponderRecordFilter
             return false;
         }
 
-        if ((mIncludeOnlyMulticastBeforeMS > 0) && (record->lastMulticastTime >= mIncludeOnlyMulticastBeforeMS))
+        if ((mIncludeOnlyMulticastBefore > chip::System::Clock::Zero) && (record->lastMulticastTime >= mIncludeOnlyMulticastBefore))
         {
             return false;
         }
@@ -154,9 +154,9 @@ class QueryResponderRecordFilter
     }
 
 private:
-    bool mIncludeAdditionalRepliesOnly     = false;
-    ReplyFilter * mReplyFilter             = nullptr;
-    uint64_t mIncludeOnlyMulticastBeforeMS = 0;
+    bool mIncludeAdditionalRepliesOnly = false;
+    ReplyFilter * mReplyFilter         = nullptr;
+    chip::System::Clock::Timestamp mIncludeOnlyMulticastBefore{ 0 };
 };
 
 /// Iterates over an array of QueryResponderRecord items, providing only 'valid' ones, where