[dataset] misc enhancements (#4888)

This commit contains a group of smaller enhancements/simplifications
in `Dataset` modules:
- Add helper `GetTlvsStart()` and `GetTlvsEnd()` to get start/end
  of Dataset TLVs sequence.
- Use `for` loop when iterating over TLVs.
- Simplify `RemoveTlv()` to get a pointer to TLV as input
- Remove single-use local TLV variables (in switch statements).
- Simplify `DatasetLocal::Compare()`.
This commit is contained in:
Abtin Keshavarzian
2020-04-25 14:53:35 -07:00
committed by GitHub
parent be48ea648c
commit 931cdedc84
4 changed files with 83 additions and 126 deletions
+35 -84
View File
@@ -67,10 +67,9 @@ void Dataset::Clear(void)
bool Dataset::IsValid(void) const
{
bool rval = true;
const Tlv *cur = reinterpret_cast<const Tlv *>(mTlvs);
const Tlv *end = reinterpret_cast<const Tlv *>(mTlvs + mLength);
const Tlv *end = GetTlvsEnd();
for (; cur < end; cur = cur->GetNext())
for (const Tlv *cur = GetTlvsStart(); cur < end; cur = cur->GetNext())
{
VerifyOrExit((cur + 1) <= end && cur->GetNext() <= end && Tlv::IsValid(*cur), rval = false);
}
@@ -86,37 +85,27 @@ const Tlv *Dataset::GetTlv(Tlv::Type aType) const
void Dataset::ConvertTo(otOperationalDataset &aDataset) const
{
const Tlv *cur = reinterpret_cast<const Tlv *>(mTlvs);
const Tlv *end = reinterpret_cast<const Tlv *>(mTlvs + mLength);
memset(&aDataset, 0, sizeof(aDataset));
while (cur < end)
for (const Tlv *cur = GetTlvsStart(); cur < GetTlvsEnd(); cur = cur->GetNext())
{
switch (cur->GetType())
{
case Tlv::kActiveTimestamp:
{
const ActiveTimestampTlv *tlv = static_cast<const ActiveTimestampTlv *>(cur);
aDataset.mActiveTimestamp = tlv->GetSeconds();
aDataset.mActiveTimestamp = static_cast<const ActiveTimestampTlv *>(cur)->GetSeconds();
aDataset.mComponents.mIsActiveTimestampPresent = true;
break;
}
case Tlv::kChannel:
{
const ChannelTlv *tlv = static_cast<const ChannelTlv *>(cur);
aDataset.mChannel = tlv->GetChannel();
aDataset.mChannel = static_cast<const ChannelTlv *>(cur)->GetChannel();
aDataset.mComponents.mIsChannelPresent = true;
break;
}
case Tlv::kChannelMask:
{
const ChannelMaskTlv *tlv = static_cast<const ChannelMaskTlv *>(cur);
uint32_t mask;
uint32_t mask;
if ((mask = tlv->GetChannelMask()) != 0)
if ((mask = static_cast<const ChannelMaskTlv *>(cur)->GetChannelMask()) != 0)
{
aDataset.mChannelMask = mask;
aDataset.mComponents.mIsChannelMaskPresent = true;
@@ -126,68 +115,45 @@ void Dataset::ConvertTo(otOperationalDataset &aDataset) const
}
case Tlv::kDelayTimer:
{
const DelayTimerTlv *tlv = static_cast<const DelayTimerTlv *>(cur);
aDataset.mDelay = tlv->GetDelayTimer();
aDataset.mDelay = static_cast<const DelayTimerTlv *>(cur)->GetDelayTimer();
aDataset.mComponents.mIsDelayPresent = true;
break;
}
case Tlv::kExtendedPanId:
{
const ExtendedPanIdTlv *tlv = static_cast<const ExtendedPanIdTlv *>(cur);
aDataset.mExtendedPanId = tlv->GetExtendedPanId();
aDataset.mExtendedPanId = static_cast<const ExtendedPanIdTlv *>(cur)->GetExtendedPanId();
aDataset.mComponents.mIsExtendedPanIdPresent = true;
break;
}
case Tlv::kMeshLocalPrefix:
{
const MeshLocalPrefixTlv *tlv = static_cast<const MeshLocalPrefixTlv *>(cur);
aDataset.mMeshLocalPrefix = tlv->GetMeshLocalPrefix();
aDataset.mMeshLocalPrefix = static_cast<const MeshLocalPrefixTlv *>(cur)->GetMeshLocalPrefix();
aDataset.mComponents.mIsMeshLocalPrefixPresent = true;
break;
}
case Tlv::kNetworkMasterKey:
{
const NetworkMasterKeyTlv *tlv = static_cast<const NetworkMasterKeyTlv *>(cur);
aDataset.mMasterKey = tlv->GetNetworkMasterKey();
aDataset.mMasterKey = static_cast<const NetworkMasterKeyTlv *>(cur)->GetNetworkMasterKey();
aDataset.mComponents.mIsMasterKeyPresent = true;
break;
}
case Tlv::kNetworkName:
{
const NetworkNameTlv *tlv = static_cast<const NetworkNameTlv *>(cur);
static_cast<Mac::NetworkName &>(aDataset.mNetworkName).Set(tlv->GetNetworkName());
static_cast<Mac::NetworkName &>(aDataset.mNetworkName)
.Set(static_cast<const NetworkNameTlv *>(cur)->GetNetworkName());
aDataset.mComponents.mIsNetworkNamePresent = true;
break;
}
case Tlv::kPanId:
{
const PanIdTlv *panid = static_cast<const PanIdTlv *>(cur);
aDataset.mPanId = panid->GetPanId();
aDataset.mPanId = static_cast<const PanIdTlv *>(cur)->GetPanId();
aDataset.mComponents.mIsPanIdPresent = true;
break;
}
case Tlv::kPendingTimestamp:
{
const PendingTimestampTlv *tlv = static_cast<const PendingTimestampTlv *>(cur);
aDataset.mPendingTimestamp = tlv->GetSeconds();
aDataset.mPendingTimestamp = static_cast<const PendingTimestampTlv *>(cur)->GetSeconds();
aDataset.mComponents.mIsPendingTimestampPresent = true;
break;
}
case Tlv::kPskc:
{
const PskcTlv *tlv = static_cast<const PskcTlv *>(cur);
aDataset.mPskc = tlv->GetPskc();
aDataset.mPskc = static_cast<const PskcTlv *>(cur)->GetPskc();
aDataset.mComponents.mIsPskcPresent = true;
break;
}
case Tlv::kSecurityPolicy:
{
@@ -201,8 +167,6 @@ void Dataset::ConvertTo(otOperationalDataset &aDataset) const
default:
break;
}
cur = cur->GetNext();
}
}
@@ -213,8 +177,8 @@ void Dataset::Set(const Dataset &aDataset)
if (mType == kActive)
{
Remove(Tlv::kPendingTimestamp);
Remove(Tlv::kDelayTimer);
RemoveTlv(Tlv::kPendingTimestamp);
RemoveTlv(Tlv::kDelayTimer);
}
mUpdateTime = aDataset.GetUpdateTime();
@@ -351,7 +315,7 @@ otError Dataset::SetTlv(Tlv::Type aType, const void *aValue, uint8_t aLength)
if (old != NULL)
{
Remove(reinterpret_cast<uint8_t *>(old), sizeof(Tlv) + old->GetLength());
RemoveTlv(old);
}
tlv.SetType(aType);
@@ -400,12 +364,12 @@ exit:
return error;
}
void Dataset::Remove(Tlv::Type aType)
void Dataset::RemoveTlv(Tlv::Type aType)
{
Tlv *tlv;
VerifyOrExit((tlv = GetTlv(aType)) != NULL, OT_NOOP);
Remove(reinterpret_cast<uint8_t *>(tlv), sizeof(Tlv) + tlv->GetLength());
RemoveTlv(tlv);
exit:
return;
@@ -416,8 +380,6 @@ otError Dataset::AppendMleDatasetTlv(Message &aMessage) const
otError error = OT_ERROR_NONE;
Mle::Tlv tlv;
Mle::Tlv::Type type;
const Tlv * cur = reinterpret_cast<const Tlv *>(mTlvs);
const Tlv * end = reinterpret_cast<const Tlv *>(mTlvs + mLength);
VerifyOrExit(mLength > 0, OT_NOOP);
@@ -427,7 +389,7 @@ otError Dataset::AppendMleDatasetTlv(Message &aMessage) const
tlv.SetLength(static_cast<uint8_t>(mLength) - sizeof(Tlv) - sizeof(Timestamp));
SuccessOrExit(error = aMessage.Append(&tlv, sizeof(Tlv)));
while (cur < end)
for (const Tlv *cur = GetTlvsStart(); cur < GetTlvsEnd(); cur = cur->GetNext())
{
if (((mType == kActive) && (cur->GetType() == Tlv::kActiveTimestamp)) ||
((mType == kPending) && (cur->GetType() == Tlv::kPendingTimestamp)))
@@ -454,18 +416,19 @@ otError Dataset::AppendMleDatasetTlv(Message &aMessage) const
{
SuccessOrExit(error = cur->AppendTo(aMessage));
}
cur = cur->GetNext();
}
exit:
return error;
}
void Dataset::Remove(uint8_t *aStart, uint8_t aLength)
void Dataset::RemoveTlv(Tlv *aTlv)
{
memmove(aStart, aStart + aLength, mLength - (static_cast<uint8_t>(aStart - mTlvs) + aLength));
mLength -= aLength;
uint8_t *start = reinterpret_cast<uint8_t *>(aTlv);
uint16_t length = sizeof(Tlv) + aTlv->GetLength();
memmove(start, start + length, mLength - (static_cast<uint8_t>(start - mTlvs) + length));
mLength -= length;
}
otError Dataset::ApplyConfiguration(Instance &aInstance, bool *aIsMasterKeyUpdated) const
@@ -473,8 +436,6 @@ otError Dataset::ApplyConfiguration(Instance &aInstance, bool *aIsMasterKeyUpdat
Mac::Mac & mac = aInstance.Get<Mac::Mac>();
KeyManager &keyManager = aInstance.Get<KeyManager>();
otError error = OT_ERROR_NONE;
const Tlv * cur = reinterpret_cast<const Tlv *>(mTlvs);
const Tlv * end = reinterpret_cast<const Tlv *>(mTlvs + mLength);
VerifyOrExit(IsValid(), error = OT_ERROR_PARSE);
@@ -483,7 +444,7 @@ otError Dataset::ApplyConfiguration(Instance &aInstance, bool *aIsMasterKeyUpdat
*aIsMasterKeyUpdated = false;
}
while (cur < end)
for (const Tlv *cur = GetTlvsStart(); cur < GetTlvsEnd(); cur = cur->GetNext())
{
switch (cur->GetType())
{
@@ -512,11 +473,8 @@ otError Dataset::ApplyConfiguration(Instance &aInstance, bool *aIsMasterKeyUpdat
break;
case Tlv::kNetworkName:
{
const NetworkNameTlv *name = static_cast<const NetworkNameTlv *>(cur);
mac.SetNetworkName(name->GetNetworkName());
mac.SetNetworkName(static_cast<const NetworkNameTlv *>(cur)->GetNetworkName());
break;
}
case Tlv::kNetworkMasterKey:
{
@@ -534,20 +492,15 @@ otError Dataset::ApplyConfiguration(Instance &aInstance, bool *aIsMasterKeyUpdat
#if OPENTHREAD_FTD
case Tlv::kPskc:
{
const PskcTlv *pskc = static_cast<const PskcTlv *>(cur);
keyManager.SetPskc(pskc->GetPskc());
keyManager.SetPskc(static_cast<const PskcTlv *>(cur)->GetPskc());
break;
}
#endif
case Tlv::kMeshLocalPrefix:
{
const MeshLocalPrefixTlv *prefix = static_cast<const MeshLocalPrefixTlv *>(cur);
aInstance.Get<Mle::MleRouter>().SetMeshLocalPrefix(prefix->GetMeshLocalPrefix());
aInstance.Get<Mle::MleRouter>().SetMeshLocalPrefix(
static_cast<const MeshLocalPrefixTlv *>(cur)->GetMeshLocalPrefix());
break;
}
case Tlv::kSecurityPolicy:
{
@@ -560,8 +513,6 @@ otError Dataset::ApplyConfiguration(Instance &aInstance, bool *aIsMasterKeyUpdat
default:
break;
}
cur = cur->GetNext();
}
exit:
@@ -570,8 +521,8 @@ exit:
void Dataset::ConvertToActive(void)
{
Remove(Tlv::kPendingTimestamp);
Remove(Tlv::kDelayTimer);
RemoveTlv(Tlv::kPendingTimestamp);
RemoveTlv(Tlv::kDelayTimer);
mType = kActive;
}
+38 -2
View File
@@ -290,7 +290,7 @@ public:
* @param[in] aType The type of a specific TLV.
*
*/
void Remove(Tlv::Type aType);
void RemoveTlv(Tlv::Type aType);
/**
* This method appends the MLE Dataset TLV but excluding MeshCoP Sub Timestamp TLV.
@@ -323,6 +323,42 @@ public:
*/
void ConvertToActive(void);
/**
* This method returns a pointer to the start of Dataset TLVs sequence.
*
* @return A pointer to the start of Dataset TLVs sequence.
*
*/
Tlv *GetTlvsStart(void) { return reinterpret_cast<Tlv *>(mTlvs); }
/**
* This method returns a pointer to the start of Dataset TLVs sequence.
*
* @return A pointer to start of Dataset TLVs sequence.
*
*/
const Tlv *GetTlvsStart(void) const { return reinterpret_cast<const Tlv *>(mTlvs); }
/**
* This method returns a pointer to the past-the-end of Dataset TLVs sequence.
*
* Note that past-the-end points to the byte after the end of the last TLV in Dataset TLVs sequence.
*
* @return A pointer to past-the-end of Dataset TLVs sequence.
*
*/
Tlv *GetTlvsEnd(void) { return reinterpret_cast<Tlv *>(mTlvs + mLength); }
/**
* This method returns a pointer to the past-the-end of Dataset TLVs sequence.
*
* Note that past-the-end points to the byte after the end of the last TLV in Dataset TLVs sequence.
*
* @return A pointer to past-the-end of Dataset TLVs sequence.
*
*/
const Tlv *GetTlvsEnd(void) const { return reinterpret_cast<const Tlv *>(mTlvs + mLength); }
/**
* This static method converts a Dataset Type to a string.
*
@@ -332,7 +368,7 @@ public:
static const char *TypeToString(Type aType);
private:
void Remove(uint8_t *aStart, uint8_t aLength);
void RemoveTlv(Tlv *aTlv);
uint8_t mTlvs[kMaxSize]; ///< The Dataset buffer
TimeMilli mUpdateTime; ///< Local time last updated
+4 -28
View File
@@ -100,8 +100,8 @@ otError DatasetLocal::Read(Dataset &aDataset) const
if (mType == Dataset::kActive)
{
aDataset.Remove(Tlv::kPendingTimestamp);
aDataset.Remove(Tlv::kDelayTimer);
aDataset.RemoveTlv(Tlv::kPendingTimestamp);
aDataset.RemoveTlv(Tlv::kDelayTimer);
}
else
{
@@ -196,32 +196,8 @@ exit:
int DatasetLocal::Compare(const Timestamp *aCompare)
{
int rval = 1;
if (aCompare == NULL)
{
if (!mTimestampPresent)
{
rval = 0;
}
else
{
rval = -1;
}
}
else
{
if (!mTimestampPresent)
{
rval = 1;
}
else
{
rval = mTimestamp.Compare(*aCompare);
}
}
return rval;
return (aCompare == NULL) ? (!mTimestampPresent ? 0 : -1)
: (!mTimestampPresent ? 1 : mTimestamp.Compare(*aCompare));
}
} // namespace MeshCoP
+6 -12
View File
@@ -353,17 +353,12 @@ void DatasetManager::SendGetResponse(const Coap::Message & aRequest,
if (aLength == 0)
{
const Tlv *cur = reinterpret_cast<const Tlv *>(dataset.GetBytes());
const Tlv *end = reinterpret_cast<const Tlv *>(dataset.GetBytes() + dataset.GetSize());
while (cur < end)
for (const Tlv *cur = dataset.GetTlvsStart(); cur < dataset.GetTlvsEnd(); cur = cur->GetNext())
{
if (cur->GetType() != Tlv::kNetworkMasterKey || Get<KeyManager>().IsObtainMasterKeyEnabled())
{
SuccessOrExit(error = cur->AppendTo(*message));
}
cur = cur->GetNext();
}
}
else
@@ -417,11 +412,10 @@ otError DatasetManager::SendSetRequest(const otOperationalDataset &aDataset, con
if (Get<Commissioner>().IsActive())
{
const Tlv *cur = reinterpret_cast<const Tlv *>(aTlvs);
const Tlv *end = reinterpret_cast<const Tlv *>(aTlvs + aLength);
bool hasSessionId = false;
for (; cur < end; cur = cur->GetNext())
for (const Tlv *cur = reinterpret_cast<const Tlv *>(aTlvs); cur < end; cur = cur->GetNext())
{
VerifyOrExit((cur + 1) <= end, error = OT_ERROR_INVALID_ARGS);
@@ -445,8 +439,8 @@ otError DatasetManager::SendSetRequest(const otOperationalDataset &aDataset, con
{
ActiveTimestampTlv timestamp;
timestamp.Init();
static_cast<Timestamp *>(&timestamp)->SetSeconds(aDataset.mActiveTimestamp);
static_cast<Timestamp *>(&timestamp)->SetTicks(0);
timestamp.SetSeconds(aDataset.mActiveTimestamp);
timestamp.SetTicks(0);
SuccessOrExit(error = timestamp.AppendTo(*message));
}
@@ -454,8 +448,8 @@ otError DatasetManager::SendSetRequest(const otOperationalDataset &aDataset, con
{
PendingTimestampTlv timestamp;
timestamp.Init();
static_cast<Timestamp *>(&timestamp)->SetSeconds(aDataset.mPendingTimestamp);
static_cast<Timestamp *>(&timestamp)->SetTicks(0);
timestamp.SetSeconds(aDataset.mPendingTimestamp);
timestamp.SetTicks(0);
SuccessOrExit(error = timestamp.AppendTo(*message));
}