BIND 10 #2267: new generateRRsetFromIterator doesn't handle RRSIGs correctly

[BIND 10 Development]

#2267: new generateRRsetFromIterator doesn't handle RRSIGs correctly
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:         |             Milestone:
  defect                             |  Sprint-20120925
                   Priority:         |            Resolution:
  medium                             |             Sensitive:  0
                  Component:  data   |           Sub-Project:  DNS
  source                             |  Estimated Difficulty:  6
                   Keywords:         |           Total Hours:  0
            Defect Severity:  N/A    |
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Thanks for the review.

 Replying to [comment:10 jelte]:

 > Looks ok, just some minor comments on memory_client.cc:
 >
 > while you are cleaning up, in void add() (lines 445-474), the second
 half could be refactored a tiny bit, since the addWildcards is the
 'exceptional' thing (addRdataSet happens both for NSEC3 and non-NSEC3),
 i.e. make it something like:
 >
 > {{{
 >         if (rrset->getType() != RRType::NSEC3()) {
 >             // Add wildcards possibly contained in the owner name to the
 >             // domain tree.
 >             // Note: this can throw an exception, breaking strong
 exception
 >             // guarantee.  (see also the note for the call to
 contextCheck()
 >             // above).
 >             addWildcards(zone_name, zone_data, rrset->getName());
 >         }
 >
 >         addRdataSet(zone_name, zone_data, rrset, sig_rrset);
 > }}}

 You mean, instead of checking the type NSEC3 at the first call to
 `addRdataSet`?  Updated the code assuming so.

 > I also suggest you slightly change the comments above the Loader class;
 'if the owner name is changed' suggests to me something like a setName()
 function. So I would prefer saying something akin to 'if a new owner name
 is encountered, no subsequent RRset will have the previous owner name'.

 Okay, done.  I also noticed some editorial errors around there, and
 fixed them.

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