From f73b52349d82652026584a3e4d962c1d42203252 Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Fri, 9 Dec 2016 08:46:19 -0800 Subject: [PATCH] Clean up Commissioning Data TLV processing. (#1051) --- src/core/meshcop/dataset_manager.cpp | 57 ++++++------------------- src/core/meshcop/joiner_router.cpp | 56 +++++++----------------- src/core/thread/energy_scan_server.cpp | 4 +- src/core/thread/mle_router.cpp | 22 +++------- src/core/thread/network_data_leader.cpp | 39 ++++++++++++++--- src/core/thread/network_data_leader.hpp | 14 ++++-- 6 files changed, 76 insertions(+), 116 deletions(-) diff --git a/src/core/meshcop/dataset_manager.cpp b/src/core/meshcop/dataset_manager.cpp index 608951840..8e606cc82 100644 --- a/src/core/meshcop/dataset_manager.cpp +++ b/src/core/meshcop/dataset_manager.cpp @@ -400,31 +400,17 @@ ThreadError DatasetManager::Set(Coap::Header &aHeader, Message &aMessage, const // verify session id is the same if (tlvType == Tlv::kCommissionerSessionId) { - uint8_t *cur; - uint8_t *end; - uint8_t length; + CommissionerSessionIdTlv *sessionId; + uint16_t rxSessionId; isUpdateFromCommissioner = true; - cur = mNetworkDataLeader.GetCommissioningData(length); - end = cur + length; - while (cur < end) - { - Tlv *data = reinterpret_cast(cur); + sessionId = static_cast(mNetworkDataLeader.GetCommissioningDataSubTlv( + Tlv::kCommissionerSessionId)); + aMessage.Read(offset + sizeof(tlv), sizeof(rxSessionId), &rxSessionId); - if (data->GetType() == Tlv::kCommissionerSessionId) - { - uint16_t sessionId; - uint16_t rxSessionId; - - sessionId = static_cast(data)->GetCommissionerSessionId(); - aMessage.Read(offset + sizeof(tlv), sizeof(rxSessionId), &rxSessionId); - VerifyOrExit(sessionId == rxSessionId, state = StateTlv::kReject); - break; - } - - cur += sizeof(Tlv) + data->GetLength(); - } + VerifyOrExit(sessionId != NULL && sessionId->GetCommissionerSessionId() == rxSessionId, + state = StateTlv::kReject); } // verify that TLV data size is less than maximum TLV value size @@ -512,38 +498,19 @@ ThreadError DatasetManager::Set(Coap::Header &aHeader, Message &aMessage, const // notify commissioner if update is from thread device if (!isUpdateFromCommissioner) { - uint8_t *cur; - uint8_t *end; - uint8_t length; - uint16_t locator = 0xffff; + BorderAgentLocatorTlv *borderAgentLocator; Ip6::Address destination; - cur = mNetworkDataLeader.GetCommissioningData(length); - VerifyOrExit(cur != NULL && length != 0, ;); - end = cur + length; - - while (cur < end) - { - Tlv *data = reinterpret_cast(cur); - - if (data->GetType() == Tlv::kBorderAgentLocator) - { - locator = static_cast(data)->GetBorderAgentLocator(); - break; - } - - cur += sizeof(Tlv) + data->GetLength(); - } - - // verify that Border Agent Locator should be available - VerifyOrExit(locator != 0xffff, ;); + borderAgentLocator = static_cast(mNetworkDataLeader.GetCommissioningDataSubTlv( + Tlv::kBorderAgentLocator)); + VerifyOrExit(borderAgentLocator != NULL,); memset(&destination, 0, sizeof(destination)); destination = mNetif.GetMle().GetMeshLocal16(); destination.mFields.m16[4] = HostSwap16(0x0000); destination.mFields.m16[5] = HostSwap16(0x00ff); destination.mFields.m16[6] = HostSwap16(0xfe00); - destination.mFields.m16[7] = HostSwap16(locator); + destination.mFields.m16[7] = HostSwap16(borderAgentLocator->GetBorderAgentLocator()); mNetif.GetLeader().SendDatasetChanged(destination); } diff --git a/src/core/meshcop/joiner_router.cpp b/src/core/meshcop/joiner_router.cpp index f2154ca09..b4a3e7721 100644 --- a/src/core/meshcop/joiner_router.cpp +++ b/src/core/meshcop/joiner_router.cpp @@ -77,14 +77,12 @@ void JoinerRouter::HandleNetifStateChanged(uint32_t aFlags, void *aContext) void JoinerRouter::HandleNetifStateChanged(uint32_t aFlags) { - uint8_t length; - VerifyOrExit(mNetif.GetMle().GetDeviceMode() & Mle::ModeTlv::kModeFFD, ;); VerifyOrExit(aFlags & OT_THREAD_NETDATA_UPDATED, ;); mNetif.GetIp6Filter().RemoveUnsecurePort(mSocket.GetSockName().mPort); - if (mNetif.GetNetworkDataLeader().GetCommissioningData(length) != NULL) + if (mNetif.GetNetworkDataLeader().GetCommissioningData() != NULL) { Ip6::SockAddr sockaddr; @@ -106,27 +104,14 @@ exit: ThreadError JoinerRouter::GetBorderAgentRloc(uint16_t &aRloc) { - ThreadError error = kThreadError_NotFound; - uint8_t *cur; - uint8_t *end; - uint8_t length; + ThreadError error = kThreadError_None; + BorderAgentLocatorTlv *borderAgentLocator; - VerifyOrExit((cur = mNetif.GetNetworkDataLeader().GetCommissioningData(length)) != NULL, ;); + borderAgentLocator = static_cast(mNetif.GetNetworkDataLeader().GetCommissioningDataSubTlv( + Tlv::kBorderAgentLocator)); + VerifyOrExit(borderAgentLocator != NULL, error = kThreadError_NotFound); - end = cur + length; - - while (cur < end) - { - Tlv *tlv = reinterpret_cast(cur); - - if (tlv->GetType() == Tlv::kBorderAgentLocator) - { - aRloc = reinterpret_cast(tlv)->GetBorderAgentLocator(); - ExitNow(error = kThreadError_None); - } - - cur += sizeof(Tlv) + tlv->GetLength(); - } + aRloc = borderAgentLocator->GetBorderAgentLocator(); exit: return error; @@ -134,30 +119,19 @@ exit: uint16_t JoinerRouter::GetJoinerUdpPort(void) { - uint16_t joinerUdpPort = OPENTHREAD_CONFIG_JOINER_UDP_PORT; - uint8_t *cur; - uint8_t *end; - uint8_t length; + uint16_t rval = OPENTHREAD_CONFIG_JOINER_UDP_PORT; + JoinerUdpPortTlv *joinerUdpPort; - VerifyOrExit(!mIsJoinerPortConfigured, joinerUdpPort = mJoinerUdpPort); - VerifyOrExit((cur = mNetif.GetNetworkDataLeader().GetCommissioningData(length)) != NULL, ;); + VerifyOrExit(!mIsJoinerPortConfigured, rval = mJoinerUdpPort); - end = cur + length; + joinerUdpPort = static_cast(mNetif.GetNetworkDataLeader().GetCommissioningDataSubTlv( + Tlv::kJoinerUdpPort)); + VerifyOrExit(joinerUdpPort != NULL,); - while (cur < end) - { - Tlv *tlv = reinterpret_cast(cur); - - if (tlv->GetType() == Tlv::kJoinerUdpPort) - { - ExitNow(joinerUdpPort = reinterpret_cast(tlv)->GetUdpPort()); - } - - cur += sizeof(Tlv) + tlv->GetLength(); - } + rval = joinerUdpPort->GetUdpPort(); exit: - return joinerUdpPort; + return rval; } ThreadError JoinerRouter::SetJoinerUdpPort(uint16_t aJoinerUdpPort) diff --git a/src/core/thread/energy_scan_server.cpp b/src/core/thread/energy_scan_server.cpp index 84967b431..9504bca11 100644 --- a/src/core/thread/energy_scan_server.cpp +++ b/src/core/thread/energy_scan_server.cpp @@ -252,11 +252,9 @@ void EnergyScanServer::HandleNetifStateChanged(uint32_t aFlags, void *aContext) void EnergyScanServer::HandleNetifStateChanged(uint32_t aFlags) { - uint8_t length; - if ((aFlags & OT_THREAD_NETDATA_UPDATED) != 0 && !mActive && - mNetif.GetNetworkDataLeader().GetCommissioningData(length) == NULL) + mNetif.GetNetworkDataLeader().GetCommissioningData() == NULL) { mActive = false; mTimer.Stop(); diff --git a/src/core/thread/mle_router.cpp b/src/core/thread/mle_router.cpp index b7d49e9ec..5631af225 100644 --- a/src/core/thread/mle_router.cpp +++ b/src/core/thread/mle_router.cpp @@ -2582,8 +2582,7 @@ ThreadError MleRouter::SendDiscoveryResponse(const Ip6::Address &aDestination, u MeshCoP::ExtendedPanIdTlv extPanId; MeshCoP::NetworkNameTlv networkName; MeshCoP::JoinerUdpPortTlv joinerUdpPort; - uint8_t *cur; - uint8_t length; + MeshCoP::Tlv *steeringData; VerifyOrExit((message = NewMessage()) != NULL, ;); message->SetLinkSecurityEnabled(false); @@ -2623,22 +2622,11 @@ ThreadError MleRouter::SendDiscoveryResponse(const Ip6::Address &aDestination, u SuccessOrExit(error = message->Append(&networkName, sizeof(tlv) + networkName.GetLength())); // Steering Data TLV - if ((cur = mNetif.GetNetworkDataLeader().GetCommissioningData(length)) != NULL) + steeringData = mNetif.GetNetworkDataLeader().GetCommissioningDataSubTlv(MeshCoP::Tlv::kSteeringData); + + if (steeringData != NULL) { - uint8_t *end = cur + length; - - while (cur < end) - { - MeshCoP::Tlv *meshcop = reinterpret_cast(cur); - - if (meshcop->GetType() == MeshCoP::Tlv::kSteeringData) - { - SuccessOrExit(message->Append(meshcop, sizeof(*meshcop) + meshcop->GetLength())); - break; - } - - cur += sizeof(*meshcop) + meshcop->GetLength(); - } + SuccessOrExit(message->Append(steeringData, sizeof(*steeringData) + steeringData->GetLength())); } // Joiner UDP Port TLV diff --git a/src/core/thread/network_data_leader.cpp b/src/core/thread/network_data_leader.cpp index 827acaef5..fac2a5805 100644 --- a/src/core/thread/network_data_leader.cpp +++ b/src/core/thread/network_data_leader.cpp @@ -453,18 +453,43 @@ exit: return error; } -uint8_t *LeaderBase::GetCommissioningData(uint8_t &aLength) +NetworkDataTlv *LeaderBase::GetCommissioningData(void) { - uint8_t *rval = NULL; + NetworkDataTlv *cur = reinterpret_cast(mTlvs); + NetworkDataTlv *end = reinterpret_cast(mTlvs + mLength); - for (NetworkDataTlv *cur = reinterpret_cast(mTlvs); - cur < reinterpret_cast(mTlvs + mLength); - cur = cur->GetNext()) + for (; cur < end; cur = cur->GetNext()) { if (cur->GetType() == NetworkDataTlv::kTypeCommissioningData) { - aLength = cur->GetLength(); - ExitNow(rval = cur->GetValue()); + ExitNow(); + } + } + + cur = NULL; + +exit: + return cur; +} + +MeshCoP::Tlv *LeaderBase::GetCommissioningDataSubTlv(MeshCoP::Tlv::Type aType) +{ + MeshCoP::Tlv *rval = NULL; + NetworkDataTlv *commissioningDataTlv; + MeshCoP::Tlv *cur; + MeshCoP::Tlv *end; + + commissioningDataTlv = GetCommissioningData(); + VerifyOrExit(commissioningDataTlv != NULL,); + + cur = reinterpret_cast(commissioningDataTlv->GetValue()); + end = reinterpret_cast(commissioningDataTlv->GetValue() + commissioningDataTlv->GetLength()); + + for (; cur < end; cur = cur->GetNext()) + { + if (cur->GetType() == aType) + { + ExitNow(rval = cur); } } diff --git a/src/core/thread/network_data_leader.hpp b/src/core/thread/network_data_leader.hpp index 7a0f591fd..0842a904c 100644 --- a/src/core/thread/network_data_leader.hpp +++ b/src/core/thread/network_data_leader.hpp @@ -172,12 +172,20 @@ public: /** * This method returns a pointer to the Commissioning Data. * - * @param[out] aLength The length of the Commissioning Data in bytes. - * * @returns A pointer to the Commissioning Data or NULL if no Commissioning Data exists. * */ - uint8_t *GetCommissioningData(uint8_t &aLength); + NetworkDataTlv *GetCommissioningData(void); + + /** + * This method returns a pointer to the Commissioning Data Sub-TLV. + * + * @param[in] aType The TLV type value. + * + * @returns A pointer to the Commissioning Data Sub-TLV or NULL if no Sub-TLV exists. + * + */ + MeshCoP::Tlv *GetCommissioningDataSubTlv(MeshCoP::Tlv::Type aType); /** * This method adds Commissioning Data to the Thread Network Data.