Stop passing our addresses from resolve in two different places. (#18708

)

We used to pass some in the DnssdService and some in a Span.  Now just
pass them all in the Span.

Fixes #15279

src/include/platform/ThreadStackManager.h

@@ -67,7 +67,7 @@ class GenericThreadStackManagerImpl_FreeRTOS;
 
 #if CHIP_DEVICE_CONFIG_ENABLE_THREAD_DNS_CLIENT
 // Declaration of callback types corresponding to DnssdResolveCallback and DnssdBrowseCallback to avoid circular including.
-using DnsResolveCallback = void (*)(void * context, chip::Dnssd::DnssdService * result, const Span<Inet::IPAddress> & extraIPs,
+using DnsResolveCallback = void (*)(void * context, chip::Dnssd::DnssdService * result, const Span<Inet::IPAddress> & addresses,
                                     CHIP_ERROR error);
 using DnsBrowseCallback  = void (*)(void * context, chip::Dnssd::DnssdService * services, size_t servicesSize, CHIP_ERROR error);
 #endif // CHIP_DEVICE_CONFIG_ENABLE_THREAD_DNS_CLIENT

src/lib/dnssd/Discovery_ImplPlatform.cpp

@@ -37,7 +37,7 @@ namespace Dnssd {
 
 namespace {
 
-static void HandleNodeResolve(void * context, DnssdService * result, const Span<Inet::IPAddress> & extraIPs, CHIP_ERROR error)
+static void HandleNodeResolve(void * context, DnssdService * result, const Span<Inet::IPAddress> & addresses, CHIP_ERROR error)
 {
     ResolverDelegateProxy * proxy = static_cast<ResolverDelegateProxy *>(context);
 
@@ -52,15 +52,10 @@ static void HandleNodeResolve(void * context, DnssdService * result, const Span<
     Platform::CopyString(nodeData.resolutionData.hostName, result->mHostName);
     Platform::CopyString(nodeData.commissionData.instanceName, result->mName);
 
-    size_t addressesFound = 0;
-    if (result->mAddress.HasValue())
-    {
-        nodeData.resolutionData.ipAddress[addressesFound] = result->mAddress.Value();
-        nodeData.resolutionData.interfaceId               = result->mInterface;
-        ++addressesFound;
-    }
+    nodeData.resolutionData.interfaceId = result->mInterface;
 
-    for (auto & ip : extraIPs)
+    size_t addressesFound = 0;
+    for (auto & ip : addresses)
     {
         if (addressesFound == ArraySize(nodeData.resolutionData.ipAddress))
         {
@@ -88,7 +83,7 @@ static void HandleNodeResolve(void * context, DnssdService * result, const Span<
     proxy->Release();
 }
 
-static void HandleNodeIdResolve(void * context, DnssdService * result, const Span<Inet::IPAddress> & extraIPs, CHIP_ERROR error)
+static void HandleNodeIdResolve(void * context, DnssdService * result, const Span<Inet::IPAddress> & addresses, CHIP_ERROR error)
 {
     ResolverDelegateProxy * proxy = static_cast<ResolverDelegateProxy *>(context);
     if (CHIP_NO_ERROR != error)
@@ -127,12 +122,7 @@ static void HandleNodeIdResolve(void * context, DnssdService * result, const Spa
     nodeData.operationalData.peerId     = peerId;
 
     size_t addressesFound = 0;
-    if (result->mAddress.HasValue())
-    {
-        nodeData.resolutionData.ipAddress[addressesFound] = result->mAddress.Value();
-        ++addressesFound;
-    }
-    for (auto & ip : extraIPs)
+    for (auto & ip : addresses)
     {
         if (addressesFound == ArraySize(nodeData.resolutionData.ipAddress))
         {
@@ -171,7 +161,8 @@ static void HandleNodeBrowse(void * context, DnssdService * services, size_t ser
         }
         else
         {
-            HandleNodeResolve(context, &services[i], Span<Inet::IPAddress>(), error);
+            Inet::IPAddress * address = &(services[i].mAddress.Value());
+            HandleNodeResolve(context, &services[i], Span<Inet::IPAddress>(address, 1), error);
         }
     }
     proxy->Release();

src/lib/dnssd/platform/Dnssd.h

@@ -87,13 +87,13 @@ struct DnssdService
  * any pointer inside this structure.
  *
  * @param[in] context     The context passed to ChipDnssdBrowse or ChipDnssdResolve.
- * @param[in] result      The mdns resolve result, can be nullptr if error happens.
- * @param[in] extraIPs    IP addresses, in addition to the one in "result", for
- *                        the same hostname.  Can be empty.
+ * @param[in] result      The mdns resolve result, can be nullptr if error
+ *                        happens.  The mAddress of this object will be ignored.
+ * @param[in] addresses   IP addresses that we resolved.
  * @param[in] error       The error code.
  *
  */
-using DnssdResolveCallback = void (*)(void * context, DnssdService * result, const Span<Inet::IPAddress> & extraIPs,
+using DnssdResolveCallback = void (*)(void * context, DnssdService * result, const Span<Inet::IPAddress> & addresses,
                                       CHIP_ERROR error);
 
 /**

src/platform/Darwin/DnssdContexts.cpp

@@ -326,9 +326,6 @@ void ResolveContext::DispatchSuccess()
             continue;
         }
 
-        // Use the first IP we got for the DnssdService.
-        interface.second.service.mAddress.SetValue(ips.front());
-        ips.erase(ips.begin());
         callback(context, &interface.second.service, Span<Inet::IPAddress>(ips.data(), ips.size()), CHIP_NO_ERROR);
         break;
     }

src/platform/ESP32/DnssdImpl.cpp

@@ -321,7 +321,7 @@ CHIP_ERROR OnBrowseDone(BrowseContext * ctx)
     return RemoveMdnsQuery(reinterpret_cast<GenericContext *>(ctx));
 }
 
-size_t GetExtraIPsSize(mdns_ip_addr_t * addr)
+size_t GetAddressCount(mdns_ip_addr_t * addr)
 {
     size_t ret = 0;
     while (addr)
@@ -334,9 +334,8 @@ size_t GetExtraIPsSize(mdns_ip_addr_t * addr)
 
 CHIP_ERROR OnResolveQuerySrvDone(ResolveContext * ctx)
 {
-    CHIP_ERROR error           = CHIP_NO_ERROR;
-    mdns_ip_addr_t * extraAddr = nullptr;
-    size_t extraIPIndex        = 0;
+    CHIP_ERROR error    = CHIP_NO_ERROR;
+    size_t addressIndex = 0;
 
     VerifyOrExit(ctx && ctx->mResolveCb, error = CHIP_ERROR_INVALID_ARGUMENT);
     VerifyOrExit(ctx->mService == nullptr && ctx->mResolveState == ResolveContext::ResolveState::QuerySrv,
@@ -359,33 +358,30 @@ CHIP_ERROR OnResolveQuerySrvDone(ResolveContext * ctx)
 
         if (ctx->mResult->addr)
         {
-            Inet::IPAddress IPAddr;
-            GetIPAddress(IPAddr, ctx->mResult->addr);
-            ctx->mService->mAddress.SetValue(IPAddr);
-            extraAddr         = ctx->mResult->addr->next;
-            ctx->mExtraIPSize = GetExtraIPsSize(extraAddr);
-            if (ctx->mExtraIPSize > 0)
+            ctx->mAddressCount = GetAddressCount(ctx->mResult->addr);
+            if (ctx->mAddressCount > 0)
             {
-                ctx->mExtraIPs =
-                    static_cast<Inet::IPAddress *>(chip::Platform::MemoryCalloc(ctx->mExtraIPSize, sizeof(Inet::IPAddress)));
-                if (ctx->mExtraIPs == nullptr)
+                ctx->mAddresses =
+                    static_cast<Inet::IPAddress *>(chip::Platform::MemoryCalloc(ctx->mAddressCount, sizeof(Inet::IPAddress)));
+                if (ctx->mAddresses == nullptr)
                 {
-                    ChipLogError(DeviceLayer, "Failed to alloc memory for ExtraIPs");
-                    error             = CHIP_ERROR_NO_MEMORY;
-                    ctx->mExtraIPSize = 0;
+                    ChipLogError(DeviceLayer, "Failed to alloc memory for addresses");
+                    error              = CHIP_ERROR_NO_MEMORY;
+                    ctx->mAddressCount = 0;
                     ExitNow();
                 }
-                while (extraAddr)
+                auto * addr = ctx->mResult->addr;
+                while (addr)
                 {
-                    GetIPAddress(ctx->mExtraIPs[extraIPIndex], extraAddr);
-                    extraIPIndex++;
-                    extraAddr = extraAddr->next;
+                    GetIPAddress(ctx->mAddressCount[addressIndex], addr);
+                    addressIndex++;
+                    addr = addr->next;
                 }
             }
             else
             {
-                ctx->mExtraIPs    = nullptr;
-                ctx->mExtraIPSize = 0;
+                ctx->mAddresses    = nullptr;
+                ctx->mAddressCount = 0;
             }
         }
     }
@@ -429,7 +425,7 @@ CHIP_ERROR OnResolveQueryTxtDone(ResolveContext * ctx)
     }
     else
     {
-        ctx->mResolveCb(ctx->mCbContext, ctx->mService, Span<Inet::IPAddress>(ctx->mExtraIPs, ctx->mExtraIPSize), error);
+        ctx->mResolveCb(ctx->mCbContext, ctx->mService, Span<Inet::IPAddress>(ctx->mAddresses, ctx->mAddressCount), error);
     }
     RemoveMdnsQuery(reinterpret_cast<GenericContext *>(ctx));
     return error;

src/platform/ESP32/DnssdImpl.h

@@ -92,8 +92,8 @@ struct ResolveContext : public GenericContext
     char mInstanceName[Common::kInstanceNameMaxLength + 1];
     DnssdResolveCallback mResolveCb;
     DnssdService * mService;
-    Inet::IPAddress * mExtraIPs;
-    size_t mExtraIPSize;
+    Inet::IPAddress * mAddresses;
+    size_t mAddressCount;
 
     enum class ResolveState
     {
@@ -119,8 +119,8 @@ struct ResolveContext : public GenericContext
         mResolveState                                 = ResolveState::QuerySrv;
         mResult                                       = nullptr;
         mService                                      = nullptr;
-        mExtraIPs                                     = nullptr;
-        mExtraIPSize                                  = 0;
+        mAddresses                                    = nullptr;
+        mAddressCount                                 = 0;
     }
 
     ~ResolveContext()
@@ -133,9 +133,9 @@ struct ResolveContext : public GenericContext
             }
             chip::Platform::MemoryFree(mService);
         }
-        if (mExtraIPs)
+        if (mAddresses)
         {
-            chip::Platform::MemoryFree(mExtraIPs);
+            chip::Platform::MemoryFree(mAddresses);
         }
         if (mResult)
         {

src/platform/Linux/DnssdImpl.cpp

@@ -729,7 +729,6 @@ void MdnsAvahi::HandleResolve(AvahiServiceResolver * resolver, AvahiIfIndex inte
     case AVAHI_RESOLVER_FOUND:
         DnssdService result = {};
 
-        result.mAddress.SetValue(chip::Inet::IPAddress());
         ChipLogError(DeviceLayer, "Avahi resolve found");
 
         Platform::CopyString(result.mName, name);
@@ -753,6 +752,7 @@ void MdnsAvahi::HandleResolve(AvahiServiceResolver * resolver, AvahiIfIndex inte
         }
 
         CHIP_ERROR result_err = CHIP_ERROR_INVALID_ADDRESS;
+        chip::Inet::IPAddress ipAddress; // Will be set of result_err is set to CHIP_NO_ERROR
         if (address)
         {
             switch (address->proto)
@@ -762,7 +762,7 @@ void MdnsAvahi::HandleResolve(AvahiServiceResolver * resolver, AvahiIfIndex inte
                 struct in_addr addr4;
 
                 memcpy(&addr4, &(address->data.ipv4), sizeof(addr4));
-                result.mAddress.SetValue(chip::Inet::IPAddress(addr4));
+                ipAddress  = chip::Inet::IPAddress(addr4);
                 result_err = CHIP_NO_ERROR;
 #else
                 ChipLogError(Discovery, "Ignoring IPv4 mDNS address.");
@@ -772,7 +772,7 @@ void MdnsAvahi::HandleResolve(AvahiServiceResolver * resolver, AvahiIfIndex inte
                 struct in6_addr addr6;
 
                 memcpy(&addr6, &(address->data.ipv6), sizeof(addr6));
-                result.mAddress.SetValue(chip::Inet::IPAddress(addr6));
+                ipAddress  = chip::Inet::IPAddress(addr6);
                 result_err = CHIP_NO_ERROR;
                 break;
             default:
@@ -802,7 +802,7 @@ void MdnsAvahi::HandleResolve(AvahiServiceResolver * resolver, AvahiIfIndex inte
 
         if (result_err == CHIP_NO_ERROR)
         {
-            context->mCallback(context->mContext, &result, Span<Inet::IPAddress>(), CHIP_NO_ERROR);
+            context->mCallback(context->mContext, &result, Span<Inet::IPAddress>(&ipAddress, 1), CHIP_NO_ERROR);
         }
         else
         {

src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp

@@ -2391,8 +2391,9 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::FromOtDnsRespons
 template <class ImplClass>
 void GenericThreadStackManagerImpl_OpenThread<ImplClass>::DispatchResolve(intptr_t context)
 {
-    auto * dnsResult = reinterpret_cast<DnsResult *>(context);
-    ThreadStackMgrImpl().mDnsResolveCallback(dnsResult->context, &(dnsResult->mMdnsService), Span<Inet::IPAddress>(),
+    auto * dnsResult            = reinterpret_cast<DnsResult *>(context);
+    Inet::IPAddress * ipAddress = &(dnsResult->mMdnsService.mAddress.Value());
+    ThreadStackMgrImpl().mDnsResolveCallback(dnsResult->context, &(dnsResult->mMdnsService), Span<Inet::IPAddress>(ipAddress, 1),
                                              dnsResult->error);
     Platform::Delete<DnsResult>(dnsResult);
 }

src/platform/Tizen/DnssdImpl.cpp

@@ -459,8 +459,7 @@ void OnResolve(dnssd_error_e result, dnssd_service_h service, void * data)
 
     if (validIP)
     {
-        mdnsService.mAddress.SetValue(ipStr);
-        rCtx->callback(rCtx->cbContext, &mdnsService, chip::Span<chip::Inet::IPAddress>(), CHIP_NO_ERROR);
+        rCtx->callback(rCtx->cbContext, &mdnsService, chip::Span<chip::Inet::IPAddress>(&ipStr, 1), CHIP_NO_ERROR);
         StopResolve(rCtx);
     }
     else

src/platform/android/DnssdImpl.cpp

@@ -217,10 +217,12 @@ void HandleResolve(jstring instanceName, jstring serviceType, jstring address, j
 {
     VerifyOrReturn(callbackHandle != 0, ChipLogError(Discovery, "HandleResolve called with callback equal to nullptr"));
 
-    const auto dispatch = [callbackHandle, contextHandle](CHIP_ERROR error, DnssdService * service = nullptr) {
+    const auto dispatch = [callbackHandle, contextHandle](CHIP_ERROR error, DnssdService * service = nullptr,
+                                                          Inet::IPAddress * address = nullptr) {
         DeviceLayer::StackLock lock;
         DnssdResolveCallback callback = reinterpret_cast<DnssdResolveCallback>(callbackHandle);
-        callback(reinterpret_cast<void *>(contextHandle), service, Span<Inet::IPAddress>(), error);
+        size_t addr_count             = (address == nullptr) ? 0 : 1;
+        callback(reinterpret_cast<void *>(contextHandle), service, Span<Inet::IPAddress>(address, addr_count), error);
     };
 
     VerifyOrReturn(address != nullptr && port != 0, dispatch(CHIP_ERROR_UNKNOWN_RESOURCE_ID));
@@ -239,10 +241,9 @@ void HandleResolve(jstring instanceName, jstring serviceType, jstring address, j
     DnssdService service = {};
     CopyString(service.mName, jniInstanceName.c_str());
     CopyString(service.mType, jniServiceType.c_str());
-    service.mAddress.SetValue(ipAddress);
     service.mPort = static_cast<uint16_t>(port);
 
-    dispatch(CHIP_NO_ERROR, &service);
+    dispatch(CHIP_NO_ERROR, &service, &ipAddress);
 }
 
 } // namespace Dnssd

src/platform/tests/TestDnssd.cpp

@@ -14,16 +14,19 @@ using chip::Dnssd::DnssdService;
 using chip::Dnssd::DnssdServiceProtocol;
 using chip::Dnssd::TextEntry;
 
-static void HandleResolve(void * context, DnssdService * result, const chip::Span<chip::Inet::IPAddress> & extraIPs,
+static void HandleResolve(void * context, DnssdService * result, const chip::Span<chip::Inet::IPAddress> & addresses,
                           CHIP_ERROR error)
 {
     char addrBuf[100];
     nlTestSuite * suite = static_cast<nlTestSuite *>(context);
 
     NL_TEST_ASSERT(suite, result != nullptr);
     NL_TEST_ASSERT(suite, error == CHIP_NO_ERROR);
-    result->mAddress.Value().ToString(addrBuf, sizeof(addrBuf));
-    printf("Service at [%s]:%u\n", addrBuf, result->mPort);
+    if (!addresses.empty())
+    {
+        addresses.data()[0].ToString(addrBuf, sizeof(addrBuf));
+        printf("Service at [%s]:%u\n", addrBuf, result->mPort);
+    }
     NL_TEST_ASSERT(suite, result->mTextEntrySize == 1);
     NL_TEST_ASSERT(suite, strcmp(result->mTextEntries[0].mKey, "key") == 0);
     NL_TEST_ASSERT(suite, strcmp(reinterpret_cast<const char *>(result->mTextEntries[0].mData), "val") == 0);