From 815305815aa2c607aebb44d2093d2d606e71a3d7 Mon Sep 17 00:00:00 2001 From: kangping Date: Wed, 12 May 2021 19:49:31 +0800 Subject: [PATCH 1/4] tmp commit for testing --- src/app/server/Mdns.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/server/Mdns.cpp b/src/app/server/Mdns.cpp index 5af680afaf9726..8827d28a134c59 100644 --- a/src/app/server/Mdns.cpp +++ b/src/app/server/Mdns.cpp @@ -107,7 +107,7 @@ CHIP_ERROR AdvertiseOperational() auto & mdnsAdvertiser = chip::Mdns::ServiceAdvertiser::Instance(); - ChipLogProgress(Discovery, "Advertise operational node 0x%08" PRIx32 "%08" PRIx32 "-0x%08" PRIx32 "%08" PRIx32, + ChipLogProgress(Discovery, "Advertise operational node %08" PRIx32 "%08" PRIx32 "-%08" PRIx32 "%08" PRIx32, static_cast(advertiseParameters.GetPeerId().GetFabricId() >> 32), static_cast(advertiseParameters.GetPeerId().GetFabricId()), static_cast(advertiseParameters.GetPeerId().GetNodeId() >> 32), From b02077e0781303f73e760ec38186e386b95d730e Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Wed, 12 May 2021 10:05:27 -0400 Subject: [PATCH 2/4] Fix locking scope for MDNS test. std::condition_variable is supposed to be able to hold the lock mutex when exiting after wait_for. If the lock on the mutex is held throughout the 'RunMainLoop', the condition variable can never exit. Updated the scope of the locks. --- src/platform/tests/TestMdns.cpp | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/platform/tests/TestMdns.cpp b/src/platform/tests/TestMdns.cpp index e1d77e9d501335..27990abe336391 100644 --- a/src/platform/tests/TestMdns.cpp +++ b/src/platform/tests/TestMdns.cpp @@ -95,25 +95,30 @@ static const nlTest sTests[] = { NL_TEST_DEF("Test Mdns::PubSub", TestMdnsPubSub int TestMdns() { std::mutex mtx; - std::unique_lock lock(mtx); std::condition_variable done; int retVal = EXIT_FAILURE; std::thread t([&mtx, &done, &retVal]() { { - std::lock_guard localLock(mtx); nlTestSuite theSuite = { "CHIP DeviceLayer mdns tests", &sTests[0], nullptr, nullptr }; nlTestRunner(&theSuite, nullptr); retVal = nlTestRunnerStats(&theSuite); } - done.notify_all(); + + { + std::lock_guard localLock(mtx); + done.notify_all(); + } }); - if (done.wait_for(lock, std::chrono::seconds(5)) == std::cv_status::timeout) { - fprintf(stderr, "mDNS test timeout, is avahi daemon running?"); - retVal = EXIT_FAILURE; + std::unique_lock lock(mtx); + if (done.wait_for(lock, std::chrono::seconds(5)) == std::cv_status::timeout) + { + fprintf(stderr, "mDNS test timeout, is avahi daemon running?\n"); + retVal = EXIT_FAILURE; + } } return retVal; } From 012722376dde2cbf5c544f2faa67eb7b2682ebc4 Mon Sep 17 00:00:00 2001 From: kangping Date: Mon, 10 May 2021 21:48:43 +0800 Subject: [PATCH 3/4] [dns-sd] fix SRP service name length --- src/lib/mdns/platform/Mdns.h | 8 +++++--- src/platform/Linux/MdnsImpl.cpp | 2 +- .../GenericThreadStackManagerImpl_OpenThread.cpp | 12 +++++++----- .../GenericThreadStackManagerImpl_OpenThread.h | 10 +++++----- src/platform/OpenThread/MdnsImpl.cpp | 4 ++-- 5 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/lib/mdns/platform/Mdns.h b/src/lib/mdns/platform/Mdns.h index a85aba2b7c53cd..778fcb19f643ee 100644 --- a/src/lib/mdns/platform/Mdns.h +++ b/src/lib/mdns/platform/Mdns.h @@ -35,9 +35,11 @@ namespace chip { namespace Mdns { -static constexpr uint8_t kMdnsNameMaxSize = 33; // [Node]-[Fabric] ID in hex - 16+1+16 -static constexpr uint8_t kMdnsProtocolTextMaxSize = 4 + 1; // "_tcp" or "_udp" -static constexpr uint8_t kMdnsTypeMaxSize = 10; // "_chip", "_chipc" or "_chipd" +static constexpr uint8_t kMdnsNameMaxSize = 33; // [Node]-[Fabric] ID in hex - 16+1+16 +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; // . +static constexpr uint8_t kMdnsTextKeyMaxSize = 9; static constexpr uint16_t kMdnsTextMaxSize = 64; enum class MdnsServiceProtocol : uint8_t diff --git a/src/platform/Linux/MdnsImpl.cpp b/src/platform/Linux/MdnsImpl.cpp index 728267641cc0b5..474238f62b0d0f 100644 --- a/src/platform/Linux/MdnsImpl.cpp +++ b/src/platform/Linux/MdnsImpl.cpp @@ -84,7 +84,7 @@ CHIP_ERROR MakeAvahiStringListFromTextEntries(TextEntry * entries, size_t size, for (size_t i = 0; i < size; i++) { - uint8_t buf[kMdnsTypeMaxSize]; + uint8_t buf[kMdnsTextKeyMaxSize + 1 + 1]; size_t offset = static_cast(snprintf(reinterpret_cast(buf), sizeof(buf), "%s=", entries[i].mKey)); if (offset + entries[i].mDataSize > sizeof(buf)) diff --git a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp index 6635494ca67bee..402b182e79c38b 100644 --- a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp +++ b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp @@ -1074,9 +1074,9 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_AddSrpService(c Impl()->LockThreadStack(); VerifyOrExit(aInstanceName, error = CHIP_ERROR_INVALID_ARGUMENT); - VerifyOrExit(strlen(aInstanceName) < SrpClient::kMaxInstanceNameSize, error = CHIP_ERROR_INVALID_STRING_LENGTH); + VerifyOrExit(strlen(aInstanceName) <= SrpClient::kMaxInstanceNameSize, error = CHIP_ERROR_INVALID_STRING_LENGTH); VerifyOrExit(aName, error = CHIP_ERROR_INVALID_ARGUMENT); - VerifyOrExit(strlen(aName) < SrpClient::kMaxNameSize, error = CHIP_ERROR_INVALID_STRING_LENGTH); + VerifyOrExit(strlen(aName) <= SrpClient::kMaxNameSize, error = CHIP_ERROR_INVALID_STRING_LENGTH); // Check if service with desired instance name already exists and try to find empty slot in array for new service for (typename SrpClient::Service & service : mSrpClient.mServices) @@ -1130,6 +1130,7 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_AddSrpService(c 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: @@ -1147,9 +1148,9 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_RemoveSrpServic Impl()->LockThreadStack(); VerifyOrExit(aInstanceName, error = CHIP_ERROR_INVALID_ARGUMENT); - VerifyOrExit(strlen(aInstanceName) < SrpClient::kMaxInstanceNameSize, error = CHIP_ERROR_INVALID_STRING_LENGTH); + VerifyOrExit(strlen(aInstanceName) <= SrpClient::kMaxInstanceNameSize, error = CHIP_ERROR_INVALID_STRING_LENGTH); VerifyOrExit(aName, error = CHIP_ERROR_INVALID_ARGUMENT); - VerifyOrExit(strlen(aName) < SrpClient::kMaxNameSize, error = CHIP_ERROR_INVALID_STRING_LENGTH); + VerifyOrExit(strlen(aName) <= SrpClient::kMaxNameSize, error = CHIP_ERROR_INVALID_STRING_LENGTH); // Check if service to remove exists. for (typename SrpClient::Service & service : mSrpClient.mServices) @@ -1163,6 +1164,7 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_RemoveSrpServic VerifyOrExit(srpService, error = MapOpenThreadError(OT_ERROR_NOT_FOUND)); + ChipLogProgress(DeviceLayer, "removing srp service: %s.%s", aInstanceName, aName); error = MapOpenThreadError(otSrpClientRemoveService(mOTInst, &(srpService->mService))); exit: @@ -1199,7 +1201,7 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_SetupSrpHost(co Impl()->LockThreadStack(); VerifyOrExit(aHostName, error = CHIP_ERROR_INVALID_ARGUMENT); - VerifyOrExit(strlen(aHostName) < SrpClient::kMaxHostNameSize, error = CHIP_ERROR_INVALID_STRING_LENGTH); + VerifyOrExit(strlen(aHostName) <= SrpClient::kMaxHostNameSize, error = CHIP_ERROR_INVALID_STRING_LENGTH); // Avoid adding the same host name multiple times if (strcmp(mSrpClient.mHostName, aHostName) != 0) diff --git a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.h b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.h index 71ca8d9ff65540..39ac2f7ceaea19 100644 --- a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.h +++ b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.h @@ -121,8 +121,8 @@ class GenericThreadStackManagerImpl_OpenThread { static constexpr uint8_t kMaxServicesNumber = CHIP_DEVICE_CONFIG_THREAD_SRP_MAX_SERVICES; static constexpr uint8_t kMaxInstanceNameSize = chip::Mdns::kMdnsNameMaxSize; - static constexpr uint8_t kMaxNameSize = chip::Mdns::kMdnsTypeMaxSize + chip::Mdns::kMdnsProtocolTextMaxSize + 1; - static constexpr uint8_t kMaxHostNameSize = 32; + static constexpr uint8_t kMaxNameSize = chip::Mdns::kMdnsTypeAndProtocolMaxSize + 1; + static constexpr uint8_t kMaxHostNameSize = 16; // Thread only supports operational discovery static constexpr uint8_t kMaxTxtEntriesNumber = chip::Mdns::OperationalAdvertisingParameters::kNumAdvertisingTxtEntries; static constexpr uint8_t kMaxTxtValueSize = chip::Mdns::OperationalAdvertisingParameters::kTxtMaxValueSize; @@ -131,14 +131,14 @@ class GenericThreadStackManagerImpl_OpenThread struct Service { otSrpClientService mService; - char mInstanceName[kMaxInstanceNameSize]; - char mName[kMaxNameSize]; + char mInstanceName[kMaxInstanceNameSize + 1]; + char mName[kMaxNameSize + 1]; otDnsTxtEntry mTxtEntries[kMaxTxtEntriesNumber]; uint8_t mTxtValueBuffers[kMaxTxtEntriesNumber][kMaxTxtValueSize]; char mTxtKeyBuffers[kMaxTxtEntriesNumber][kMaxTxtKeySize]; }; - char mHostName[kMaxHostNameSize]; + char mHostName[kMaxHostNameSize + 1]; otIp6Address mHostAddress; Service mServices[kMaxServicesNumber]; }; diff --git a/src/platform/OpenThread/MdnsImpl.cpp b/src/platform/OpenThread/MdnsImpl.cpp index 8f920f3924c890..8493d64d087594 100644 --- a/src/platform/OpenThread/MdnsImpl.cpp +++ b/src/platform/OpenThread/MdnsImpl.cpp @@ -48,7 +48,7 @@ CHIP_ERROR ChipMdnsPublishService(const MdnsService * service) VerifyOrExit(service, result = CHIP_ERROR_INVALID_ARGUMENT); - char serviceType[kMdnsTypeMaxSize + kMdnsProtocolTextMaxSize + 1]; + char serviceType[chip::Mdns::kMdnsTypeAndProtocolMaxSize + 1]; snprintf(serviceType, sizeof(serviceType), "%s.%s", service->mType, GetProtocolString(service->mProtocol)); // Try to remove service before adding it, as SRP doesn't allow to update existing services. @@ -74,7 +74,7 @@ CHIP_ERROR ChipMdnsStopPublishService(const MdnsService * service) if (service == nullptr) return CHIP_ERROR_INVALID_ARGUMENT; - char serviceType[kMdnsTypeMaxSize + kMdnsProtocolTextMaxSize + 1]; + char serviceType[chip::Mdns::kMdnsTypeAndProtocolMaxSize + 1]; snprintf(serviceType, sizeof(serviceType), "%s.%s", service->mType, GetProtocolString(service->mProtocol)); return ThreadStackMgr().RemoveSrpService(service->mName, serviceType); From 903596df3bec1c34275bf48a96bc2a819fd4e17f Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Tue, 11 May 2021 05:09:43 +0000 Subject: [PATCH 4/4] Restyled by clang-format --- src/lib/mdns/platform/Mdns.h | 9 ++++----- src/platform/Linux/MdnsImpl.cpp | 2 +- .../GenericThreadStackManagerImpl_OpenThread.h | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/lib/mdns/platform/Mdns.h b/src/lib/mdns/platform/Mdns.h index 778fcb19f643ee..cdbd08bace9c34 100644 --- a/src/lib/mdns/platform/Mdns.h +++ b/src/lib/mdns/platform/Mdns.h @@ -35,12 +35,11 @@ namespace chip { namespace Mdns { -static constexpr uint8_t kMdnsNameMaxSize = 33; // [Node]-[Fabric] ID in hex - 16+1+16 -static constexpr uint8_t kMdnsProtocolTextMaxSize = 4; // "_tcp" or "_udp" -static constexpr uint8_t kMdnsTypeMaxSize = 6; // "_chip", "_chipc" or "_chipd" +static constexpr uint8_t kMdnsNameMaxSize = 33; // [Node]-[Fabric] ID in hex - 16+1+16 +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; // . -static constexpr uint8_t kMdnsTextKeyMaxSize = 9; -static constexpr uint16_t kMdnsTextMaxSize = 64; +static constexpr uint16_t kMdnsTextMaxSize = 64; enum class MdnsServiceProtocol : uint8_t { diff --git a/src/platform/Linux/MdnsImpl.cpp b/src/platform/Linux/MdnsImpl.cpp index 474238f62b0d0f..982fc5b76ed112 100644 --- a/src/platform/Linux/MdnsImpl.cpp +++ b/src/platform/Linux/MdnsImpl.cpp @@ -84,7 +84,7 @@ CHIP_ERROR MakeAvahiStringListFromTextEntries(TextEntry * entries, size_t size, for (size_t i = 0; i < size; i++) { - uint8_t buf[kMdnsTextKeyMaxSize + 1 + 1]; + uint8_t buf[chip::Mdns::kMdnsTextMaxSize]; size_t offset = static_cast(snprintf(reinterpret_cast(buf), sizeof(buf), "%s=", entries[i].mKey)); if (offset + entries[i].mDataSize > sizeof(buf)) diff --git a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.h b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.h index 39ac2f7ceaea19..c5e6ecaca1c5fb 100644 --- a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.h +++ b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.h @@ -121,7 +121,7 @@ class GenericThreadStackManagerImpl_OpenThread { static constexpr uint8_t kMaxServicesNumber = CHIP_DEVICE_CONFIG_THREAD_SRP_MAX_SERVICES; static constexpr uint8_t kMaxInstanceNameSize = chip::Mdns::kMdnsNameMaxSize; - static constexpr uint8_t kMaxNameSize = chip::Mdns::kMdnsTypeAndProtocolMaxSize + 1; + static constexpr uint8_t kMaxNameSize = chip::Mdns::kMdnsTypeAndProtocolMaxSize; static constexpr uint8_t kMaxHostNameSize = 16; // Thread only supports operational discovery static constexpr uint8_t kMaxTxtEntriesNumber = chip::Mdns::OperationalAdvertisingParameters::kNumAdvertisingTxtEntries;