espressif/openthread
1
0
Fork 0
mirror of https://github.com/espressif/openthread.git synced 2026-07-29 23:27:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

[message] disallow SetPriority() on enqueued messages in PriorityQueue (#11624)

Browse Source 
This commit modifies `Message::SetPriority()` to prevent changing a
message's priority after it has been enqueued in a `PriorityQueue`.
Attempting to do so will now return `kErrorInvalidState`.

The functionality for altering the priority of an already-enqueued
message is not currently used or required. Should this behavior
become necessary in the future, the recommended approach is to
explicitly dequeue the message first, then change its priority, and
finally re-add it to the queue. This makes the intended behavior
clearer and more explicit within the code.

This commit also updates the `test_priority_queue` unit test to
reflect this change.
This commit is contained in:
Abtin Keshavarzian
2025-06-23 13:15:53 -07:00
committed by GitHub
parent 7617b602b6
commit c6f3f1ff31
3 changed files with 17 additions and 95 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/core/common/message.cpp
+4 -18
View File
@@ -328,31 +328,17 @@ bool Message::IsMleCommand(Mle::Command aMleCommand) const
Error Message::SetPriority(Priority aPriority)
{
Error error = kErrorNone;
uint8_t priority = static_cast<uint8_t>(aPriority);
PriorityQueue *priorityQueue;
Error error = kErrorNone;
uint8_t priority = static_cast<uint8_t>(aPriority);
static_assert(kNumPriorities <= 4, "`Metadata::mPriority` as a 2-bit field cannot fit all `Priority` values");
VerifyOrExit(priority < kNumPriorities, error = kErrorInvalidArgs);
VerifyOrExit(IsInAQueue(), GetMetadata().mPriority = priority);
VerifyOrExit(GetMetadata().mPriority != priority);
priorityQueue = GetPriorityQueue();
if (priorityQueue != nullptr)
{
priorityQueue->Dequeue(*this);
}
VerifyOrExit(!IsInAPriorityQueue(), error = kErrorInvalidState);
GetMetadata().mPriority = priority;
if (priorityQueue != nullptr)
{
priorityQueue->Enqueue(*this);
}
exit:
return error;
}
@@ -869,7 +855,7 @@ void Message::SetMessageQueue(MessageQueue *aMessageQueue)
void Message::SetPriorityQueue(PriorityQueue *aPriorityQueue)
{
GetMetadata().mQueue = aPriorityQueue;
GetMetadata().mInPriorityQ = true;
GetMetadata().mInPriorityQ = aPriorityQueue != nullptr;
}
//---------------------------------------------------------------------------------------------------------------------
src/core/common/message.hpp
+6 -2
View File
@@ -633,13 +633,15 @@ public:
/**
* Sets the messages priority.
* If the message is already queued in a priority queue, changing the priority ensures to
* update the message in the associated queue.
*
* If the message is already enqueued in a priority queue, the priority cannot be changed and `kErrorInvalidState`
* is returned.
*
* @param[in] aPriority The message priority level.
*
* @retval kErrorNone Successfully set the priority for the message.
* @retval kErrorInvalidArgs Priority level is not invalid.
* @retval kErrorInvalidState Message is already queued in a priority queue.
*/
Error SetPriority(Priority aPriority);
@@ -1569,6 +1571,8 @@ private:
}
bool IsInAQueue(void) const { return (GetMetadata().mQueue != nullptr); }
bool IsInAPriorityQueue(void) const { return GetMetadata().mInPriorityQ; }
void SetMessageQueue(MessageQueue *aMessageQueue);
void SetPriorityQueue(PriorityQueue *aPriorityQueue);
tests/unit/test_priority_queue.cpp
+7 -75
View File
@@ -135,44 +135,11 @@ void VerifyPriorityQueueContent(PriorityQueue &aPriorityQueue, int aExpectedLeng
VerifyOrQuit(message == nullptr, "`for` loop iteration resulted in fewer entries than expected");
}
// This function verifies the content of the message queue to match the passed in messages
void VerifyMsgQueueContent(MessageQueue &aMessageQueue, int aExpectedLength, ...)
{
va_list args;
Message *message;
Message *msgArg;
va_start(args, aExpectedLength);
if (aExpectedLength == 0)
{
message = aMessageQueue.GetHead();
VerifyOrQuit(message == nullptr, "is not empty when expected len is zero.");
}
else
{
for (message = aMessageQueue.GetHead(); message != nullptr; message = message->GetNext())
{
VerifyOrQuit(aExpectedLength != 0, "contains more entries than expected");
msgArg = va_arg(args, Message *);
VerifyOrQuit(msgArg == message, "content does not match what is expected.");
aExpectedLength--;
}
VerifyOrQuit(aExpectedLength == 0, "contains less entries than expected");
}
va_end(args);
}
void TestPriorityQueue(void)
{
Instance *instance;
MessagePool *messagePool;
PriorityQueue queue;
MessageQueue messageQueue;
Message *msgNet[kNumTestMessages];
Message *msgHigh[kNumTestMessages];
Message *msgNor[kNumTestMessages];
@@ -264,7 +231,7 @@ void TestPriorityQueue(void)
queue.Dequeue(*msgHigh[3]);
VerifyPriorityQueueContent(queue, 0);
// Change the priority of an already queued message and check the order change in the queue.
// Validate that priority of an already queued message in a priority queue cannot change
queue.Enqueue(*msgNor[0]);
VerifyPriorityQueueContent(queue, 1, msgNor[0]);
queue.Enqueue(*msgHigh[0]);
@@ -272,54 +239,19 @@ void TestPriorityQueue(void)
queue.Enqueue(*msgLow[0]);
VerifyPriorityQueueContent(queue, 3, msgHigh[0], msgNor[0], msgLow[0]);
SuccessOrQuit(msgNor[0]->SetPriority(Message::kPriorityNet));
VerifyPriorityQueueContent(queue, 3, msgNor[0], msgHigh[0], msgLow[0]);
SuccessOrQuit(msgLow[0]->SetPriority(Message::kPriorityLow));
VerifyPriorityQueueContent(queue, 3, msgNor[0], msgHigh[0], msgLow[0]);
SuccessOrQuit(msgLow[0]->SetPriority(Message::kPriorityNormal));
VerifyPriorityQueueContent(queue, 3, msgNor[0], msgHigh[0], msgLow[0]);
SuccessOrQuit(msgLow[0]->SetPriority(Message::kPriorityHigh));
VerifyPriorityQueueContent(queue, 3, msgNor[0], msgHigh[0], msgLow[0]);
SuccessOrQuit(msgLow[0]->SetPriority(Message::kPriorityNet));
VerifyPriorityQueueContent(queue, 3, msgNor[0], msgLow[0], msgHigh[0]);
SuccessOrQuit(msgNor[0]->SetPriority(Message::kPriorityNormal));
SuccessOrQuit(msgLow[0]->SetPriority(Message::kPriorityLow));
VerifyOrQuit(msgNor[0]->SetPriority(Message::kPriorityNet) == kErrorInvalidState);
VerifyOrQuit(msgLow[0]->SetPriority(Message::kPriorityLow) == kErrorInvalidState);
VerifyOrQuit(msgLow[0]->SetPriority(Message::kPriorityNormal) == kErrorInvalidState);
VerifyOrQuit(msgHigh[0]->SetPriority(Message::kPriorityNormal) == kErrorInvalidState);
VerifyPriorityQueueContent(queue, 3, msgHigh[0], msgNor[0], msgLow[0]);
messageQueue.Enqueue(*msgNor[1]);
messageQueue.Enqueue(*msgHigh[1]);
messageQueue.Enqueue(*msgNet[1]);
VerifyMsgQueueContent(messageQueue, 3, msgNor[1], msgHigh[1], msgNet[1]);
// Change priority of message and check for not in messageQueue.
SuccessOrQuit(msgNor[1]->SetPriority(Message::kPriorityNet));
VerifyMsgQueueContent(messageQueue, 3, msgNor[1], msgHigh[1], msgNet[1]);
SuccessOrQuit(msgLow[0]->SetPriority(Message::kPriorityHigh));
VerifyPriorityQueueContent(queue, 3, msgHigh[0], msgLow[0], msgNor[0]);
VerifyMsgQueueContent(messageQueue, 3, msgNor[1], msgHigh[1], msgNet[1]);
// Remove messages from the two queues
// Remove messages from the queue
queue.Dequeue(*msgHigh[0]);
VerifyPriorityQueueContent(queue, 2, msgLow[0], msgNor[0]);
VerifyMsgQueueContent(messageQueue, 3, msgNor[1], msgHigh[1], msgNet[1]);
messageQueue.Dequeue(*msgNet[1]);
VerifyPriorityQueueContent(queue, 2, msgLow[0], msgNor[0]);
VerifyMsgQueueContent(messageQueue, 2, msgNor[1], msgHigh[1]);
messageQueue.Dequeue(*msgHigh[1]);
VerifyPriorityQueueContent(queue, 2, msgLow[0], msgNor[0]);
VerifyMsgQueueContent(messageQueue, 1, msgNor[1]);
VerifyPriorityQueueContent(queue, 2, msgNor[0], msgLow[0]);
queue.Dequeue(*msgLow[0]);
VerifyPriorityQueueContent(queue, 1, msgNor[0]);
VerifyMsgQueueContent(messageQueue, 1, msgNor[1]);
queue.Dequeue(*msgNor[0]);
VerifyPriorityQueueContent(queue, 0);
messageQueue.Dequeue(*msgNor[1]);
VerifyMsgQueueContent(messageQueue, 0);
for (Message *message : msgNor)
{