Merge 903596d into 01fbb86

project-chip · May 12, 2021 · 5743028 · 5743028
2 parents 01fbb86 + 903596d
commit 5743028
Show file tree

Hide file tree

Showing 7 changed files with 32 additions and 24 deletions.
diff --git 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<uint32_t>(advertiseParameters.GetPeerId().GetFabricId() >> 32),
                     static_cast<uint32_t>(advertiseParameters.GetPeerId().GetFabricId()),
                     static_cast<uint32_t>(advertiseParameters.GetPeerId().GetNodeId() >> 32),

diff --git a/src/lib/mdns/platform/Mdns.h b/src/lib/mdns/platform/Mdns.h
@@ -35,10 +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 uint16_t kMdnsTextMaxSize        = 64;
+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; // <type>.<protocol>
+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
@@ -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[chip::Mdns::kMdnsTextMaxSize];
         size_t offset = static_cast<size_t>(snprintf(reinterpret_cast<char *>(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
@@ -1074,9 +1074,9 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::_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<ImplClass>::_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<ImplClass>::_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<ImplClass>::_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<ImplClass>::_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
@@ -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;
+        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
@@ -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);

diff --git 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<std::mutex> lock(mtx);
     std::condition_variable done;
     int retVal = EXIT_FAILURE;
 
     std::thread t([&mtx, &done, &retVal]() {
         {
-            std::lock_guard<std::mutex> localLock(mtx);
             nlTestSuite theSuite = { "CHIP DeviceLayer mdns tests", &sTests[0], nullptr, nullptr };
 
             nlTestRunner(&theSuite, nullptr);
             retVal = nlTestRunnerStats(&theSuite);
         }
-        done.notify_all();
+
+        {
+            std::lock_guard<std::mutex> 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<std::mutex> 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;
 }