Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
May 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/include/platform/ThreadStackManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 8 additions & 17 deletions src/lib/dnssd/Discovery_ImplPlatform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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))
{
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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))
{
Expand Down Expand Up @@ -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();
Expand Down
8 changes: 4 additions & 4 deletions src/lib/dnssd/platform/Dnssd.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

/**
Expand Down
3 changes: 0 additions & 3 deletions src/platform/Darwin/DnssdContexts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
42 changes: 19 additions & 23 deletions src/platform/ESP32/DnssdImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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,
Expand All @@ -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;
}
}
}
Expand Down Expand Up @@ -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;
Expand Down
12 changes: 6 additions & 6 deletions src/platform/ESP32/DnssdImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -119,8 +119,8 @@ struct ResolveContext : public GenericContext
mResolveState = ResolveState::QuerySrv;
mResult = nullptr;
mService = nullptr;
mExtraIPs = nullptr;
mExtraIPSize = 0;
mAddresses = nullptr;
mAddressCount = 0;
}

~ResolveContext()
Expand All @@ -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)
{
Expand Down
8 changes: 4 additions & 4 deletions src/platform/Linux/DnssdImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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)
Expand All @@ -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.");
Expand All @@ -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:
Expand Down Expand Up @@ -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
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
3 changes: 1 addition & 2 deletions src/platform/Tizen/DnssdImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 5 additions & 4 deletions src/platform/android/DnssdImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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
Expand Down
9 changes: 6 additions & 3 deletions src/platform/tests/TestDnssd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down