Skip to content

Commit be935b7

Browse files
nivi-applenivi-apple
committed
Address review comments
1 parent b35892c commit be935b7

File tree

3 files changed

+40
-48
lines changed

3 files changed

+40
-48
lines changed

‎src/platform/Darwin/DnssdContexts.cpp

+7-23
Original file line numberDiff line numberDiff line change
@@ -465,8 +465,7 @@ void BrowseWithDelegateContext::OnBrowseRemove(const char * name, const char * t
465465
ResolveContext::ResolveContext(void * cbContext, DnssdResolveCallback cb, chip::Inet::IPAddressType cbAddressType,
466466
const char * instanceNameToResolve, BrowseContext * browseCausingResolve,
467467
std::shared_ptr<uint32_t> && consumerCounterToUse) :
468-
browseThatCausedResolve(browseCausingResolve),
469-
resolveContextWithSRPType({ this, true }), resolveContextWithNonSRPType({ this, false })
468+
browseThatCausedResolve(browseCausingResolve)
470469
{
471470
type = ContextType::Resolve;
472471
context = cbContext;
@@ -478,8 +477,7 @@ ResolveContext::ResolveContext(void * cbContext, DnssdResolveCallback cb, chip::
478477

479478
ResolveContext::ResolveContext(CommissioningResolveDelegate * delegate, chip::Inet::IPAddressType cbAddressType,
480479
const char * instanceNameToResolve, std::shared_ptr<uint32_t> && consumerCounterToUse) :
481-
browseThatCausedResolve(nullptr),
482-
resolveContextWithSRPType({ this, true }), resolveContextWithNonSRPType({ this, false })
480+
browseThatCausedResolve(nullptr)
483481
{
484482
type = ContextType::Resolve;
485483
context = delegate;
@@ -556,7 +554,7 @@ void ResolveContext::DispatchSuccess()
556554
for (auto & interface : interfaces)
557555
{
558556
if (TryReportingResultsForInterfaceIndex(interface.first.interfaceId, interface.first.hostname,
559-
interface.first.isSRPTypeRequested))
557+
interface.first.isSRPResult))
560558
{
561559
break;
562560
}
@@ -568,15 +566,15 @@ void ResolveContext::DispatchSuccess()
568566
}
569567
}
570568

571-
bool ResolveContext::TryReportingResultsForInterfaceIndex(uint32_t interfaceIndex, const std::string & hostname, bool isSRPType)
569+
bool ResolveContext::TryReportingResultsForInterfaceIndex(uint32_t interfaceIndex, const std::string & hostname, bool isSRPResolve)
572570
{
573571
if (interfaceIndex == 0)
574572
{
575573
// Not actually an interface we have.
576574
return false;
577575
}
578576

579-
InterfaceKey interfaceKey = { interfaceIndex, hostname, isSRPType };
577+
InterfaceKey interfaceKey = { interfaceIndex, hostname, isSRPResolve };
580578
auto & interface = interfaces[interfaceKey];
581579
auto & ips = interface.addresses;
582580

@@ -607,26 +605,12 @@ bool ResolveContext::TryReportingResultsForInterfaceIndex(uint32_t interfaceInde
607605

608606
bool ResolveContext::TryReportingResultsForInterfaceIndex(uint32_t interfaceIndex)
609607
{
610-
if (interfaceIndex == 0)
611-
{
612-
// Not actually an interface we have.
613-
return false;
614-
}
615-
616608
for (auto & interface : interfaces)
617609
{
618610
if (interface.first.interfaceId == interfaceIndex)
619611
{
620-
auto & ips = interface.second.addresses;
621-
622-
// Some interface may not have any ips, just ignore them.
623-
if (ips.size() == 0)
624-
{
625-
continue;
626-
}
627-
628612
if (TryReportingResultsForInterfaceIndex(interface.first.interfaceId, interface.first.hostname,
629-
interface.first.isSRPTypeRequested))
613+
interface.first.isSRPResult))
630614
{
631615
return true;
632616
}
@@ -750,7 +734,7 @@ void ResolveContext::OnNewInterface(uint32_t interfaceId, const char * fullname,
750734
// resolving.
751735
interface.fullyQualifiedDomainName = hostnameWithDomain;
752736

753-
InterfaceKey interfaceKey = { interfaceId, hostnameWithDomain, isSRPType };
737+
InterfaceKey interfaceKey = { interfaceId, hostnameWithDomain, isFromSRPResolve };
754738
interfaces.insert(std::make_pair(std::move(interfaceKey), std::move(interface)));
755739
}
756740

‎src/platform/Darwin/DnssdImpl.cpp

+28-21
Original file line numberDiff line numberDiff line change
@@ -279,20 +279,19 @@ static void OnGetAddrInfo(DNSServiceRef sdRef, DNSServiceFlags flags, uint32_t i
279279
auto contextWithType = reinterpret_cast<ResolveContextWithType *>(context);
280280
VerifyOrReturn(contextWithType != nullptr, ChipLogError(Discovery, "ResolveContextWithType is null"));
281281

282-
auto sdCtx = reinterpret_cast<ResolveContext *>(contextWithType->context);
282+
auto sdCtx = contextWithType->context;
283283
ReturnOnFailure(MdnsContexts::GetInstance().Has(sdCtx));
284284
LogOnFailure(__func__, err);
285285

286286
if (kDNSServiceErr_NoError == err)
287287
{
288-
InterfaceKey interfaceKey = { interfaceId, hostname, contextWithType->isSRP };
289-
sdCtx->OnNewAddress(interfaceKey, address);
288+
InterfaceKey interfaceKey = { interfaceId, hostname, contextWithType->isSRPResolve };
289+
CHIP_ERROR error = sdCtx->OnNewAddress(interfaceKey, address);
290290

291-
// Set the flag to start the timer for resolve on SRP domain to complete if the key has the SRP type requested flag set to
292-
// true.
293-
if (interfaceKey.isSRPTypeRequested)
291+
// If we saw an address resolved on the SRP domain, set the shouldStartSRPTimerForResolve to false.
292+
if (error == CHIP_NO_ERROR && contextWithType->isSRPResolve)
294293
{
295-
sdCtx->shoulStartSRPTimerForResolve = true;
294+
sdCtx->shouldStartSRPTimerForResolve = false;
296295
}
297296
}
298297

@@ -304,7 +303,7 @@ static void OnGetAddrInfo(DNSServiceRef sdRef, DNSServiceFlags flags, uint32_t i
304303
VerifyOrReturn(sdCtx->HasAddress(), sdCtx->Finalize(kDNSServiceErr_BadState));
305304

306305
// We should complete the resolve if we got a resolution on non SRP domain and no resolution on SRP domain was requested.
307-
if (!sdCtx->shoulStartSRPTimerForResolve)
306+
if (!sdCtx->shouldStartSRPTimerForResolve)
308307
{
309308
sdCtx->Finalize();
310309
}
@@ -332,19 +331,21 @@ static void GetAddrInfo(ResolveContext * sdCtx)
332331

333332
for (auto & interface : sdCtx->interfaces)
334333
{
334+
if (interface.second.isDNSLookUpRequested)
335+
{
336+
continue;
337+
}
338+
335339
auto interfaceId = interface.first.interfaceId;
336340
auto hostname = interface.second.fullyQualifiedDomainName.c_str();
337341
auto sdRefCopy = sdCtx->serviceRef; // Mandatory copy because of kDNSServiceFlagsShareConnection
338342

339-
if (!interface.second.isDNSLookUpRequested)
340-
{
341-
ResolveContextWithType * contextWithType =
342-
(interface.first.isSRPResult) ? &sdCtx->resolveContextWithSRPType : &sdCtx->resolveContextWithNonSRPType;
343-
auto err = DNSServiceGetAddrInfo(&sdRefCopy, kGetAddrInfoFlags, interfaceId, protocol, hostname, OnGetAddrInfo,
344-
contextWithType);
345-
VerifyOrReturn(kDNSServiceErr_NoError == err, sdCtx->Finalize(err));
346-
interface.second.isDNSLookUpRequested = true;
347-
}
343+
ResolveContextWithType * contextWithType =
344+
(interface.first.isSRPResult) ? &sdCtx->resolveContextWithSRPType : &sdCtx->resolveContextWithNonSRPType;
345+
auto err = DNSServiceGetAddrInfo(&sdRefCopy, kGetAddrInfoFlags, interfaceId, protocol, hostname, OnGetAddrInfo,
346+
contextWithType);
347+
VerifyOrReturn(kDNSServiceErr_NoError == err, sdCtx->Finalize(err));
348+
interface.second.isDNSLookUpRequested = true;
348349
}
349350
}
350351

@@ -358,13 +359,13 @@ static void OnResolve(DNSServiceRef sdRef, DNSServiceFlags flags, uint32_t inter
358359
auto contextWithType = reinterpret_cast<ResolveContextWithType *>(context);
359360
VerifyOrReturn(contextWithType != nullptr, ChipLogError(Discovery, "ResolveContextWithType is null"));
360361

361-
auto sdCtx = reinterpret_cast<ResolveContext *>(contextWithType->context);
362+
auto sdCtx = contextWithType->context;
362363
ReturnOnFailure(MdnsContexts::GetInstance().Has(sdCtx));
363364
LogOnFailure(__func__, err);
364365

365366
if (kDNSServiceErr_NoError == err)
366367
{
367-
sdCtx->OnNewInterface(interfaceId, fullname, hostname, port, txtLen, txtRecord, contextWithType->isSRPType);
368+
sdCtx->OnNewInterface(interfaceId, fullname, hostname, port, txtLen, txtRecord, contextWithType->isSRPResolve);
368369
}
369370

370371
if (!(flags & kDNSServiceFlagsMoreComing))
@@ -381,6 +382,12 @@ static CHIP_ERROR ResolveWithContext(ResolveContext * sdCtx, uint32_t interfaceI
381382

382383
auto err = DNSServiceResolve(&sdRef, kResolveFlags, interfaceId, name, type, domain, OnResolve, contextWithType);
383384
VerifyOrReturnError(kDNSServiceErr_NoError == err, sdCtx->Finalize(err));
385+
386+
// Set the flag to start the timer for resolve on SRP domain to complete if a resolve has been requested on the SRP domain.
387+
if (contextWithType->isSRPResolve)
388+
{
389+
sdCtx->shouldStartSRPTimerForResolve = true;
390+
}
384391
return CHIP_NO_ERROR;
385392
}
386393

@@ -399,8 +406,8 @@ static CHIP_ERROR Resolve(ResolveContext * sdCtx, uint32_t interfaceId, chip::In
399406
{
400407
ReturnErrorOnFailure(
401408
ResolveWithContext(sdCtx, interfaceId, type, name, domain,
402-
IsSRPType(domain) ? &sdCtx->resolveContextWithSRPType : &sdCtx->resolveContextWithNonSRPType));
403-
sdCtx->shoulStartSRPTimerForResolve = false;
409+
IsSRPDomain(domain) ? &sdCtx->resolveContextWithSRPType : &sdCtx->resolveContextWithNonSRPType));
410+
sdCtx->shouldStartSRPTimerForResolve = false;
404411
}
405412
else
406413
{

‎src/platform/Darwin/DnssdImpl.h

+5-4
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,8 @@ struct InterfaceKey
235235
inline bool operator<(const InterfaceKey & other) const
236236
{
237237
return (this->interfaceId < other.interfaceId) ||
238-
((this->interfaceId == other.interfaceId) && (this->hostname < other.hostname));
238+
((this->interfaceId == other.interfaceId) && (this->hostname < other.hostname)) ||
239+
((this->interfaceId == other.interfaceId) && (this->hostname == other.hostname) && (this->isSRPResult < other.isSRPResult));
239240
}
240241

241242
uint32_t interfaceId;
@@ -245,7 +246,7 @@ struct InterfaceKey
245246

246247
struct ResolveContextWithType
247248
{
248-
ResolveContextWithType() = default;
249+
ResolveContextWithType() = delete;
249250
~ResolveContextWithType() = default;
250251

251252
ResolveContext * const context;
@@ -266,8 +267,8 @@ struct ResolveContext : public GenericContext
266267
bool shouldStartSRPTimerForResolve = false;
267268
bool isSRPTimerRunning = false;
268269

269-
ResolveContextWithType resolveContextWithSRPType;
270-
ResolveContextWithType resolveContextWithNonSRPType;
270+
ResolveContextWithType resolveContextWithSRPType = {this , true};
271+
ResolveContextWithType resolveContextWithNonSRPType = {this, false};
271272

272273
// browseCausingResolve can be null.
273274
ResolveContext(void * cbContext, DnssdResolveCallback cb, chip::Inet::IPAddressType cbAddressType,

0 commit comments

Comments
 (0)