BIND 10 #1551: in-memory datasource allow RRSIG, NSEC, and CNAME at same time

[BIND 10 Development]

#1551: in-memory datasource allow RRSIG, NSEC, and CNAME at same time
-------------------------------------+-------------------------------------
                   Reporter:  jreed  |                 Owner:  jinmei
                       Type:         |                Status:  reviewing
  defect                             |             Milestone:
                   Priority:  major  |  Sprint-20120207
                  Component:  data   |            Resolution:
  source                             |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  4
Feature Depending on Ticket:         |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:9 vorner]:

 > The code seems to do what it should. But I think the condition looks
 really awkward and complicated:
 >
 > {{{#!c++
 >         // owner name except with NSEC, which is the only RR that can
 coexist
 >         // with CNAME (and also RRSIG, which is handled separately)
 >         if (rrset.getType() == RRType::CNAME()) {
 >             if (!domain.empty() &&
 >                 (domain.size() > 1 ||
 >                  (domain.begin()->second->getType() != RRType::NSEC())))
 {
 > }}}
 >
 > Could I propose using a for loop, iterating through all the RRsets there
 and stopping on the first one that is not NSEC? ...

 I agree that the quoted original code wasn't intuitive.  I didn't like
 it either but thought it was probably more efficient as in the vast
 majority of the cases the given zone should be valid and the check
 would stop at empty().  But that's probably a premature (and probably
 not so effective anyway) optimization, and especially because you also
 saw it awkward, I now think it's better to prefer clarity.

 I used find_if from <algorithm> instead of explicit loop:
 {{{#!c++
             if (find_if(domain.begin(), domain.end(), IsNotNSEC())
                 != domain.end()) {
 }}}
 because, IMO, explicit loop + check is also an indirect idiom from
 what it actually means and is essentially counter intuitive (but the
 idiom is so common for many experienced programmers that it could look
 natural).  Unfortunately, in C++ the algorithmic approach also
 requires an indirection (we need to define a separate predicate
 class), so, in the end, it may be a matter of taste.

 So, if you strongly prefer a for loop in this case, I can live with
 that.

-- 
Ticket URL: <http://bind10.isc.org/ticket/1551#comment:10>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development