Use safer System::Clock types in dnssd (#11062)

* Use safer System::Clock types in dnssd

#### Problem

Code uses plain integers to represent time values and relies on
users to get the unit scale correct.

Part of #10062 _Some operations on System::Clock types are not safe_

#### Change overview

Convert `src/lib/dnssd` to use the safer `Clock` types.

#### Testing

CI; no change to functionality intended.

Conversion includes `TestActiveResolveAttempts.cpp`.

* restyle

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