Skip to content

Commit 1113490

Browse files
Damian-Nordicrestyled-commits
authored andcommitted
[thread/cleanup] Change GetThreadProvision to return OperationalDataset (#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>
1 parent 2c71fee commit 1113490

8 files changed

+41
-47
lines changed

src/include/platform/ThreadStackManager.h

+7-3
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ struct TextEntry;
3636
struct DnssdService;
3737
} // namespace Dnssd
3838

39+
namespace Thread {
40+
class OperationalDataset;
41+
} // namespace Thread
42+
3943
namespace DeviceLayer {
4044

4145
class PlatformManagerImpl;
@@ -93,7 +97,7 @@ class ThreadStackManager
9397
bool IsThreadEnabled();
9498
bool IsThreadProvisioned();
9599
bool IsThreadAttached();
96-
CHIP_ERROR GetThreadProvision(ByteSpan & netInfo);
100+
CHIP_ERROR GetThreadProvision(Thread::OperationalDataset & dataset);
97101
CHIP_ERROR GetAndLogThreadStatsCounters();
98102
CHIP_ERROR GetAndLogThreadTopologyMinimal();
99103
CHIP_ERROR GetAndLogThreadTopologyFull();
@@ -342,9 +346,9 @@ inline bool ThreadStackManager::IsThreadAttached()
342346
return static_cast<ImplClass *>(this)->_IsThreadAttached();
343347
}
344348

345-
inline CHIP_ERROR ThreadStackManager::GetThreadProvision(ByteSpan & netInfo)
349+
inline CHIP_ERROR ThreadStackManager::GetThreadProvision(Thread::OperationalDataset & dataset)
346350
{
347-
return static_cast<ImplClass *>(this)->_GetThreadProvision(netInfo);
351+
return static_cast<ImplClass *>(this)->_GetThreadProvision(dataset);
348352
}
349353

350354
inline CHIP_ERROR ThreadStackManager::SetThreadProvision(ByteSpan netInfo)

src/lib/support/ThreadOperationalDataset.cpp

+15-9
Original file line numberDiff line numberDiff line change
@@ -277,28 +277,34 @@ CHIP_ERROR OperationalDataset::SetChannel(uint16_t aChannel)
277277

278278
CHIP_ERROR OperationalDataset::GetExtendedPanId(uint8_t (&aExtendedPanId)[kSizeExtendedPanId]) const
279279
{
280-
const ThreadTLV * tlv = Locate(ThreadTLV::kExtendedPanId);
280+
ByteSpan extPanIdSpan;
281+
CHIP_ERROR error = GetExtendedPanIdAsByteSpan(extPanIdSpan);
281282

282-
if (tlv != nullptr)
283+
if (error != CHIP_NO_ERROR)
283284
{
284-
memcpy(aExtendedPanId, tlv->GetValue(), sizeof(aExtendedPanId));
285-
return CHIP_NO_ERROR;
285+
return error;
286286
}
287287

288-
return CHIP_ERROR_TLV_TAG_NOT_FOUND;
288+
memcpy(aExtendedPanId, extPanIdSpan.data(), extPanIdSpan.size());
289+
return CHIP_NO_ERROR;
289290
}
290291

291292
CHIP_ERROR OperationalDataset::GetExtendedPanIdAsByteSpan(ByteSpan & span) const
292293
{
293294
const ThreadTLV * tlv = Locate(ThreadTLV::kExtendedPanId);
294295

295-
if (tlv != nullptr)
296+
if (tlv == nullptr)
296297
{
297-
span = ByteSpan(reinterpret_cast<const uint8_t *>(tlv->GetValue()), tlv->GetLength());
298-
return CHIP_NO_ERROR;
298+
return CHIP_ERROR_TLV_TAG_NOT_FOUND;
299299
}
300300

301-
return CHIP_ERROR_TLV_TAG_NOT_FOUND;
301+
if (tlv->GetLength() != kSizeExtendedPanId)
302+
{
303+
return CHIP_ERROR_INVALID_TLV_ELEMENT;
304+
}
305+
306+
span = ByteSpan(static_cast<const uint8_t *>(tlv->GetValue()), tlv->GetLength());
307+
return CHIP_NO_ERROR;
302308
}
303309

304310
CHIP_ERROR OperationalDataset::SetExtendedPanId(const uint8_t (&aExtendedPanId)[kSizeExtendedPanId])

src/platform/Linux/NetworkCommissioningThreadDriver.cpp

