[asterisk-dev] [Code Review] 4542: DNS: Add NAPTR support and tests

[Kevin Harwell]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4542/#review14984
-----------------------------------------------------------



/team/group/dns/include/asterisk/dns_internal.h
<https://reviewboard.asterisk.org/r/4542/#comment25605>

    These methods should not start with ast_* unless they are meant to be exposed externally/exported. If they are then they should be moved to a more "public" include file.



/team/group/dns/main/dns_naptr.c
<https://reviewboard.asterisk.org/r/4542/#comment25604>

    This seems like it should be a non assert check. What happens if asterisk is not compiled without debug on and this is false?



/team/group/dns/res/res_resolver_unbound.c
<https://reviewboard.asterisk.org/r/4542/#comment25608>

    Any reason to continue if a failure occurs?



/team/group/dns/tests/test_dns_naptr.c
<https://reviewboard.asterisk.org/r/4542/#comment25606>

    Should these failures break the loop and just goto cleanup as well?


- Kevin Harwell


On March 27, 2015, 9:45 a.m., Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4542/
> -----------------------------------------------------------
> 
> (Updated March 27, 2015, 9:45 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This adds NAPTR support for DNS in Asterisk.
> 
> The main parts of this are the functions for allocating a DNS NAPTR record when a resolver wishes to add a NAPTR record, the sorting algorithm for sorting DNS NAPTR records, and the tests that use a mock DNS resolver.
> 
> NOTE: There is likely to be some overlap here in this review and Josh's SRV review (/r/4528). Our stance on this is that we will factor out the duplicated code once both SRV and NAPTR have been merged into the main DNS branch. The factoring out of common functions will be placed in its own review.
> 
> 
> Diffs
> -----
> 
>   /team/group/dns/tests/test_dns_naptr.c PRE-CREATION 
>   /team/group/dns/res/res_resolver_unbound.c 433573 
>   /team/group/dns/main/dns_naptr.c 433573 
>   /team/group/dns/main/dns_core.c 433573 
>   /team/group/dns/include/asterisk/dns_internal.h 433573 
> 
> Diff: https://reviewboard.asterisk.org/r/4542/diff/
> 
> 
> Testing
> -------
> 
> All previous DNS tests continue to pass, and all new tests added in this review pass as well.
> 
> 
> Thanks,
> 
> Mark Michelson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20150331/77540e8e/attachment-0001.html>