From 94308042527f61587e7a89d993c1db47cb1e425b Mon Sep 17 00:00:00 2001 From: C Freeman Date: Sun, 13 Jun 2021 05:21:10 -0400 Subject: [PATCH] Mdns: move Hostname into MdnsService struct (#7520) * Move host name into MdnsService * Fix MdnsTest variable used uninitialized. * Add back error checking. --- src/lib/mdns/Discovery_ImplPlatform.cpp | 32 +++++++------------ src/lib/mdns/Discovery_ImplPlatform.h | 1 - src/lib/mdns/platform/Mdns.h | 14 +++----- src/platform/Darwin/MdnsImpl.cpp | 12 +++---- src/platform/Darwin/MdnsImpl.h | 5 +-- src/platform/ESP32/MdnsImpl.cpp | 10 +++--- src/platform/Linux/MdnsImpl.cpp | 9 +++--- ...GenericThreadStackManagerImpl_OpenThread.h | 2 +- src/platform/OpenThread/MdnsImpl.cpp | 10 +++--- src/platform/tests/TestMdns.cpp | 1 + 10 files changed, 40 insertions(+), 56 deletions(-) diff --git a/src/lib/mdns/Discovery_ImplPlatform.cpp b/src/lib/mdns/Discovery_ImplPlatform.cpp index 706935f825ce2f..1e597328ca05a8 100644 --- a/src/lib/mdns/Discovery_ImplPlatform.cpp +++ b/src/lib/mdns/Discovery_ImplPlatform.cpp @@ -102,24 +102,6 @@ void DiscoveryImplPlatform::HandleMdnsError(void * context, CHIP_ERROR error) } } -CHIP_ERROR DiscoveryImplPlatform::SetupHostname(chip::ByteSpan macOrEui64) -{ - char nameBuffer[17]; - CHIP_ERROR error = MakeHostName(nameBuffer, sizeof(nameBuffer), macOrEui64); - if (error != CHIP_NO_ERROR) - { - ChipLogError(Discovery, "Failed to create mdns hostname: %s", ErrorStr(error)); - return error; - } - error = ChipMdnsSetHostname(nameBuffer); - if (error != CHIP_NO_ERROR) - { - ChipLogError(Discovery, "Failed to setup mdns hostname: %s", ErrorStr(error)); - return error; - } - return CHIP_NO_ERROR; -} - CHIP_ERROR DiscoveryImplPlatform::Advertise(const CommissionAdvertisingParameters & params) { CHIP_ERROR error = CHIP_NO_ERROR; @@ -153,7 +135,12 @@ CHIP_ERROR DiscoveryImplPlatform::Advertise(const CommissionAdvertisingParameter return CHIP_ERROR_INCORRECT_STATE; } - ReturnErrorOnFailure(SetupHostname(params.GetMac())); + error = MakeHostName(service.mHostName, sizeof(service.mHostName), params.GetMac()); + if (error != CHIP_NO_ERROR) + { + ChipLogError(Discovery, "Failed to create mdns hostname: %s", ErrorStr(error)); + return error; + } snprintf(service.mName, sizeof(service.mName), "%08" PRIX32 "%08" PRIX32, static_cast(mCommissionInstanceName >> 32), static_cast(mCommissionInstanceName)); @@ -386,7 +373,12 @@ CHIP_ERROR DiscoveryImplPlatform::Advertise(const OperationalAdvertisingParamete mrpRetryIntervalEntries[textEntrySize++] = { "CRA", reinterpret_cast(mrpRetryIntervalActiveBuf), strlen(mrpRetryIntervalActiveBuf) }; - ReturnErrorOnFailure(SetupHostname(params.GetMac())); + error = MakeHostName(service.mHostName, sizeof(service.mHostName), params.GetMac()); + if (error != CHIP_NO_ERROR) + { + ChipLogError(Discovery, "Failed to create mdns hostname: %s", ErrorStr(error)); + return error; + } ReturnErrorOnFailure(MakeInstanceName(service.mName, sizeof(service.mName), params.GetPeerId())); strncpy(service.mType, kOperationalServiceName, sizeof(service.mType)); service.mProtocol = MdnsServiceProtocol::kMdnsProtocolTcp; diff --git a/src/lib/mdns/Discovery_ImplPlatform.h b/src/lib/mdns/Discovery_ImplPlatform.h index 8b1835c199dd85..46a354aaa2eeb2 100644 --- a/src/lib/mdns/Discovery_ImplPlatform.h +++ b/src/lib/mdns/Discovery_ImplPlatform.h @@ -71,7 +71,6 @@ class DiscoveryImplPlatform : public ServiceAdvertiser, public Resolver static void HandleMdnsInit(void * context, CHIP_ERROR initError); static void HandleMdnsError(void * context, CHIP_ERROR initError); static CHIP_ERROR GenerateRotatingDeviceId(char rotatingDeviceIdHexBuffer[], size_t & rotatingDeviceIdHexBufferSize); - CHIP_ERROR SetupHostname(chip::ByteSpan macOrEui64); #ifdef DETAIL_LOGGING static void PrintEntries(const MdnsService * service); #endif diff --git a/src/lib/mdns/platform/Mdns.h b/src/lib/mdns/platform/Mdns.h index cdbd08bace9c34..f85d06324f5d8a 100644 --- a/src/lib/mdns/platform/Mdns.h +++ b/src/lib/mdns/platform/Mdns.h @@ -35,7 +35,8 @@ namespace chip { namespace Mdns { -static constexpr uint8_t kMdnsNameMaxSize = 33; // [Node]-[Fabric] ID in hex - 16+1+16 +static constexpr uint8_t kMdnsInstanceNameMaxSize = 33; // [Node]-[Fabric] ID in hex - 16+1+16 +static constexpr uint8_t kMdnsHostNameMaxSize = 16; // 64-bits in hex. static constexpr uint8_t kMdnsProtocolTextMaxSize = 4; // "_tcp" or "_udp" static constexpr uint8_t kMdnsTypeMaxSize = 6; // "_chip", "_chipc" or "_chipd" static constexpr uint8_t kMdnsTypeAndProtocolMaxSize = kMdnsTypeMaxSize + kMdnsProtocolTextMaxSize + 1; // . @@ -57,7 +58,8 @@ struct TextEntry struct MdnsService { - char mName[kMdnsNameMaxSize + 1]; + char mName[kMdnsInstanceNameMaxSize + 1]; + char mHostName[kMdnsHostNameMaxSize + 1] = ""; char mType[kMdnsTypeMaxSize + 1]; MdnsServiceProtocol mProtocol; Inet::IPAddressType mAddressType; @@ -112,14 +114,6 @@ using MdnsAsyncReturnCallback = void (*)(void * context, CHIP_ERROR error); */ CHIP_ERROR ChipMdnsInit(MdnsAsyncReturnCallback initCallback, MdnsAsyncReturnCallback errorCallback, void * context); -/** - * This function sets the host name for services. - * - * @param[in] hostname The hostname. - * - */ -CHIP_ERROR ChipMdnsSetHostname(const char * hostname); - /** * This function publishes an service via mDNS. * diff --git a/src/platform/Darwin/MdnsImpl.cpp b/src/platform/Darwin/MdnsImpl.cpp index e434df2d830f25..e3e914e4714bf8 100644 --- a/src/platform/Darwin/MdnsImpl.cpp +++ b/src/platform/Darwin/MdnsImpl.cpp @@ -319,7 +319,7 @@ void OnBrowseAdd(BrowseContext * context, const char * name, const char * type, service.mProtocol = context->protocol; strncpy(service.mName, name, sizeof(service.mName)); - service.mName[kMdnsNameMaxSize] = 0; + service.mName[kMdnsInstanceNameMaxSize] = 0; strncpy(service.mType, regtype, sizeof(service.mType)); service.mType[kMdnsTypeMaxSize] = 0; @@ -483,17 +483,15 @@ CHIP_ERROR ChipMdnsInit(MdnsAsyncReturnCallback successCallback, MdnsAsyncReturn return CHIP_NO_ERROR; } -CHIP_ERROR ChipMdnsSetHostname(const char * hostname) -{ - MdnsContexts::GetInstance().SetHostname(hostname); - return CHIP_NO_ERROR; -} - CHIP_ERROR ChipMdnsPublishService(const MdnsService * service) { VerifyOrReturnError(service != nullptr, CHIP_ERROR_INVALID_ARGUMENT); VerifyOrReturnError(IsSupportedProtocol(service->mProtocol), CHIP_ERROR_INVALID_ARGUMENT); + if (strcmp(service->mHostName, "") != 0) + { + MdnsContexts::GetInstance().SetHostname(service->mHostName); + } std::string regtype = GetFullTypeWithSubTypes(service->mType, service->mProtocol, service->mSubTypes, service->mSubTypeSize); uint32_t interfaceId = GetInterfaceId(service->mInterface); diff --git a/src/platform/Darwin/MdnsImpl.h b/src/platform/Darwin/MdnsImpl.h index ffd388ff373f0f..ac00b8db5109e3 100644 --- a/src/platform/Darwin/MdnsImpl.h +++ b/src/platform/Darwin/MdnsImpl.h @@ -71,7 +71,8 @@ struct BrowseContext : public GenericContext struct ResolveContext : public GenericContext { MdnsResolveCallback callback; - char name[kMdnsNameMaxSize + 1]; + + char name[kMdnsInstanceNameMaxSize + 1]; chip::Inet::IPAddressType addressType; ResolveContext(void * cbContext, MdnsResolveCallback cb, const char * cbContextName, chip::Inet::IPAddressType cbAddressType) @@ -88,7 +89,7 @@ struct GetAddrInfoContext : public GenericContext { MdnsResolveCallback callback; std::vector textEntries; - char name[kMdnsNameMaxSize + 1]; + char name[kMdnsInstanceNameMaxSize + 1]; uint32_t interfaceId; uint16_t port; diff --git a/src/platform/ESP32/MdnsImpl.cpp b/src/platform/ESP32/MdnsImpl.cpp index 38bcfaea812959..edc9ea1faa3f68 100644 --- a/src/platform/ESP32/MdnsImpl.cpp +++ b/src/platform/ESP32/MdnsImpl.cpp @@ -60,17 +60,17 @@ static const char * GetProtocolString(MdnsServiceProtocol protocol) return protocol == MdnsServiceProtocol::kMdnsProtocolTcp ? "_tcp" : "_udp"; } -CHIP_ERROR ChipMdnsSetHostname(const char * hostname) -{ - return mdns_hostname_set(hostname) == ESP_OK ? CHIP_NO_ERROR : CHIP_ERROR_INTERNAL; -} - CHIP_ERROR ChipMdnsPublishService(const MdnsService * service) { CHIP_ERROR error = CHIP_NO_ERROR; mdns_txt_item_t * items = nullptr; esp_err_t espError; + if (strcmp(service->mHostName, "") != 0) + { + VerifyOrExit(mdns_hostname_set(service->mHostName) == ESP_OK, error = CHIP_ERROR_INTERNAL); + } + VerifyOrExit(service->mTextEntrySize <= UINT8_MAX, error = CHIP_ERROR_INVALID_ARGUMENT); if (service->mTextEntries) { diff --git a/src/platform/Linux/MdnsImpl.cpp b/src/platform/Linux/MdnsImpl.cpp index 7f0032efa0af6e..96bb3d03be6b69 100644 --- a/src/platform/Linux/MdnsImpl.cpp +++ b/src/platform/Linux/MdnsImpl.cpp @@ -761,13 +761,12 @@ CHIP_ERROR ChipMdnsInit(MdnsAsyncReturnCallback initCallback, MdnsAsyncReturnCal return MdnsAvahi::GetInstance().Init(initCallback, errorCallback, context); } -CHIP_ERROR ChipMdnsSetHostname(const char * hostname) -{ - return MdnsAvahi::GetInstance().SetHostname(hostname); -} - CHIP_ERROR ChipMdnsPublishService(const MdnsService * service) { + if (strcmp(service->mHostName, "") != 0) + { + ReturnErrorOnFailure(MdnsAvahi::GetInstance().SetHostname(service->mHostName)); + } return MdnsAvahi::GetInstance().PublishService(*service); } diff --git a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.h b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.h index c5e6ecaca1c5fb..34b3911a30522a 100644 --- a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.h +++ b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.h @@ -120,7 +120,7 @@ class GenericThreadStackManagerImpl_OpenThread struct SrpClient { static constexpr uint8_t kMaxServicesNumber = CHIP_DEVICE_CONFIG_THREAD_SRP_MAX_SERVICES; - static constexpr uint8_t kMaxInstanceNameSize = chip::Mdns::kMdnsNameMaxSize; + static constexpr uint8_t kMaxInstanceNameSize = chip::Mdns::kMdnsInstanceNameMaxSize; static constexpr uint8_t kMaxNameSize = chip::Mdns::kMdnsTypeAndProtocolMaxSize; static constexpr uint8_t kMaxHostNameSize = 16; // Thread only supports operational discovery diff --git a/src/platform/OpenThread/MdnsImpl.cpp b/src/platform/OpenThread/MdnsImpl.cpp index 8493d64d087594..202b27dcebe17a 100644 --- a/src/platform/OpenThread/MdnsImpl.cpp +++ b/src/platform/OpenThread/MdnsImpl.cpp @@ -32,11 +32,6 @@ CHIP_ERROR ChipMdnsInit(MdnsAsyncReturnCallback initCallback, MdnsAsyncReturnCal return CHIP_NO_ERROR; } -CHIP_ERROR ChipMdnsSetHostname(const char * hostname) -{ - return ThreadStackMgr().SetupSrpHost(hostname); -} - const char * GetProtocolString(MdnsServiceProtocol protocol) { return protocol == MdnsServiceProtocol::kMdnsProtocolUdp ? "_udp" : "_tcp"; @@ -47,6 +42,11 @@ CHIP_ERROR ChipMdnsPublishService(const MdnsService * service) CHIP_ERROR result = CHIP_NO_ERROR; VerifyOrExit(service, result = CHIP_ERROR_INVALID_ARGUMENT); + if (strcmp(service->mHostName, "") != 0) + { + CHIP_ERROR hostNameErr = ThreadStackMgr().SetupSrpHost(service->mHostName); + VerifyOrExit(hostNameErr == CHIP_NO_ERROR, result = hostNameErr); + } char serviceType[chip::Mdns::kMdnsTypeAndProtocolMaxSize + 1]; snprintf(serviceType, sizeof(serviceType), "%s.%s", service->mType, GetProtocolString(service->mProtocol)); diff --git a/src/platform/tests/TestMdns.cpp b/src/platform/tests/TestMdns.cpp index 906ffd7170b070..d9d70157fc8aa9 100644 --- a/src/platform/tests/TestMdns.cpp +++ b/src/platform/tests/TestMdns.cpp @@ -57,6 +57,7 @@ static void InitCallback(void * context, CHIP_ERROR error) service.mPort = 80; strcpy(service.mName, "test"); strcpy(service.mType, "_mock"); + service.mAddressType = chip::Inet::kIPAddressType_Any; service.mProtocol = MdnsServiceProtocol::kMdnsProtocolTcp; entry.mKey = key; entry.mData = reinterpret_cast(val);