+3-7
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,10 @@ namespace NetworkCommissioning {
4343

4444
CHIP_ERROR LinuxThreadDriver::Init(BaseDriver::NetworkStatusChangeCallback * networkStatusChangeCallback)
4545
{
46-
ByteSpan currentProvision;
4746
VerifyOrReturnError(ConnectivityMgrImpl().IsThreadAttached(), CHIP_NO_ERROR);
48-
VerifyOrReturnError(ThreadStackMgrImpl().GetThreadProvision(currentProvision) == CHIP_NO_ERROR, CHIP_NO_ERROR);
47+
VerifyOrReturnError(ThreadStackMgrImpl().GetThreadProvision(mStagingNetwork) == CHIP_NO_ERROR, CHIP_NO_ERROR);
4948

50-
mSavedNetwork.Init(currentProvision);
51-
mStagingNetwork.Init(currentProvision);
49+
mSavedNetwork.Init(mStagingNetwork.AsByteSpan());
5250

5351
ThreadStackMgrImpl().SetNetworkStatusChangeCallback(networkStatusChangeCallback);
5452

@@ -185,14 +183,12 @@ bool LinuxThreadDriver::ThreadNetworkIterator::Next(Network & item)
185183
item.connected = false;
186184
exhausted = true;
187185

188-
ByteSpan currentProvision;
189186
Thread::OperationalDataset currentDataset;
190187
uint8_t enabledExtPanId[Thread::kSizeExtendedPanId];
191188

192189
// The Thread network is not actually enabled.
193190
VerifyOrReturnError(ConnectivityMgrImpl().IsThreadAttached(), true);
194-
VerifyOrReturnError(ThreadStackMgrImpl().GetThreadProvision(currentProvision) == CHIP_NO_ERROR, true);
195-
VerifyOrReturnError(currentDataset.Init(currentProvision) == CHIP_NO_ERROR, true);
191+
VerifyOrReturnError(ThreadStackMgrImpl().GetThreadProvision(currentDataset) == CHIP_NO_ERROR, true);
196192
// The Thread network is not enabled, but has a different extended pan id.
197193
VerifyOrReturnError(currentDataset.GetExtendedPanId(enabledExtPanId) == CHIP_NO_ERROR, true);
198194
VerifyOrReturnError(memcmp(extpanid, enabledExtPanId, kSizeExtendedPanId) == 0, true);

src/platform/Linux/ThreadStackManagerImpl.cpp

+3-8
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ CHIP_ERROR ThreadStackManagerImpl::_SetThreadProvision(ByteSpan netInfo)
235235
return PlatformMgr().PostEvent(&event);
236236
}
237237

238-
CHIP_ERROR ThreadStackManagerImpl::_GetThreadProvision(ByteSpan & netInfo)
238+
CHIP_ERROR ThreadStackManagerImpl::_GetThreadProvision(Thread::OperationalDataset & dataset)
239239
{
240240
VerifyOrReturnError(mProxy, CHIP_ERROR_INCORRECT_STATE);
241241

@@ -257,23 +257,20 @@ CHIP_ERROR ThreadStackManagerImpl::_GetThreadProvision(ByteSpan & netInfo)
257257

258258
if (response == nullptr)
259259
{
260-
netInfo = ByteSpan();
261260
return CHIP_ERROR_KEY_NOT_FOUND;
262261
}
263262

264263
std::unique_ptr<GVariant, GVariantDeleter> tupleContent(g_variant_get_child_value(response.get(), 0));
265264

266265
if (tupleContent == nullptr)
267266
{
268-
netInfo = ByteSpan();
269267
return CHIP_ERROR_KEY_NOT_FOUND;
270268
}
271269

272270
std::unique_ptr<GVariant, GVariantDeleter> value(g_variant_get_variant(tupleContent.get()));
273271

274272
if (value == nullptr)
275273
{
276-
netInfo = ByteSpan();
277274
return CHIP_ERROR_KEY_NOT_FOUND;
278275
}
279276

@@ -282,7 +279,7 @@ CHIP_ERROR ThreadStackManagerImpl::_GetThreadProvision(ByteSpan & netInfo)
282279
ReturnErrorOnFailure(mDataset.Init(ByteSpan(data, size)));
283280
}
284281

285-
netInfo = mDataset.AsByteSpan();
282+
dataset.Init(mDataset.AsByteSpan());
286283

287284
return CHIP_NO_ERROR;
288285
}
@@ -716,20 +713,18 @@ void ThreadStackManagerImpl::_UpdateNetworkStatus()
716713
// Thread is not enabled, then we are not trying to connect to the network.
717714
VerifyOrReturn(IsThreadEnabled() && mpStatusChangeCallback != nullptr);
718715

