BIND 10 #2091: use encoded name data in RBNode

[BIND 10 Development]

#2091: use encoded name data in RBNode
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20120731
  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:         |
  scalable inmemory                  |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei


Comment:

 OK, let's leave the description as it is in the trac2091a.

 So, to the rest:
  * This comment seems to be reversed. The binary is not followed by the
 node, but it is following it (the node is first, the label second):
 {{{
         ...This is encoded
 /// as opaque binary immediately followed by the main node object.
 }}}
  * In this piece of code, the getName needlessly asserts a prerequisite it
 doesn't have to care about. Also, the same assert is called twice:
 {{{#!c++
     const isc::dns::Name getName() const {
         assert(labels_capacity_ != 0); // shouldn't be called on a NULL
 node.
         return (dns::Name(dns::LabelSequence(getLabelsData()).toText()));
     }

     /// \brief Return the label sequence of the node.
     ///
     /// This method returns the label sequence corresponding to this node
     /// in the form of \c dns::LabelSequence object.  Any modification to
     /// the tree can invalidate the returned \c LabelSequence object or
 copy
     /// of it; in general, it's expected to be used in a very limited
 scope.
     dns::LabelSequence getLabels() const {
         assert(labels_capacity_ != 0); // shouldn't be called on a NULL
 node.
         return (dns::LabelSequence(getLabelsData()));
     }
 }}}
  * To the same piece of code, isn't there a constructor for name which
 could be used from binary data? It could be faster than transforming it to
 text and back.
  * What is the above note this one talks about? It seems to refer to the
 flag at the doxygen about the function, but that is far away.
 {{{#!c++
     // Swap flags bitfields; yes, this is ugly.  The right solution is to
     // implement the above note, then we won't have to swap the flags in
 the
     // first place.
 }}}
  * What is the purpose of the enum right under that comment? Why can't the
 tmp be just uint32_t?
  * Could the commented-out code be removed completely?
 {{{#!c++
     // bb < c, no common label
     //comparisonChecks(chain, -1, 1,
 NameComparisonResult::COMMONANCESTOR);
     comparisonChecks(chain, -1, 0, NameComparisonResult::NONE);
 }}}

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