[message] add range-based for loop for message/priority queue (#7481)

This commit adds support for range-based `for` loop iteration over
`MessageQueue` and `PriorityQueue`. It adds `ConstIterator` and
`Iterator` in `Message` class. The non-const `for` loop iteration
(which uses `Message::Iterator`) works properly and is safe to use
even when the entry is removed from the queue during iteration. This
commit updates other core modules to use the new `for` loop iteration
model which helps simplify the code. It also updates the unit tests
to validate the behavior of the newly added iteration mechanisms.

src/core/coap/coap.cpp

@@ -79,18 +79,15 @@ void CoapBase::ClearRequests(const Ip6::Address &aAddress)
 
 void CoapBase::ClearRequests(const Ip6::Address *aAddress)
 {
- Message *nextMessage;
-
- for (Message *message = mPendingRequests.GetHead(); message != nullptr; message = nextMessage)
+ for (Message &message : mPendingRequests)
  {
  Metadata metadata;
 
- nextMessage = message->GetNextCoapMessage();
- metadata.ReadFrom(*message);
+ metadata.ReadFrom(message);
 
  if ((aAddress == nullptr) || (metadata.mSourceAddress == *aAddress))
  {
- FinalizeCoapTransaction(*message, metadata, nullptr, nullptr, kErrorAbort);
+ FinalizeCoapTransaction(message, metadata, nullptr, nullptr, kErrorAbort);
  }
  }
 }
@@ -421,19 +418,16 @@ void CoapBase::HandleRetransmissionTimer(void)
  TimeMilli now = TimerMilli::GetNow();
  TimeMilli nextTime = now.GetDistantFuture();
  Metadata metadata;
- Message * nextMessage;
  Ip6::MessageInfo messageInfo;
 
- for (Message *message = mPendingRequests.GetHead(); message != nullptr; message = nextMessage)
+ for (Message &message : mPendingRequests)
  {
- nextMessage = message->GetNextCoapMessage();
-
- metadata.ReadFrom(*message);
+ metadata.ReadFrom(message);
 
  if (now >= metadata.mNextTimerShot)
  {
 #if OPENTHREAD_CONFIG_COAP_OBSERVE_API_ENABLE
- if (message->IsRequest() && metadata.mObserve && metadata.mAcknowledged)
+ if (message.IsRequest() && metadata.mObserve && metadata.mAcknowledged)
  {
  // This is a RFC7641 subscription. Do not time out.
  continue;
@@ -443,15 +437,15 @@ void CoapBase::HandleRetransmissionTimer(void)
  if (!metadata.mConfirmable || (metadata.mRetransmissionsRemaining == 0))
  {
  // No expected response or acknowledgment.
- FinalizeCoapTransaction(*message, metadata, nullptr, nullptr, kErrorResponseTimeout);
+ FinalizeCoapTransaction(message, metadata, nullptr, nullptr, kErrorResponseTimeout);
  continue;
  }
 
  // Increment retransmission counter and timer.
  metadata.mRetransmissionsRemaining--;
  metadata.mRetransmissionTimeout *= 2;
  metadata.mNextTimerShot = now + metadata.mRetransmissionTimeout;
- metadata.UpdateIn(*message);
+ metadata.UpdateIn(message);
 
  // Retransmit
  if (!metadata.mAcknowledged)
@@ -465,7 +459,7 @@ void CoapBase::HandleRetransmissionTimer(void)
 #endif
  messageInfo.SetMulticastLoop(metadata.mMulticastLoop);
 
- SendCopy(*message, messageInfo);
+ SendCopy(message, messageInfo);
  }
  }
 
@@ -498,17 +492,15 @@ void CoapBase::FinalizeCoapTransaction(Message & aRequest,
 Error CoapBase::AbortTransaction(ResponseHandler aHandler, void *aContext)
 {
  Error error = kErrorNotFound;
- Message *nextMessage;
  Metadata metadata;
 
- for (Message *message = mPendingRequests.GetHead(); message != nullptr; message = nextMessage)
+ for (Message &message : mPendingRequests)
  {
- nextMessage = message->GetNextCoapMessage();
- metadata.ReadFrom(*message);
+ metadata.ReadFrom(message);
 
  if (metadata.mResponseHandler == aHandler && metadata.mResponseContext == aContext)
  {
- FinalizeCoapTransaction(*message, metadata, nullptr, nullptr, kErrorAbort);
+ FinalizeCoapTransaction(message, metadata, nullptr, nullptr, kErrorAbort);
  error = kErrorNone;
  }
  }
@@ -965,11 +957,11 @@ Message *CoapBase::FindRelatedRequest(const Message & aResponse,
  const Ip6::MessageInfo &aMessageInfo,
  Metadata & aMetadata)
 {
- Message *message;
+ Message *request = nullptr;
 
- for (message = mPendingRequests.GetHead(); message != nullptr; message = message->GetNextCoapMessage())
+ for (Message &message : mPendingRequests)
  {
- aMetadata.ReadFrom(*message);
+ aMetadata.ReadFrom(message);
 
  if (((aMetadata.mDestinationAddress == aMessageInfo.GetPeerAddr()) ||
  aMetadata.mDestinationAddress.IsMulticast() ||
@@ -980,17 +972,19 @@ Message *CoapBase::FindRelatedRequest(const Message & aResponse,
  {
  case kTypeReset:
  case kTypeAck:
- if (aResponse.GetMessageId() == message->GetMessageId())
+ if (aResponse.GetMessageId() == message.GetMessageId())
  {
+ request = &message;
  ExitNow();
  }
 
  break;
 
  case kTypeConfirmable:
  case kTypeNonConfirmable:
- if (aResponse.IsTokenEqual(*message))
+ if (aResponse.IsTokenEqual(message))
  {
+ request = &message;
  ExitNow();
  }
 
@@ -1000,7 +994,7 @@ Message *CoapBase::FindRelatedRequest(const Message & aResponse,
  }
 
 exit:
- return message;
+ return request;
 }
 
 void CoapBase::Receive(ot::Message &aMessage, const Ip6::MessageInfo &aMessageInfo)
@@ -1449,25 +1443,26 @@ Error ResponsesQueue::GetMatchedResponseCopy(const Message & aRequest,
 
 const Message *ResponsesQueue::FindMatchedResponse(const Message &aRequest, const Ip6::MessageInfo &aMessageInfo) const
 {
- Message *message;
+ const Message *response = nullptr;
 
- for (message = mQueue.GetHead(); message != nullptr; message = message->GetNextCoapMessage())
+ for (const Message &message : mQueue)
  {
- if (message->GetMessageId() == aRequest.GetMessageId())
+ if (message.GetMessageId() == aRequest.GetMessageId())
  {
  ResponseMetadata metadata;
 
- metadata.ReadFrom(*message);
+ metadata.ReadFrom(message);
 
  if ((metadata.mMessageInfo.GetPeerPort() == aMessageInfo.GetPeerPort()) &&
  (metadata.mMessageInfo.GetPeerAddr() == aMessageInfo.GetPeerAddr()))
  {
+ response = &message;
  break;
  }
  }
  }
 
- return message;
+ return response;
 }
 
 void ResponsesQueue::EnqueueResponse(Message & aMessage,
@@ -1506,15 +1501,15 @@ void ResponsesQueue::UpdateQueue(void)
  // `kMaxCachedResponses` remove the one with earliest dequeue
  // time.
 
- for (Message *message = mQueue.GetHead(); message != nullptr; message = message->GetNextCoapMessage())
+ for (Message &message : mQueue)
  {
  ResponseMetadata metadata;
 
- metadata.ReadFrom(*message);
+ metadata.ReadFrom(message);
 
  if ((earliestMsg == nullptr) || (metadata.mDequeueTime < earliestDequeueTime))
  {
- earliestMsg = message;
+ earliestMsg = &message;
  earliestDequeueTime = metadata.mDequeueTime;
  }
 
@@ -1546,19 +1541,16 @@ void ResponsesQueue::HandleTimer(void)
 {
  TimeMilli now = TimerMilli::GetNow();
  TimeMilli nextDequeueTime = now.GetDistantFuture();
- Message * nextMessage;
 
- for (Message *message = mQueue.GetHead(); message != nullptr; message = nextMessage)
+ for (Message &message : mQueue)
  {
  ResponseMetadata metadata;
 
- nextMessage = message->GetNextCoapMessage();
-
- metadata.ReadFrom(*message);
+ metadata.ReadFrom(message);
 
  if (now >= metadata.mDequeueTime)
  {
- DequeueResponse(*message);
+ DequeueResponse(message);
  continue;
  }
 

src/core/coap/coap_message.cpp

@@ -476,6 +476,16 @@ const char *Message::CodeToString(void) const
 }
 #endif // OPENTHREAD_CONFIG_COAP_API_ENABLE
 
+Message::Iterator MessageQueue::begin(void)
+{
+ return Message::Iterator(GetHead());
+}
+
+Message::ConstIterator MessageQueue::begin(void) const
+{
+ return Message::ConstIterator(GetHead());
+}
+
 Error Option::Iterator::Init(const Message &aMessage)
 {
  Error error = kErrorParse;

src/core/coap/coap_message.hpp

@@ -166,6 +166,7 @@ enum OptionNumber : uint16_t
 class Message : public ot::Message
 {
  friend class Option;
+ friend class MessageQueue;
 
 public:
  static constexpr uint8_t kDefaultTokenLength = OT_COAP_DEFAULT_TOKEN_LENGTH; ///< Default token length.
@@ -956,6 +957,27 @@ class Message : public ot::Message
 #endif
  };
 
+ class ConstIterator : public ot::Message::ConstIterator
+ {
+ public:
+ using ot::Message::ConstIterator::ConstIterator;
+
+ const Message &operator*(void) { return static_cast<const Message &>(ot::Message::ConstIterator::operator*()); }
+ const Message *operator->(void)
+ {
+ return static_cast<const Message *>(ot::Message::ConstIterator::operator->());
+ }
+ };
+
+ class Iterator : public ot::Message::Iterator
+ {
+ public:
+ using ot::Message::Iterator::Iterator;
+
+ Message &operator*(void) { return static_cast<Message &>(ot::Message::Iterator::operator*()); }
+ Message *operator->(void) { return static_cast<Message *>(ot::Message::Iterator::operator->()); }
+ };
+
  static_assert(sizeof(HelpData) <= sizeof(Ip6::Header) + sizeof(Ip6::HopByHopHeader) + sizeof(Ip6::OptionMpl) +
  sizeof(Ip6::Udp::Header),
  "HelpData size exceeds the size of the reserved region in the message");
@@ -1000,7 +1022,15 @@ class MessageQueue : public ot::MessageQueue
  * @returns A pointer to the first message.
  *
  */
- Message *GetHead(void) const { return static_cast<Message *>(ot::MessageQueue::GetHead()); }
+ Message *GetHead(void) { return static_cast<Message *>(ot::MessageQueue::GetHead()); }
+
+ /**
+ * This method returns a pointer to the first message.
+ *
+ * @returns A pointer to the first message.
+ *
+ */
+ const Message *GetHead(void) const { return static_cast<const Message *>(ot::MessageQueue::GetHead()); }
 
  /**
  * This method adds a message to the end of the queue.
@@ -1034,6 +1064,17 @@ class MessageQueue : public ot::MessageQueue
  *
  */
  void DequeueAndFree(Message &aMessage) { ot::MessageQueue::DequeueAndFree(aMessage); }
+
+ // The following methods are intended to support range-based `for`
+ // loop iteration over the queue entries and should not be used
+ // directly. The range-based `for` works correctly even if the
+ // current entry is removed from the queue during iteration.
+
+ Message::Iterator begin(void);
+ Message::Iterator end(void) { return Message::Iterator(); }
+
+ Message::ConstIterator begin(void) const;
+ Message::ConstIterator end(void) const { return Message::ConstIterator(); }
 };
 
 /**