[message] introduce ReadAndAdvance() to simplify sequential parsing (#13106)

This commit introduces `Message::ReadAndAdvance()` and its template
flavor to the `Message` class. This helper method reads data from a
`Message` at a given `OffsetRange` and advances the `OffsetRange` by
the number of bytes read upon success.

Sequential parsing of structured data (such as TLVs or protocol
headers) is a common pattern across the OpenThread codebase.
Previously, this required two separate calls: one to `Read()` and
another to `AdvanceOffset()`. The new `ReadAndAdvance()` method
consolidates these into a single, safer operation that ensures the
offset is only advanced if the read operation succeeds.

This commit updates numerous call sites across the core stack
(MLE, BBR, DatasetManager, NetworkDiagnostic, DHCPv6, etc.) to use
the new helper, improving code clarity and reducing boilerplate.
This commit is contained in:
Abtin Keshavarzian
2026-05-14 22:57:08 -07:00
committed by GitHub
parent 27737f616e
commit a84fc2e50b
17 changed files with 72 additions and 58 deletions
+2 -4
View File
@@ -166,8 +166,7 @@ void Manager::HandleMulticastListenerRegistration(const Coap::Msg &aMsg)
{
while (!offsetRange.IsEmpty())
{
IgnoreError(aMsg.mMessage.Read(offsetRange, address));
offsetRange.AdvanceOffset(sizeof(Ip6::Address));
IgnoreError(aMsg.mMessage.ReadAndAdvance(offsetRange, address));
addresses[failedAddressNum++] = address;
}
}
@@ -217,8 +216,7 @@ void Manager::HandleMulticastListenerRegistration(const Coap::Msg &aMsg)
while (!offsetRange.IsEmpty())
{
IgnoreError(aMsg.mMessage.Read(offsetRange, address));
offsetRange.AdvanceOffset(sizeof(Ip6::Address));
IgnoreError(aMsg.mMessage.ReadAndAdvance(offsetRange, address));
if (timeout == 0)
{
+1 -2
View File
@@ -475,8 +475,7 @@ Error Dhcp6PdClient::ParseHeaderAndValidateMessage(Message &aMessage, Header &aH
offsetRange.InitFromMessageOffsetToEnd(aMessage);
SuccessOrExit(error = aMessage.Read(offsetRange, aHeader));
offsetRange.AdvanceOffset(sizeof(Header));
SuccessOrExit(error = aMessage.ReadAndAdvance(offsetRange, aHeader));
aMessage.SetOffset(offsetRange.GetOffset());
+12
View File
@@ -663,6 +663,18 @@ exit:
return error;
}
Error Message::ReadAndAdvance(OffsetRange &aOffsetRange, void *aBuf, uint16_t aLength) const
{
Error error = Read(aOffsetRange, aBuf, aLength);
if (error == kErrorNone)
{
aOffsetRange.AdvanceOffset(aLength);
}
return error;
}
Error Message::ReadAtAndAdvanceOffset(void *aBuf, uint16_t aLength)
{
Error error;
+31
View File
@@ -943,6 +943,37 @@ public:
return Read(aOffsetRange, &aObject, sizeof(ObjectType));
}
/**
* Reads a given number of bytes from the message at a given offset range and advances the offset range.
*
* @param[in,out] aOffsetRange The offset range in the message to read from. On success, it is advanced.
* @param[out] aBuf A pointer to a data buffer to copy the read bytes into.
* @param[in] aLength Number of bytes to read.
*
* @retval kErrorNone Requested bytes were successfully read from message. @p aOffsetRange is advanced.
* @retval kErrorParse Not enough bytes remaining to read the requested @p aLength. @p aOffsetRange is unchanged.
*/
Error ReadAndAdvance(OffsetRange &aOffsetRange, void *aBuf, uint16_t aLength) const;
/**
* Reads an object from the message at a given offset range and advances the offset range.
*
* @tparam ObjectType The object type to read from the message.
*
* @param[in,out] aOffsetRange The offset range in the message to read from. On success, it is advanced.
* @param[out] aObject A reference to the object to read into.
*
* @retval kErrorNone Object @p aObject was successfully read from message. @p aOffsetRange is advanced.
* @retval kErrorParse Not enough bytes remaining in message to read the entire object. @p aOffsetRange is
* unchanged.
*/
template <typename ObjectType> Error ReadAndAdvance(OffsetRange &aOffsetRange, ObjectType &aObject) const
{
static_assert(!TypeTraits::IsPointer<ObjectType>::kValue, "ObjectType must not be a pointer");
return ReadAndAdvance(aOffsetRange, &aObject, sizeof(ObjectType));
}
/**
* Reads a given number of bytes from the message at the current message offset and advances the message offset.
*
+1 -2
View File
@@ -1016,8 +1016,7 @@ void Manager::CoapDtlsSession::HandleTmfProxyTx(Coap::Msg &aMsg)
SuccessOrExit(error = Tlv::FindTlvValueOffsetRange(aMsg.mMessage, Tlv::kUdpEncapsulation, offsetRange));
SuccessOrExit(error = aMsg.mMessage.Read(offsetRange, udpEncapHeader));
offsetRange.AdvanceOffset(sizeof(UdpEncapsulationTlvHeader));
SuccessOrExit(error = aMsg.mMessage.ReadAndAdvance(offsetRange, udpEncapHeader));
VerifyOrExit(udpEncapHeader.GetSourcePort() > 0 && udpEncapHeader.GetDestinationPort() > 0, error = kErrorDrop);
+1 -2
View File
@@ -543,9 +543,8 @@ Coap::Message *DatasetManager::ProcessGetRequest(const Coap::Message &aReques
{
uint8_t tlvType;
IgnoreError(aRequest.Read(offsetRange, tlvType));
IgnoreError(aRequest.ReadAndAdvance(offsetRange, tlvType));
tlvList.Add(tlvType);
offsetRange.AdvanceOffset(sizeof(uint8_t));
}
// MGMT_PENDING_GET.rsp must include Delay Timer TLV (Thread 1.1.1
+1 -3
View File
@@ -407,9 +407,7 @@ Error Client::ProcessIaNaOption(const Message &aMessage)
Option::Iterator iterator;
SuccessOrExit(error = Option::FindOption(aMessage, Option::kIaNa, offsetRange));
SuccessOrExit(error = aMessage.Read(offsetRange, option));
offsetRange.AdvanceOffset(sizeof(IaNaOption));
SuccessOrExit(error = aMessage.ReadAndAdvance(offsetRange, option));
// Iterate over and check the sub-options within `IaNaOption`.
+1 -3
View File
@@ -243,12 +243,10 @@ Error Server::ProcessIaNaOption(const Message &aMessage, uint32_t &aIaid)
Option::Iterator iterator;
SuccessOrExit(error = Option::FindOption(aMessage, Option::kIaNa, offsetRange));
SuccessOrExit(error = aMessage.Read(offsetRange, iaNaOption));
SuccessOrExit(error = aMessage.ReadAndAdvance(offsetRange, iaNaOption));
aIaid = iaNaOption.GetIaid();
offsetRange.AdvanceOffset(sizeof(IaNaOption));
mPrefixAgentsMask = 0;
// Iterate and parse `kIaAddress` sub-options within `IaNaOption`.
+1 -2
View File
@@ -234,8 +234,7 @@ bool Header::HasSourceRouteOption(const Message &aMessage) const
uint8_t optionType;
uint8_t optionLen;
SuccessOrExit(aMessage.Read(range, optionType));
range.AdvanceOffset(sizeof(uint8_t));
SuccessOrExit(aMessage.ReadAndAdvance(range, optionType));
if (optionType == kOptionEnd)
{
+2 -5
View File
@@ -291,8 +291,7 @@ Error Ip6::RemoveMplOption(Message &aMessage)
offsetRange.InitFromMessageFullLength(aMessage);
IgnoreError(aMessage.Read(offsetRange, ip6Header));
offsetRange.AdvanceOffset(sizeof(ip6Header));
IgnoreError(aMessage.ReadAndAdvance(offsetRange, ip6Header));
VerifyOrExit(ip6Header.GetNextHeader() == kProtoHopOpts);
@@ -885,9 +884,7 @@ bool Ip6::HasIp6InIpTunnel(const Message &aMessage, uint8_t aNextHeader) const
{
FragmentHeader fragHeader;
SuccessOrExit(aMessage.Read(offsetRange, fragHeader));
VerifyOrExit(offsetRange.Contains(sizeof(FragmentHeader)));
offsetRange.AdvanceOffset(sizeof(FragmentHeader));
SuccessOrExit(aMessage.ReadAndAdvance(offsetRange, fragHeader));
aNextHeader = fragHeader.GetNextHeader();
}
else
+3 -6
View File
@@ -1892,8 +1892,7 @@ Error Mle::ProcessAddressRegistrationTlv(RxInfo &aRxInfo, Child &aChild)
Ip6::Address address;
// Read out the control byte (first byte in entry)
SuccessOrExit(error = aRxInfo.mMessage.Read(offsetRange, controlByte));
offsetRange.AdvanceOffset(sizeof(uint8_t));
SuccessOrExit(error = aRxInfo.mMessage.ReadAndAdvance(offsetRange, controlByte));
count++;
address.Clear();
@@ -1907,8 +1906,7 @@ Error Mle::ProcessAddressRegistrationTlv(RxInfo &aRxInfo, Child &aChild)
uint8_t contextId = AddressRegistrationTlv::GetContextId(controlByte);
Lowpan::Context context;
IgnoreError(aRxInfo.mMessage.Read(offsetRange, address.GetIid()));
offsetRange.AdvanceOffset(sizeof(Ip6::InterfaceIdentifier));
IgnoreError(aRxInfo.mMessage.ReadAndAdvance(offsetRange, address.GetIid()));
Get<NetworkData::Leader>().FindContextForId(contextId, context);
@@ -1925,8 +1923,7 @@ Error Mle::ProcessAddressRegistrationTlv(RxInfo &aRxInfo, Child &aChild)
{
// Uncompressed entry contains the full IPv6 address.
IgnoreError(aRxInfo.mMessage.Read(offsetRange, address));
offsetRange.AdvanceOffset(sizeof(Ip6::Address));
IgnoreError(aRxInfo.mMessage.ReadAndAdvance(offsetRange, address));
}
#if OPENTHREAD_CONFIG_REFERENCE_DEVICE_ENABLE
+2 -4
View File
@@ -53,8 +53,7 @@ Error RouteTlv::Data::ParseFrom(const Message &aMessage, const OffsetRange &aOff
bool isEven = true;
#endif
SuccessOrExit(error = aMessage.Read(offsetRange, routerIdMask));
offsetRange.AdvanceOffset(sizeof(routerIdMask));
SuccessOrExit(error = aMessage.ReadAndAdvance(offsetRange, routerIdMask));
mIdSequence = routerIdMask.GetSequence();
@@ -79,8 +78,7 @@ Error RouteTlv::Data::ParseFrom(const Message &aMessage, const OffsetRange &aOff
entry->mRouterId = routerId;
#if !OPENTHREAD_CONFIG_MLE_LONG_ROUTES_ENABLE
SuccessOrExit(error = aMessage.Read<uint8_t>(offsetRange, entry->mRouteData));
offsetRange.AdvanceOffset(sizeof(uint8_t));
SuccessOrExit(error = aMessage.ReadAndAdvance<uint8_t>(offsetRange, entry->mRouteData));
#else
{
EntryType value;
+1 -2
View File
@@ -586,8 +586,7 @@ Coap::Message *Leader::ProcessCommissionerGetRequest(const Coap::Message &aMessa
uint8_t type;
const MeshCoP::Tlv *subTlv;
IgnoreError(aMessage.Read(offsetRange, type));
offsetRange.AdvanceOffset(sizeof(type));
IgnoreError(aMessage.ReadAndAdvance(offsetRange, type));
subTlv = FindCommissioningDataSubTlv(type);
+4 -8
View File
@@ -863,8 +863,7 @@ Error Server::TlvTypeListIterator::ReadNextTlvType(uint8_t &aTlvType)
while (!mOffsetRange.IsEmpty())
{
SuccessOrExit(error = mMessage->Read(mOffsetRange, aTlvType));
mOffsetRange.AdvanceOffset(sizeof(uint8_t));
SuccessOrExit(error = mMessage->ReadAndAdvance(mOffsetRange, aTlvType));
if (!mProcessedTlvs.Has(aTlvType))
{
@@ -1058,8 +1057,7 @@ static Error ParseEnhancedRoute(const Message &aMessage, uint16_t aOffset, otNet
continue;
}
SuccessOrExit(error = aMessage.Read(offsetRange, entry));
offsetRange.AdvanceOffset(sizeof(entry));
SuccessOrExit(error = aMessage.ReadAndAdvance(offsetRange, entry));
aNetworkDiagEnhRoute.mRouteData[index].mRouterId = routerId;
entry.Parse(aNetworkDiagEnhRoute.mRouteData[index]);
@@ -1095,8 +1093,7 @@ Error Client::ParseChildTable(ChildTable &aChildTable, const Message &aMessage,
{
ChildTableTlvEntry entry;
SuccessOrExit(error = aMessage.Read(aOffsetRange, entry));
aOffsetRange.AdvanceOffset(sizeof(ChildTableTlvEntry));
SuccessOrExit(error = aMessage.ReadAndAdvance(aOffsetRange, entry));
entry.Parse(aChildTable.mTable[aChildTable.mCount]);
aChildTable.mCount++;
@@ -1112,8 +1109,7 @@ void Client::ParseIp6AddrList(Ip6AddrList &aIp6Addrs, const Message &aMessage, O
while (aOffsetRange.Contains(sizeof(Ip6::Address)) && (aIp6Addrs.mCount < GetArrayLength(aIp6Addrs.mList)))
{
IgnoreError(aMessage.Read(aOffsetRange, aIp6Addrs.mList[aIp6Addrs.mCount]));
aOffsetRange.AdvanceOffset(sizeof(Ip6::Address));
IgnoreError(aMessage.ReadAndAdvance(aOffsetRange, aIp6Addrs.mList[aIp6Addrs.mCount]));
aIp6Addrs.mCount++;
}
+1 -3
View File
@@ -63,9 +63,7 @@ Error Ip6AddressesTlv::FindIn(const Message &aMessage, Mlr::AddressArray &aAddre
{
Ip6::Address address;
SuccessOrExit(error = aMessage.Read(offsetRange, address));
offsetRange.AdvanceOffset(sizeof(Ip6::Address));
SuccessOrExit(error = aMessage.ReadAndAdvance(offsetRange, address));
SuccessOrExit(error = aAddresses.AddUnique(address));
}
+6 -8
View File
@@ -390,8 +390,7 @@ bool MeshDiag::ProcessChildrenIp6AddrsAnswer(Coap::Message &aMessage, const Ip6:
// Read the `ChildIp6AddressListTlvValue` (which contains the
// child RLOC16) and then prepare the `Ip6AddrIterator`.
SuccessOrExit(aMessage.Read(offsetRange, tlvValue));
offsetRange.AdvanceOffset(sizeof(tlvValue));
SuccessOrExit(aMessage.ReadAndAdvance(offsetRange, tlvValue));
ip6AddrIterator.mMessage = &aMessage;
ip6AddrIterator.mOffsetRange = offsetRange;
@@ -520,12 +519,12 @@ exit:
Error MeshDiag::Ip6AddrIterator::GetNextAddress(Ip6::Address &aAddress)
{
Error error = kErrorNone;
Error error = kErrorNotFound;
VerifyOrExit(mMessage != nullptr, error = kErrorNotFound);
VerifyOrExit(mMessage != nullptr);
SuccessOrExit(mMessage->ReadAndAdvance(mOffsetRange, aAddress));
VerifyOrExit(mMessage->Read(mOffsetRange, aAddress) == kErrorNone, error = kErrorNotFound);
mOffsetRange.AdvanceOffset(sizeof(Ip6::Address));
error = kErrorNone;
exit:
return error;
@@ -555,8 +554,7 @@ Error MeshDiag::ChildIterator::GetNextChildInfo(ChildInfo &aChildInfo)
VerifyOrExit(mMessage != nullptr);
SuccessOrExit(mMessage->Read(mOffsetRange, entry));
mOffsetRange.AdvanceOffset(sizeof(ChildTableTlvEntry));
SuccessOrExit(mMessage->ReadAndAdvance(mOffsetRange, entry));
entry.Parse(info);
+2 -4
View File
@@ -342,10 +342,9 @@ void Dhcp6Msg::ParseFrom(const Message &aMessage)
Clear();
offsetRange.InitFromMessageFullLength(aMessage);
SuccessOrQuit(aMessage.Read(offsetRange, header));
SuccessOrQuit(aMessage.ReadAndAdvance(offsetRange, header));
mMsgType = header.GetMsgType();
mTransactionId = header.GetTransactionId();
offsetRange.AdvanceOffset(sizeof(header));
while (!offsetRange.IsEmpty())
{
@@ -432,8 +431,7 @@ void Dhcp6Msg::ParseFrom(const Message &aMessage)
break;
case Dhcp6::Option::kIaPd:
SuccessOrQuit(aMessage.Read(optionOffsetRange, iaPdOption));
optionOffsetRange.AdvanceOffset(sizeof(iaPdOption));
SuccessOrQuit(aMessage.ReadAndAdvance(optionOffsetRange, iaPdOption));
VerifyOrQuit(!mIaPds.ContainsMatching(iaPdOption.GetIaid()));
iaPd = mIaPds.PushBack();