[thread/cleanup] Change GetThreadProvision to return OperationalDatas…

…et (#17054)

* [thread] Change GetThreadProvision to return OperationalDataset

OpenThread stack manager has OperationalDataset member only
used as a temporary object for returning an operational
dataset as ByteSpan in GetThreadProvision method. Change the
interface to save >250 bytes of RAM.

Additionally, add bound checks in
OperationalDataset::GetExtendedPanId methods.

* Restyled by clang-format

Co-authored-by: Restyled.io <commits@restyled.io>

src/include/platform/ThreadStackManager.h

@@ -36,6 +36,10 @@ struct TextEntry;
 struct DnssdService;
 } // namespace Dnssd
 
+namespace Thread {
+class OperationalDataset;
+} // namespace Thread
+
 namespace DeviceLayer {
 
 class PlatformManagerImpl;
@@ -93,7 +97,7 @@ class ThreadStackManager
  bool IsThreadEnabled();
  bool IsThreadProvisioned();
  bool IsThreadAttached();
- CHIP_ERROR GetThreadProvision(ByteSpan & netInfo);
+ CHIP_ERROR GetThreadProvision(Thread::OperationalDataset & dataset);
  CHIP_ERROR GetAndLogThreadStatsCounters();
  CHIP_ERROR GetAndLogThreadTopologyMinimal();
  CHIP_ERROR GetAndLogThreadTopologyFull();
@@ -342,9 +346,9 @@ inline bool ThreadStackManager::IsThreadAttached()
  return static_cast<ImplClass *>(this)->_IsThreadAttached();
 }
 
-inline CHIP_ERROR ThreadStackManager::GetThreadProvision(ByteSpan & netInfo)
+inline CHIP_ERROR ThreadStackManager::GetThreadProvision(Thread::OperationalDataset & dataset)
 {
- return static_cast<ImplClass *>(this)->_GetThreadProvision(netInfo);
+ return static_cast<ImplClass *>(this)->_GetThreadProvision(dataset);
 }
 
 inline CHIP_ERROR ThreadStackManager::SetThreadProvision(ByteSpan netInfo)

src/lib/support/ThreadOperationalDataset.cpp

@@ -277,28 +277,34 @@ CHIP_ERROR OperationalDataset::SetChannel(uint16_t aChannel)
 
 CHIP_ERROR OperationalDataset::GetExtendedPanId(uint8_t (&aExtendedPanId)[kSizeExtendedPanId]) const
 {
- const ThreadTLV * tlv = Locate(ThreadTLV::kExtendedPanId);
+ ByteSpan extPanIdSpan;
+ CHIP_ERROR error = GetExtendedPanIdAsByteSpan(extPanIdSpan);
 
- if (tlv != nullptr)
+ if (error != CHIP_NO_ERROR)
  {
- memcpy(aExtendedPanId, tlv->GetValue(), sizeof(aExtendedPanId));
- return CHIP_NO_ERROR;
+ return error;
  }
 
- return CHIP_ERROR_TLV_TAG_NOT_FOUND;
+ memcpy(aExtendedPanId, extPanIdSpan.data(), extPanIdSpan.size());
+ return CHIP_NO_ERROR;
 }
 
 CHIP_ERROR OperationalDataset::GetExtendedPanIdAsByteSpan(ByteSpan & span) const
 {
  const ThreadTLV * tlv = Locate(ThreadTLV::kExtendedPanId);
 
- if (tlv != nullptr)
+ if (tlv == nullptr)
  {
- span = ByteSpan(reinterpret_cast<const uint8_t *>(tlv->GetValue()), tlv->GetLength());
- return CHIP_NO_ERROR;
+ return CHIP_ERROR_TLV_TAG_NOT_FOUND;
  }
 
- return CHIP_ERROR_TLV_TAG_NOT_FOUND;
+ if (tlv->GetLength() != kSizeExtendedPanId)
+ {
+ return CHIP_ERROR_INVALID_TLV_ELEMENT;
+ }
+
+ span = ByteSpan(static_cast<const uint8_t *>(tlv->GetValue()), tlv->GetLength());
+ return CHIP_NO_ERROR;
 }
 
 CHIP_ERROR OperationalDataset::SetExtendedPanId(const uint8_t (&aExtendedPanId)[kSizeExtendedPanId])

