> 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