BIND 10 #397: Port generic red-black tree (RBT) data structure from BIND-9

BIND 10 Development do-not-reply at isc.org
Mon Dec 13 10:54:53 UTC 2010


#397: Port generic red-black tree (RBT) data structure from BIND-9
------------------------------+---------------------------------------------
      Reporter:  zzchen_pku   |        Owner:  jinmei   
          Type:  enhancement  |       Status:  reviewing
      Priority:  major        |    Milestone:           
     Component:  data source  |   Resolution:           
      Keywords:               |    Sensitive:  0        
Estimatedhours:  0.0          |        Hours:  0        
      Billable:  1            |   Totalhours:  0        
      Internal:  0            |  
------------------------------+---------------------------------------------
Changes (by hanfeng):

  * owner:  hanfeng => jinmei


Comment:

 Replying to [comment:36 jinmei]:
 > First, the previous comment about test coverage doesn't seem to be
 > addressed:
 Done
 >
 > '''namespace helper'''
 > Okay, but leave a comment on indent() that the only reason for having
 > it as a member function (even though it doesn't have to get access to
 > the private members) is to hide a helper function.
 comment added

 > '''RBTreeColor'''
 >  - RBTreeColor: it seems this can (should) be non public
 Basically, no matter which RBTree class will be instantiated(according to
 the template parameter)
 they all use the same color, so I put it outside RBNode and RBTree
 template class definition.

 >
 > >>  - type of data_: I don't think it works well.  getting a pointer
 from
 > Hmm, I don't see why we cannot use smart pointers (of course we
 > couldn't call "delete" for example but addressing such things should
 > be quite trivial).
 Now use shared_ptr to save the node data

 > '''RBTree class definition'''
 > Okay, and I'll refactor the resulting code a little bit:
 > {{{
 >                 if (!node->isEmpty()) {
 >                     ret = RBTree<T>::PARTIALMATCH;
 >                     *target = node;
 >                 }
 >                 node = node->down_;
 > }}}
 Modified accordingly.

 > '''nodeFission'''
 > >>  - down_node initialization could be simplified (and more efficient):
 > >
 > There's still redundant re-initialization
 Modified accordingly

 > Also, now that RBNode<T>::swap() isn't used, let's remove them (if
 > it's needed in future, reintroduce it at that point).
 swap is removed

 > The revised logic for the subdomain case looks okay.  Actually, I like
 > the simplified code.  One nit though: as far as I can see we don't
 > need the auto_ptr hack at the end of the function:
 I will keep it. actually, start from memory allocation to
 the end of the function call, we are not sure whether there will be
 exception raised

 >
 > '''RBTree<T>::findHelper'''
 Comment is added.

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


More information about the bind10-tickets mailing list