src/platform/Linux/NetworkCommissioningThreadDriver.cpp

@@ -43,12 +43,10 @@ namespace NetworkCommissioning {
 
 CHIP_ERROR LinuxThreadDriver::Init(BaseDriver::NetworkStatusChangeCallback * networkStatusChangeCallback)
 {
- ByteSpan currentProvision;
  VerifyOrReturnError(ConnectivityMgrImpl().IsThreadAttached(), CHIP_NO_ERROR);
- VerifyOrReturnError(ThreadStackMgrImpl().GetThreadProvision(currentProvision) == CHIP_NO_ERROR, CHIP_NO_ERROR);
+ VerifyOrReturnError(ThreadStackMgrImpl().GetThreadProvision(mStagingNetwork) == CHIP_NO_ERROR, CHIP_NO_ERROR);
 
- mSavedNetwork.Init(currentProvision);
- mStagingNetwork.Init(currentProvision);
+ mSavedNetwork.Init(mStagingNetwork.AsByteSpan());
 
  ThreadStackMgrImpl().SetNetworkStatusChangeCallback(networkStatusChangeCallback);
 
@@ -185,14 +183,12 @@ bool LinuxThreadDriver::ThreadNetworkIterator::Next(Network & item)
  item.connected = false;
  exhausted = true;
 
- ByteSpan currentProvision;
  Thread::OperationalDataset currentDataset;
  uint8_t enabledExtPanId[Thread::kSizeExtendedPanId];
 
  // The Thread network is not actually enabled.
  VerifyOrReturnError(ConnectivityMgrImpl().IsThreadAttached(), true);
- VerifyOrReturnError(ThreadStackMgrImpl().GetThreadProvision(currentProvision) == CHIP_NO_ERROR, true);
- VerifyOrReturnError(currentDataset.Init(currentProvision) == CHIP_NO_ERROR, true);
+ VerifyOrReturnError(ThreadStackMgrImpl().GetThreadProvision(currentDataset) == CHIP_NO_ERROR, true);
  // The Thread network is not enabled, but has a different extended pan id.
  VerifyOrReturnError(currentDataset.GetExtendedPanId(enabledExtPanId) == CHIP_NO_ERROR, true);
  VerifyOrReturnError(memcmp(extpanid, enabledExtPanId, kSizeExtendedPanId) == 0, true);

src/platform/Linux/ThreadStackManagerImpl.cpp

@@ -235,7 +235,7 @@ CHIP_ERROR ThreadStackManagerImpl::_SetThreadProvision(ByteSpan netInfo)
  return PlatformMgr().PostEvent(&event);
 }
 
-CHIP_ERROR ThreadStackManagerImpl::_GetThreadProvision(ByteSpan & netInfo)
+CHIP_ERROR ThreadStackManagerImpl::_GetThreadProvision(Thread::OperationalDataset & dataset)
 {
  VerifyOrReturnError(mProxy, CHIP_ERROR_INCORRECT_STATE);
 
@@ -257,23 +257,20 @@ CHIP_ERROR ThreadStackManagerImpl::_GetThreadProvision(ByteSpan & netInfo)
 
  if (response == nullptr)
  {
- netInfo = ByteSpan();
  return CHIP_ERROR_KEY_NOT_FOUND;
  }
 
  std::unique_ptr<GVariant, GVariantDeleter> tupleContent(g_variant_get_child_value(response.get(), 0));
 
  if (tupleContent == nullptr)
  {
- netInfo = ByteSpan();
  return CHIP_ERROR_KEY_NOT_FOUND;
  }
 
  std::unique_ptr<GVariant, GVariantDeleter> value(g_variant_get_variant(tupleContent.get()));
 
  if (value == nullptr)
  {
- netInfo = ByteSpan();
  return CHIP_ERROR_KEY_NOT_FOUND;
  }
 
@@ -282,7 +279,7 @@ CHIP_ERROR ThreadStackManagerImpl::_GetThreadProvision(ByteSpan & netInfo)
  ReturnErrorOnFailure(mDataset.Init(ByteSpan(data, size)));
  }
 
- netInfo = mDataset.AsByteSpan();
+ dataset.Init(mDataset.AsByteSpan());
 
  return CHIP_NO_ERROR;
 }
