> On March 31, 2015, 10:25 a.m., Matt Jordan wrote: > > /team/group/dns/main/dns_naptr.c, lines 447-449 > > <https://reviewboard.asterisk.org/r/4542/diff/2/?file=73012#file73012line447> > > > > Suggestion: since this is repeated after each check, you may want to > > macro-tize it: > > > > #define CHECK_END_OF_RECORD do { \ > > if (ptr => end_of_record) { \ > > return NULL; \ > > } } while (0) > > > > > > Then you can just put: > > > > ptr += 2; > > CHECK_END_OF_RECORD; > > > > Or something like that.
Doing this hides return points. Which adds a potential for memory and ref leaks. - rmudgett ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4542/#review14972 ----------------------------------------------------------- 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 > >
-- _____________________________________________________________________ -- 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