719-
ByteSpan datasetTLV;
720716
Thread::OperationalDataset dataset;
721717
uint8_t extpanid[Thread::kSizeExtendedPanId];
722718

723719
// If we have not provisioned any Thread network, return the status from last network scan,
724720
// If we have provisioned a network, we assume the ot-br-posix is activitely connecting to that network.
725-
CHIP_ERROR err = ThreadStackMgrImpl().GetThreadProvision(datasetTLV);
721+
CHIP_ERROR err = ThreadStackMgrImpl().GetThreadProvision(dataset);
726722
if (err != CHIP_NO_ERROR)
727723
{
728724
ChipLogError(DeviceLayer, "Failed to get configured network when updating network status: %s", err.AsString());
729725
return;
730726
}
731727

732-
VerifyOrReturn(dataset.Init(datasetTLV) == CHIP_NO_ERROR);
733728
// The Thread network is not enabled, but has a different extended pan id.
734729
VerifyOrReturn(dataset.GetExtendedPanId(extpanid) == CHIP_NO_ERROR);
735730

src/platform/Linux/ThreadStackManagerImpl.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ class ThreadStackManagerImpl : public ThreadStackManager
5454

5555
void _OnPlatformEvent(const ChipDeviceEvent * event);
5656

57-
CHIP_ERROR _GetThreadProvision(ByteSpan & netInfo);
57+
CHIP_ERROR _GetThreadProvision(Thread::OperationalDataset & dataset);
5858

5959
CHIP_ERROR _SetThreadProvision(ByteSpan netInfo);
6060

src/platform/OpenThread/GenericNetworkCommissioningThreadDriver.cpp

+3-7
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,12 @@ namespace NetworkCommissioning {
3737

3838
CHIP_ERROR GenericThreadDriver::Init(Internal::BaseDriver::NetworkStatusChangeCallback * statusChangeCallback)
3939
{
40-
ByteSpan currentProvision;
4140
ThreadStackMgrImpl().SetNetworkStatusChangeCallback(statusChangeCallback);
4241

4342
VerifyOrReturnError(ThreadStackMgrImpl().IsThreadAttached(), CHIP_NO_ERROR);
44-
VerifyOrReturnError(ThreadStackMgrImpl().GetThreadProvision(currentProvision) == CHIP_NO_ERROR, CHIP_NO_ERROR);
43+
VerifyOrReturnError(ThreadStackMgrImpl().GetThreadProvision(mStagingNetwork) == CHIP_NO_ERROR, CHIP_NO_ERROR);
4544

46-
mSavedNetwork.Init(currentProvision);
47-
mStagingNetwork.Init(currentProvision);
45+
mSavedNetwork.Init(mStagingNetwork.AsByteSpan());
4846

4947
return CHIP_NO_ERROR;
5048
}
@@ -182,14 +180,12 @@ bool GenericThreadDriver::ThreadNetworkIterator::Next(Network & item)
182180
item.connected = false;
183181
exhausted = true;
184182

185-
ByteSpan currentProvision;
186183
Thread::OperationalDataset currentDataset;
187184
uint8_t enabledExtPanId[Thread::kSizeExtendedPanId];
188185

189186
// The Thread network is not actually enabled.
190187
VerifyOrReturnError(ConnectivityMgrImpl().IsThreadAttached(), true);
191-
VerifyOrReturnError(ThreadStackMgrImpl().GetThreadProvision(currentProvision) == CHIP_NO_ERROR, true);
192-
VerifyOrReturnError(currentDataset.Init(currentProvision) == CHIP_NO_ERROR, true);
188+
VerifyOrReturnError(ThreadStackMgrImpl().GetThreadProvision(currentDataset) == CHIP_NO_ERROR, true);
193189
// The Thread network is not enabled, but has a different extended pan id.
194190
VerifyOrReturnError(currentDataset.GetExtendedPanId(enabledExtPanId) == CHIP_NO_ERROR, true);
195191
VerifyOrReturnError(memcmp(extpanid, enabledExtPanId, kSizeExtendedPanId) == 0, true);

src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp

+7-9
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,7 @@ bool GenericThreadStackManagerImpl_OpenThread<ImplClass>::_IsThreadProvisioned(v
304304
}
305305

306306
template <class ImplClass>
307-
CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::_GetThreadProvision(ByteSpan & netInfo)
307+
CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::_GetThreadProvision(Thread::OperationalDataset & dataset)
308308
{
309309
VerifyOrReturnError(Impl()->IsThreadProvisioned(), CHIP_ERROR_INCORRECT_STATE);
310310
otOperationalDatasetTlvs datasetTlv;
@@ -317,8 +317,7 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::_GetThreadProvis
317317
return MapOpenThreadError(otErr);
318318
}
319319

320-
ReturnErrorOnFailure(mActiveDataset.Init(ByteSpan(datasetTlv.mTlvs, datasetTlv.mLength)));
321-
netInfo = mActiveDataset.AsByteSpan();
320+
ReturnErrorOnFailure(dataset.Init(ByteSpan(datasetTlv.mTlvs, datasetTlv.mLength)));
322321

323322
return CHIP_NO_ERROR;
324323
}
@@ -1853,24 +1852,23 @@ void GenericThreadStackManagerImpl_OpenThread<ImplClass>::_UpdateNetworkStatus()
18531852