@@ -716,20 +713,18 @@ void ThreadStackManagerImpl::_UpdateNetworkStatus()
  // Thread is not enabled, then we are not trying to connect to the network.
  VerifyOrReturn(IsThreadEnabled() && mpStatusChangeCallback != nullptr);
 
- ByteSpan datasetTLV;
  Thread::OperationalDataset dataset;
  uint8_t extpanid[Thread::kSizeExtendedPanId];
 
  // If we have not provisioned any Thread network, return the status from last network scan,
  // If we have provisioned a network, we assume the ot-br-posix is activitely connecting to that network.
- CHIP_ERROR err = ThreadStackMgrImpl().GetThreadProvision(datasetTLV);
+ CHIP_ERROR err = ThreadStackMgrImpl().GetThreadProvision(dataset);
  if (err != CHIP_NO_ERROR)
  {
  ChipLogError(DeviceLayer, "Failed to get configured network when updating network status: %s", err.AsString());
  return;
  }
 
- VerifyOrReturn(dataset.Init(datasetTLV) == CHIP_NO_ERROR);
  // The Thread network is not enabled, but has a different extended pan id.
  VerifyOrReturn(dataset.GetExtendedPanId(extpanid) == CHIP_NO_ERROR);
 

src/platform/Linux/ThreadStackManagerImpl.h

@@ -54,7 +54,7 @@ class ThreadStackManagerImpl : public ThreadStackManager
 
  void _OnPlatformEvent(const ChipDeviceEvent * event);
 
- CHIP_ERROR _GetThreadProvision(ByteSpan & netInfo);
+ CHIP_ERROR _GetThreadProvision(Thread::OperationalDataset & dataset);
 
  CHIP_ERROR _SetThreadProvision(ByteSpan netInfo);
 

src/platform/OpenThread/GenericNetworkCommissioningThreadDriver.cpp

@@ -37,14 +37,12 @@ namespace NetworkCommissioning {
 
 CHIP_ERROR GenericThreadDriver::Init(Internal::BaseDriver::NetworkStatusChangeCallback * statusChangeCallback)
 {
- ByteSpan currentProvision;
  ThreadStackMgrImpl().SetNetworkStatusChangeCallback(statusChangeCallback);
 
  VerifyOrReturnError(ThreadStackMgrImpl().IsThreadAttached(), CHIP_NO_ERROR);
- VerifyOrReturnError(ThreadStackMgrImpl().GetThreadProvision(currentProvision) == CHIP_NO_ERROR, CHIP_NO_ERROR);
+ VerifyOrReturnError(ThreadStackMgrImpl().GetThreadProvision(mStagingNetwork) == CHIP_NO_ERROR, CHIP_NO_ERROR);
 
- mSavedNetwork.Init(currentProvision);
- mStagingNetwork.Init(currentProvision);
+ mSavedNetwork.Init(mStagingNetwork.AsByteSpan());
 
  return CHIP_NO_ERROR;
 }
@@ -182,14 +180,12 @@ bool GenericThreadDriver::ThreadNetworkIterator::Next(Network & item)
  item.connected = false;
  exhausted = true;
 
- ByteSpan currentProvision;
  Thread::OperationalDataset currentDataset;
  uint8_t enabledExtPanId[Thread::kSizeExtendedPanId];
 
  // The Thread network is not actually enabled.
  VerifyOrReturnError(ConnectivityMgrImpl().IsThreadAttached(), true);
- VerifyOrReturnError(ThreadStackMgrImpl().GetThreadProvision(currentProvision) == CHIP_NO_ERROR, true);
- VerifyOrReturnError(currentDataset.Init(currentProvision) == CHIP_NO_ERROR, true);
+ VerifyOrReturnError(ThreadStackMgrImpl().GetThreadProvision(currentDataset) == CHIP_NO_ERROR, true);
  // The Thread network is not enabled, but has a different extended pan id.
  VerifyOrReturnError(currentDataset.GetExtendedPanId(enabledExtPanId) == CHIP_NO_ERROR, true);
  VerifyOrReturnError(memcmp(extpanid, enabledExtPanId, kSizeExtendedPanId) == 0, true);

src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp

@@ -304,7 +304,7 @@ bool GenericThreadStackManagerImpl_OpenThread<ImplClass>::_IsThreadProvisioned(v
 }
 
 template <class ImplClass>
-CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::_GetThreadProvision(ByteSpan & netInfo)
+CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::_GetThreadProvision(Thread::OperationalDataset & dataset)
 {
  VerifyOrReturnError(Impl()->IsThreadProvisioned(), CHIP_ERROR_INCORRECT_STATE);
  otOperationalDatasetTlvs datasetTlv;
@@ -317,8 +317,7 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::_GetThreadProvis
  return MapOpenThreadError(otErr);
  }
 
- ReturnErrorOnFailure(mActiveDataset.Init(ByteSpan(datasetTlv.mTlvs, datasetTlv.mLength)));
- netInfo = mActiveDataset.AsByteSpan();
+ ReturnErrorOnFailure(dataset.Init(ByteSpan(datasetTlv.mTlvs, datasetTlv.mLength)));
 
  return CHIP_NO_ERROR;
 }
