diff --git a/src/include/platform/ThreadStackManager.h b/src/include/platform/ThreadStackManager.h index d253518ca584de..e31cb422cde7ea 100644 --- a/src/include/platform/ThreadStackManager.h +++ b/src/include/platform/ThreadStackManager.h @@ -93,9 +93,9 @@ class ThreadStackManager CHIP_ERROR SetThreadEnabled(bool val); #if CHIP_DEVICE_CONFIG_ENABLE_THREAD_SRP_CLIENT - CHIP_ERROR - AddSrpService(const char * aInstanceName, const char * aName, uint16_t aPort, chip::Mdns::TextEntry * aTxtEntries, - size_t aTxtEntiresSize, uint32_t aLeaseInterval, uint32_t aKeyLeaseInterval); + CHIP_ERROR AddSrpService(const char * aInstanceName, const char * aName, uint16_t aPort, + const Span & aSubTypes, const Span & aTxtEntries, + uint32_t aLeaseInterval, uint32_t aKeyLeaseInterval); CHIP_ERROR RemoveSrpService(const char * aInstanceName, const char * aName); CHIP_ERROR RemoveAllSrpServices(); CHIP_ERROR SetupSrpHost(const char * aHostName); @@ -241,10 +241,11 @@ inline CHIP_ERROR ThreadStackManager::SetThreadEnabled(bool val) #if CHIP_DEVICE_CONFIG_ENABLE_THREAD_SRP_CLIENT inline CHIP_ERROR ThreadStackManager::AddSrpService(const char * aInstanceName, const char * aName, uint16_t aPort, - chip::Mdns::TextEntry * aTxtEntries, size_t aTxtEntiresSize, - uint32_t aLeaseInterval = 0, uint32_t aKeyLeaseInterval = 0) + const Span & aSubTypes, + const Span & aTxtEntries, uint32_t aLeaseInterval = 0, + uint32_t aKeyLeaseInterval = 0) { - return static_cast(this)->_AddSrpService(aInstanceName, aName, aPort, aTxtEntries, aTxtEntiresSize, aLeaseInterval, + return static_cast(this)->_AddSrpService(aInstanceName, aName, aPort, aSubTypes, aTxtEntries, aLeaseInterval, aKeyLeaseInterval); } diff --git a/src/lib/mdns/Advertiser.h b/src/lib/mdns/Advertiser.h index 89380ac42d4f41..8ac8adc6c4dfd9 100644 --- a/src/lib/mdns/Advertiser.h +++ b/src/lib/mdns/Advertiser.h @@ -44,12 +44,16 @@ static constexpr size_t kKeyRotatingIdMaxLength = 100; static constexpr size_t kKeyPairingInstructionMaxLength = 128; static constexpr size_t kKeyPairingHintMaxLength = 10; -static constexpr size_t kSubTypeShortDiscriminatorMaxLength = 3; -static constexpr size_t kSubTypeLongDiscriminatorMaxLength = 4; -static constexpr size_t kSubTypeVendorMaxLength = 5; -static constexpr size_t kSubTypeDeviceTypeMaxLength = 5; -static constexpr size_t kSubTypeCommissioningModeMaxLength = 1; -static constexpr size_t kSubTypeAdditionalPairingMaxLength = 1; +static constexpr size_t kSubTypeShortDiscriminatorMaxLength = 4; // _S
+static constexpr size_t kSubTypeLongDiscriminatorMaxLength = 6; // _L +static constexpr size_t kSubTypeVendorMaxLength = 7; // _V +static constexpr size_t kSubTypeDeviceTypeMaxLength = 5; // _T +static constexpr size_t kSubTypeCommissioningModeMaxLength = 3; // _C +static constexpr size_t kSubTypeAdditionalPairingMaxLength = 3; // _A +static constexpr size_t kSubTypeMaxNumber = 6; +static constexpr size_t kSubTypeMaxLength = + std::max({ kSubTypeShortDiscriminatorMaxLength, kSubTypeLongDiscriminatorMaxLength, kSubTypeVendorMaxLength, + kSubTypeDeviceTypeMaxLength, kSubTypeCommissioningModeMaxLength, kSubTypeAdditionalPairingMaxLength }); enum class CommssionAdvertiseMode : uint8_t { diff --git a/src/lib/mdns/Discovery_ImplPlatform.cpp b/src/lib/mdns/Discovery_ImplPlatform.cpp index 3a500eaac9a213..7db89127dd3880 100644 --- a/src/lib/mdns/Discovery_ImplPlatform.cpp +++ b/src/lib/mdns/Discovery_ImplPlatform.cpp @@ -135,15 +135,15 @@ CHIP_ERROR DiscoveryImplPlatform::Advertise(const CommissionAdvertisingParameter // size of textEntries array should be count of Bufs above TextEntry textEntries[9]; size_t textEntrySize = 0; - // add underscore, character and newline to lengths for sub types (ex. _S) - char shortDiscriminatorSubtype[kSubTypeShortDiscriminatorMaxLength + 3]; - char longDiscriminatorSubtype[kSubTypeLongDiscriminatorMaxLength + 4]; - char vendorSubType[kSubTypeVendorMaxLength + 3]; - char commissioningModeSubType[kSubTypeCommissioningModeMaxLength + 3]; - char openWindowSubType[kSubTypeAdditionalPairingMaxLength + 3]; - char deviceTypeSubType[kSubTypeDeviceTypeMaxLength + 3]; + // add null-character to the subtypes + char shortDiscriminatorSubtype[kSubTypeShortDiscriminatorMaxLength + 1]; + char longDiscriminatorSubtype[kSubTypeLongDiscriminatorMaxLength + 1]; + char vendorSubType[kSubTypeVendorMaxLength + 1]; + char commissioningModeSubType[kSubTypeCommissioningModeMaxLength + 1]; + char openWindowSubType[kSubTypeAdditionalPairingMaxLength + 1]; + char deviceTypeSubType[kSubTypeDeviceTypeMaxLength + 1]; // size of subTypes array should be count of SubTypes above - const char * subTypes[6]; + const char * subTypes[kSubTypeMaxNumber]; size_t subTypeSize = 0; if (!mMdnsInitialized) diff --git a/src/lib/support/Span.h b/src/lib/support/Span.h index c4706a672f2930..4d51a510b9f1d0 100644 --- a/src/lib/support/Span.h +++ b/src/lib/support/Span.h @@ -34,7 +34,8 @@ template class Span { public: - using pointer = T *; + using pointer = T *; + using const_pointer = const T *; constexpr Span() : mDataBuf(nullptr), mDataLen(0) {} constexpr Span(pointer databuf, size_t datalen) : mDataBuf(databuf), mDataLen(datalen) {} @@ -45,6 +46,10 @@ class Span constexpr pointer data() const { return mDataBuf; } constexpr size_t size() const { return mDataLen; } constexpr bool empty() const { return size() == 0; } + constexpr const_pointer begin() const { return data(); } + constexpr const_pointer end() const { return data() + size(); } + constexpr pointer begin() { return data(); } + constexpr pointer end() { return data() + size(); } // Allow data_equal for spans that are over the same type up to const-ness. template , std::remove_const_t>::value>> diff --git a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp index 69346732331bca..37bbe8eaf07076 100644 --- a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp +++ b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp @@ -52,6 +52,8 @@ #include #include +#include + extern "C" void otSysProcessDrivers(otInstance * aInstance); #if CHIP_DEVICE_CONFIG_THREAD_ENABLE_CLI @@ -1075,12 +1077,14 @@ void GenericThreadStackManagerImpl_OpenThread::OnSrpClientStateChange template CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_AddSrpService(const char * aInstanceName, const char * aName, - uint16_t aPort, chip::Mdns::TextEntry * aTxtEntries, - size_t aTxtEntriesSize, uint32_t aLeaseInterval, - uint32_t aKeyLeaseInterval) + uint16_t aPort, + const Span & aSubTypes, + const Span & aTxtEntries, + uint32_t aLeaseInterval, uint32_t aKeyLeaseInterval) { CHIP_ERROR error = CHIP_NO_ERROR; typename SrpClient::Service * srpService = nullptr; + size_t entryId = 0; Impl()->LockThreadStack(); @@ -1096,9 +1100,7 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_AddSrpService(c // Remove possible existing entry if ((strcmp(service.mInstanceName, aInstanceName) == 0) && (strcmp(service.mName, aName) == 0)) { - VerifyOrExit(MapOpenThreadError(otSrpClientClearService(mOTInst, &(service.mService))) == CHIP_NO_ERROR, - error = MapOpenThreadError(OT_ERROR_FAILED)); - + SuccessOrExit(error = MapOpenThreadError(otSrpClientClearService(mOTInst, &service.mService))); // Clear memory immediately, as OnSrpClientNotification will not be called. memset(&service, 0, sizeof(service)); } @@ -1126,35 +1128,60 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_AddSrpService(c srpService->mService.mPort = aPort; - // Check if there are some optional text entries to add. - if (aTxtEntries && aTxtEntriesSize != 0) - { - VerifyOrExit(aTxtEntriesSize <= SrpClient::kMaxTxtEntriesNumber, error = CHIP_ERROR_INVALID_LIST_LENGTH); +#if OPENTHREAD_API_VERSION >= 132 + VerifyOrExit(aSubTypes.size() <= ArraySize(srpService->mSubTypeBuffers), error = CHIP_ERROR_BUFFER_TOO_SMALL); + entryId = 0; - srpService->mService.mNumTxtEntries = static_cast(aTxtEntriesSize); + for (const char * subType : aSubTypes) + { + auto & destBuffer = srpService->mSubTypeBuffers[entryId]; + VerifyOrExit(strlen(subType) < ArraySize(destBuffer), error = CHIP_ERROR_BUFFER_TOO_SMALL); + strcpy(destBuffer, subType); - for (uint8_t entryId = 0; entryId < aTxtEntriesSize; entryId++) - { - VerifyOrExit(aTxtEntries[entryId].mDataSize <= SrpClient::kMaxTxtValueSize, error = CHIP_ERROR_BUFFER_TOO_SMALL); - VerifyOrExit((strlen(aTxtEntries[entryId].mKey) + 1) <= SrpClient::kMaxTxtKeySize, error = CHIP_ERROR_BUFFER_TOO_SMALL); + srpService->mSubTypes[entryId++] = destBuffer; + } - srpService->mTxtEntries[entryId].mValueLength = static_cast(aTxtEntries[entryId].mDataSize); - memcpy(&(srpService->mTxtValueBuffers[entryId][0]), aTxtEntries[entryId].mData, aTxtEntries[entryId].mDataSize); - srpService->mTxtEntries[entryId].mValue = &(srpService->mTxtValueBuffers[entryId][0]); + srpService->mSubTypes[entryId] = nullptr; + srpService->mService.mSubTypeLabels = srpService->mSubTypes; +#endif - memcpy(&(srpService->mTxtKeyBuffers[entryId][0]), aTxtEntries[entryId].mKey, strlen(aTxtEntries[entryId].mKey) + 1); - srpService->mTxtEntries[entryId].mKey = &(srpService->mTxtKeyBuffers[entryId][0]); - } + // Initialize TXT entries + VerifyOrExit(aTxtEntries.size() <= ArraySize(srpService->mTxtEntries), error = CHIP_ERROR_BUFFER_TOO_SMALL); + entryId = 0; - srpService->mService.mTxtEntries = srpService->mTxtEntries; + for (const chip::Mdns::TextEntry & entry : aTxtEntries) + { + VerifyOrExit(strlen(entry.mKey) < SrpClient::kMaxTxtKeySize, error = CHIP_ERROR_BUFFER_TOO_SMALL); + strcpy(srpService->mTxtKeyBuffers[entryId], entry.mKey); + + VerifyOrExit(entry.mDataSize <= SrpClient::kMaxTxtValueSize, error = CHIP_ERROR_BUFFER_TOO_SMALL); + memcpy(srpService->mTxtValueBuffers[entryId], entry.mData, entry.mDataSize); + + using OtTxtValueLength = decltype(srpService->mTxtEntries[entryId].mValueLength); + static_assert(SrpClient::kMaxTxtValueSize <= std::numeric_limits::max(), + "DNS TXT value length may not fit in otDnsTxtEntry structure"); + srpService->mTxtEntries[entryId].mKey = srpService->mTxtKeyBuffers[entryId]; + srpService->mTxtEntries[entryId].mValue = srpService->mTxtValueBuffers[entryId]; + srpService->mTxtEntries[entryId].mValueLength = static_cast(entry.mDataSize); + ++entryId; } + using OtNumTxtEntries = decltype(srpService->mService.mNumTxtEntries); + static_assert(ArraySize(srpService->mTxtEntries) <= std::numeric_limits::max(), + "Number of DNS TXT entries may not fit in otSrpClientService structure"); + srpService->mService.mNumTxtEntries = static_cast(aTxtEntries.size()); + srpService->mService.mTxtEntries = srpService->mTxtEntries; + ChipLogProgress(DeviceLayer, "advertising srp service: %s.%s", srpService->mService.mInstanceName, srpService->mService.mName); error = MapOpenThreadError(otSrpClientAddService(mOTInst, &(srpService->mService))); exit: - Impl()->UnlockThreadStack(); + if (srpService != nullptr && error != CHIP_NO_ERROR) + { + memset(srpService, 0, sizeof(*srpService)); + } + Impl()->UnlockThreadStack(); return error; } diff --git a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.h b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.h index cd542ec69f4481..168a1ba295ec0f 100644 --- a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.h +++ b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.h @@ -97,9 +97,9 @@ class GenericThreadStackManagerImpl_OpenThread void _OnWoBLEAdvertisingStop(void); #if CHIP_DEVICE_CONFIG_ENABLE_THREAD_SRP_CLIENT - CHIP_ERROR - _AddSrpService(const char * aInstanceName, const char * aName, uint16_t aPort, chip::Mdns::TextEntry * aTxtEntries, - size_t aTxtEntiresSize, uint32_t aLeaseInterval, uint32_t aKeyLeaseInterval); + CHIP_ERROR _AddSrpService(const char * aInstanceName, const char * aName, uint16_t aPort, + const Span & aSubTypes, const Span & aTxtEntries, + uint32_t aLeaseInterval, uint32_t aKeyLeaseInterval); CHIP_ERROR _RemoveSrpService(const char * aInstanceName, const char * aName); CHIP_ERROR _RemoveAllSrpServices(); CHIP_ERROR _SetupSrpHost(const char * aHostName); @@ -153,6 +153,11 @@ class GenericThreadStackManagerImpl_OpenThread otSrpClientService mService; char mInstanceName[kMaxInstanceNameSize + 1]; char mName[kMaxNameSize + 1]; +#if OPENTHREAD_API_VERSION >= 132 + // TODO: use fixed buffer allocator to reduce the memory footprint from N*M to sum(M_i) + char mSubTypeBuffers[chip::Mdns::kSubTypeMaxNumber][chip::Mdns::kSubTypeMaxLength + 1]; + const char * mSubTypes[chip::Mdns::kSubTypeMaxNumber + 1]; // extra entry for nullptr at the end +#endif otDnsTxtEntry mTxtEntries[kMaxTxtEntriesNumber]; uint8_t mTxtValueBuffers[kMaxTxtEntriesNumber][kMaxTxtValueSize]; char mTxtKeyBuffers[kMaxTxtEntriesNumber][kMaxTxtKeySize]; diff --git a/src/platform/OpenThread/MdnsImpl.cpp b/src/platform/OpenThread/MdnsImpl.cpp index d581c89b443563..8dd8d2c389cf87 100644 --- a/src/platform/OpenThread/MdnsImpl.cpp +++ b/src/platform/OpenThread/MdnsImpl.cpp @@ -40,23 +40,18 @@ const char * GetProtocolString(MdnsServiceProtocol protocol) CHIP_ERROR ChipMdnsPublishService(const MdnsService * service) { #if CHIP_DEVICE_CONFIG_ENABLE_THREAD_SRP_CLIENT - CHIP_ERROR result = CHIP_NO_ERROR; - - VerifyOrExit(service, result = CHIP_ERROR_INVALID_ARGUMENT); + ReturnErrorCodeIf(service == nullptr, CHIP_ERROR_INVALID_ARGUMENT); if (strcmp(service->mHostName, "") != 0) { - CHIP_ERROR hostNameErr = ThreadStackMgr().SetupSrpHost(service->mHostName); - VerifyOrExit(hostNameErr == CHIP_NO_ERROR, result = hostNameErr); + ReturnErrorOnFailure(ThreadStackMgr().SetupSrpHost(service->mHostName)); } char serviceType[chip::Mdns::kMdnsTypeAndProtocolMaxSize + 1]; snprintf(serviceType, sizeof(serviceType), "%s.%s", service->mType, GetProtocolString(service->mProtocol)); - result = - ThreadStackMgr().AddSrpService(service->mName, serviceType, service->mPort, service->mTextEntries, service->mTextEntrySize); - -exit: - return result; + Span subTypes(service->mSubTypes, service->mSubTypeSize); + Span textEntries(service->mTextEntries, service->mTextEntrySize); + return ThreadStackMgr().AddSrpService(service->mName, serviceType, service->mPort, subTypes, textEntries); #else return CHIP_ERROR_NOT_IMPLEMENTED; #endif // CHIP_DEVICE_CONFIG_ENABLE_THREAD_SRP_CLIENT