BIND 10 #2310: In-memory version of ZoneFinder::findAtOrigin()

[BIND 10 Development]

#2310: In-memory version of ZoneFinder::findAtOrigin()
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  task          |  jinmei
            Priority:  medium        |                       Status:
           Component:  data source   |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130205
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  5             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei


Comment:

 Hello

 I don't see anything substantial, just minor details.

 Could we use the `htonl` instead of the in-house implementation of
 `setTTLInNetOrder`? It just duplicates call (and the library one could be
 inlined as compiler built-in). Note that the htonl explicitly uses
 `uint32_t`.

 Would it make sense to cache the result of createTTLFromData? How heavy is
 the creation of the TTL object?

 The `findAtOrigin` method is a lot simpler than the generic `find`. It has
 fewer cases. After thinking about it, I got to conclusion the other cases
 can not happen, because they generally handle things that can't legally
 exist in the origin (like not having the node or having CNAME there). But
 I think this reasoning should be described in comment in the method,
 otherwise another reader will come and stop to think if the other cases
 are just missing.

 Is this function correct? Can't SOA be added to other places than just
 origin? Or, if it doesn't matter, is it OK to just pass `ROOT_NAME` every
 time, even to SOA?

 {{{#!c++
 ConstRRsetPtr
 convertRRset(ConstRRsetPtr src) {
     // If the type is SOA, textToRRset performs a stricter check, so we
 should
     // specify the origin.
     const Name& origin = (src->getType() == RRType::SOA()) ?
         src->getName() : Name::ROOT_NAME();
     return (textToRRset(src->toText(), src->getClass(), origin));
 }
 }}}

 This is similar. Could we just simplify it and pass the origin always?
 {{{#!diff
 -            *zone_data[i].rrset = textToRRset(zone_data[i].text);
 +            if (zone_data[i].rrset == &rr_soa_) {
 +                // This is zone's SOA.  We need to specify the origin for
 +                // textToRRset; otherwise it would throw.
 +                *zone_data[i].rrset = textToRRset(zone_data[i].text,
 class_,
 +                                                  origin_);
 +            } else {
 +                *zone_data[i].rrset = textToRRset(zone_data[i].text);
 +            }
 }}}

 Should there be a test where `min_ttl` is requested and the TTL of the
 RRset is lower, so it wins?

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