18541853
ByteSpan datasetTLV;
18551854
Thread::OperationalDataset dataset;
1856-
uint8_t extpanid[chip::Thread::kSizeExtendedPanId];
1855+
ByteSpan extpanid;
18571856

18581857
// If we have not provisioned any Thread network, return the status from last network scan,
18591858
// If we have provisioned a network, we assume the ot-br-posix is activitely connecting to that network.
1860-
ReturnOnFailure(ThreadStackMgrImpl().GetThreadProvision(datasetTLV));
1861-
ReturnOnFailure(dataset.Init(datasetTLV));
1859+
ReturnOnFailure(ThreadStackMgrImpl().GetThreadProvision(dataset));
18621860
// The Thread network is not enabled, but has a different extended pan id.
1863-
ReturnOnFailure(dataset.GetExtendedPanId(extpanid));
1861+
ReturnOnFailure(dataset.GetExtendedPanIdAsByteSpan(extpanid));
18641862
// If we don't have a valid dataset, we are not attempting to connect the network.
18651863

18661864
// We have already connected to the network, thus return success.
18671865
if (ThreadStackMgrImpl().IsThreadAttached())
18681866
{
1869-
mpStatusChangeCallback->OnNetworkingStatusChange(Status::kSuccess, MakeOptional(ByteSpan(extpanid)), NullOptional);
1867+
mpStatusChangeCallback->OnNetworkingStatusChange(Status::kSuccess, MakeOptional(extpanid), NullOptional);
18701868
}
18711869
else
18721870
{
1873-
mpStatusChangeCallback->OnNetworkingStatusChange(Status::kNetworkNotFound, MakeOptional(ByteSpan(extpanid)),
1871+
mpStatusChangeCallback->OnNetworkingStatusChange(Status::kNetworkNotFound, MakeOptional(extpanid),
18741872
MakeOptional(static_cast<int32_t>(OT_ERROR_DETACHED)));
18751873
}
18761874
}

src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.h

+2-3
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ class GenericThreadStackManagerImpl_OpenThread
8787

8888
bool _IsThreadProvisioned(void);
8989
bool _IsThreadAttached(void);
90-
CHIP_ERROR _GetThreadProvision(ByteSpan & netInfo);
90+
CHIP_ERROR _GetThreadProvision(Thread::OperationalDataset & dataset);
9191
CHIP_ERROR _SetThreadProvision(ByteSpan netInfo);
9292
CHIP_ERROR _AttachToThreadNetwork(ByteSpan netInfo, NetworkCommissioning::Internal::WirelessDriver::ConnectCallback * callback);
9393
void _OnThreadAttachFinished(void);
@@ -149,8 +149,7 @@ class GenericThreadStackManagerImpl_OpenThread
149149
// ===== Private members for use by this class only.
150150

151151
otInstance * mOTInst;
152-
uint64_t mOverrunCount = 0;
153-
Thread::OperationalDataset mActiveDataset = {};
152+
uint64_t mOverrunCount = 0;
154153

155154
NetworkCommissioning::ThreadDriver::ScanCallback * mpScanCallback;
156155
NetworkCommissioning::Internal::WirelessDriver::ConnectCallback * mpConnectCallback;

0 commit comments

Comments
 (0)