BIND 10 #1579: Update database ZoneFinder::find() for negative cases of NSEC3-signed zones

[BIND 10 Development]

#1579: Update database ZoneFinder::find() for negative cases of NSEC3-signed zones
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20120403
  medium                             |            Resolution:
                  Component:  data   |             Sensitive:  0
  source                             |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  5
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:19 haikuo]:
 > after our discussion, I adjust my codes. please review it.

 First off, please basically respond to each itemized point one by
 one.  I cannot be sure whether you addressed all points or part of
 them, and whether you agreed or disagreed with me about each case (you
 don't necessarily have to agree, of course, but if you didn't say
 anything I cannot tell).  In any case, please remember that such a
 one-line response to itemized specific comments would be generally not
 acceptable.

 Secondly, in fact, it doesn't address one of my main concerns: "If we
 know it's an NSEC3-signed zone, I wouldn't bother to look for NSEC
 because it's a waste. I'd introduce a helper method or something to
 unify such cases in an efficient way".  And, findInternal() is now
 terribly unreadable and non understandable.  If you cannot be sure
 about my concern, could I propose revised code rather than trying to
 explain my intent further?

 Third, I've still seen some editorial issues.  I've directly updated
 the branch and committed the changes.  Some are clearly described in
 the coding guideline page, but you still seem to not be able to follow
 them (although, to be fair, it's not only you, and I myself sometimes
 make such errors).  I'd suggest you reread the guideline page.  For
 the rationale of specific changes, please see the commit logs.

 Finally, the added test cases look basically okay, although I'd
 suggest testing some strange cases like the zone having both NSEC and
 NSEC3PARAM.  I suspect that the current implementation doesn't follow
 this API spec (the last sentence):
 {{{#!c++
     /// First, it tells the application whether the zone is signed with
     /// NSEC or NSEC3 via the \c isNSEC(3)Signed() method.  Any sanely
 signed
     /// zone should be signed with either (and only one) of these two
 types
     /// of RRs; however, the application should expect that the zone could
     /// be broken and these methods could both return false.  But this
 method
     /// should ensure that not both of these methods return true.
 }}}

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