@@ -1853,24 +1852,23 @@ void GenericThreadStackManagerImpl_OpenThread<ImplClass>::_UpdateNetworkStatus()
 
  ByteSpan datasetTLV;
  Thread::OperationalDataset dataset;
- uint8_t extpanid[chip::Thread::kSizeExtendedPanId];
+ ByteSpan extpanid;
 
  // If we have not provisioned any Thread network, return the status from last network scan,
  // If we have provisioned a network, we assume the ot-br-posix is activitely connecting to that network.
- ReturnOnFailure(ThreadStackMgrImpl().GetThreadProvision(datasetTLV));
- ReturnOnFailure(dataset.Init(datasetTLV));
+ ReturnOnFailure(ThreadStackMgrImpl().GetThreadProvision(dataset));
  // The Thread network is not enabled, but has a different extended pan id.
- ReturnOnFailure(dataset.GetExtendedPanId(extpanid));
+ ReturnOnFailure(dataset.GetExtendedPanIdAsByteSpan(extpanid));
  // If we don't have a valid dataset, we are not attempting to connect the network.
 
  // We have already connected to the network, thus return success.
  if (ThreadStackMgrImpl().IsThreadAttached())
  {
- mpStatusChangeCallback->OnNetworkingStatusChange(Status::kSuccess, MakeOptional(ByteSpan(extpanid)), NullOptional);
+ mpStatusChangeCallback->OnNetworkingStatusChange(Status::kSuccess, MakeOptional(extpanid), NullOptional);
  }
  else
  {
- mpStatusChangeCallback->OnNetworkingStatusChange(Status::kNetworkNotFound, MakeOptional(ByteSpan(extpanid)),
+ mpStatusChangeCallback->OnNetworkingStatusChange(Status::kNetworkNotFound, MakeOptional(extpanid),
  MakeOptional(static_cast<int32_t>(OT_ERROR_DETACHED)));
  }
 }

src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.h

@@ -87,7 +87,7 @@ class GenericThreadStackManagerImpl_OpenThread
 
  bool _IsThreadProvisioned(void);
  bool _IsThreadAttached(void);
- CHIP_ERROR _GetThreadProvision(ByteSpan & netInfo);
+ CHIP_ERROR _GetThreadProvision(Thread::OperationalDataset & dataset);
  CHIP_ERROR _SetThreadProvision(ByteSpan netInfo);
  CHIP_ERROR _AttachToThreadNetwork(ByteSpan netInfo, NetworkCommissioning::Internal::WirelessDriver::ConnectCallback * callback);
  void _OnThreadAttachFinished(void);
@@ -149,8 +149,7 @@ class GenericThreadStackManagerImpl_OpenThread
  // ===== Private members for use by this class only.
 
  otInstance * mOTInst;
- uint64_t mOverrunCount = 0;
- Thread::OperationalDataset mActiveDataset = {};
+ uint64_t mOverrunCount = 0;
 
  NetworkCommissioning::ThreadDriver::ScanCallback * mpScanCallback;
  NetworkCommissioning::Internal::WirelessDriver::ConnectCallback * mpConnectCallback;