BIND 10 #2750: support DomainTree::remove()

BIND 10 Development do-not-reply at isc.org
Tue Sep 3 04:59:59 UTC 2013


#2750: support DomainTree::remove()
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  task          |  UnAssigned
            Priority:  medium        |                       Status:
           Component:  data source   |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130903
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  7             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |  shared memory data source
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by muks):

 * owner:  muks => UnAssigned
 * status:  assigned => reviewing


Comment:

 Hello

 I feel the branch `trac2750` is ready for review now. A few notes for
 the reviewer:

 * It is easiest to review this whole work as a single patch. Don't
   attempt to review it commit by commit as a lot of the code has changed
   among the patches.

 * The changes to `find()` and other `DomainTree` methods are to add
   non-`const` variants of these methods so that we can use the resulting
   mutable nodes.

 * The patch may be big, but once you review the non-`const` changes, the
   rest should be straightforward. Begin by looking at the `remove()`
   method which first does a normal BST node removal. Then look at the
   `tryNodeFusion()` method. Go to the `removeRebalance()` call
   last. Note that even if `removeRebalance()` or `tryNodeFusion()` are
   not called, the resulting tree is still a valid binary-search tree. So
   I advise you to begin with the `remove()` method.

 * All methods are commented so that a developer can follow what is
   happening. If you are unable to follow some part of code, or think the
   comments are inadequate, please make a note of it, especially in
   methods such as `removeRebalance()`.

 * We test at the interface, and not every single case of code in
   `removeRebalance()` directly. Why we do this is is explained in the
   comment for `DomainTreeTest.insertAndRemove` unittest. Note that all
   cases will be covered eventually, for the RB tree to be valid, and you
   can verify this with `lcov`.

 * The outstanding major problem with this branch is that it does not
   compile with `clang++`. `clang++` does not seem to support templated
   static method calls (`find<void*>()` for the `CBARG` type). Please
   verify that this is indeed the case, or provide your observation.

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


More information about the bind10-tickets mailing list