> On March 31, 2015, 4:50 p.m., Kevin Harwell wrote:
> > /team/group/dns/main/dns_naptr.c, line 244
> > <https://reviewboard.asterisk.org/r/4542/diff/2/?file=73012#file73012line244>
> >
> > This seems like it should be a non assert check. What happens if
> > asterisk is not compiled without debug on and this is false?
The reason this is an assertion is because it should never be false.
Before regexp_repl_invalid is called, we divide the regexp based on the
delimiter, and we ensure that we do not divide on backslash-escaped delimiters.
This means that the regexp repl section cannot end with a backslash, unless it
ends in a backslash-escaped backslash ("\\"). Current code would find the first
backslash of the pair, and since the backslash is escaping an illegal
character, the routine would return -1 before possibly finding the second
backslash. If the code were altered to allow for backslash-escaped backslash,
then the code would have to be altered from its current form and the assertion
would need to be changed to a different type of check.
> On March 31, 2015, 4:50 p.m., Kevin Harwell wrote:
> > /team/group/dns/res/res_resolver_unbound.c, lines 1254-1291
> > <https://reviewboard.asterisk.org/r/4542/diff/2/?file=73013#file73013line1254>
> >
> > Any reason to continue if a failure occurs?
Since previous failures don't affect future operations, my thought was to go
through with the whole thing so that we can see if everything is failing or
only a specific case. Of course the status update messages here aren't very
helpful for determining what specific failure was seen, though. I can address
that though.
> On March 31, 2015, 4:50 p.m., Kevin Harwell wrote:
> > /team/group/dns/tests/test_dns_naptr.c, lines 439-461
> > <https://reviewboard.asterisk.org/r/4542/diff/2/?file=73014#file73014line439>
> >
> > Should these failures break the loop and just goto cleanup as well?
See my previous comment. A failure of one record doesn't necessarily mean the
failure of another. This way, all failures can be on display if the unit test
fails for whatever reason.
- Mark
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4542/#review14984
-----------------------------------------------------------
On March 27, 2015, 2:45 p.m., Mark Michelson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4542/
> -----------------------------------------------------------
>
> (Updated March 27, 2015, 2:45 p.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
>
>
--
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
