BIND 10 #447: MemoryZone::find()

[BIND 10 Development]

#447: MemoryZone::find()
------------------------------+---------------------------------------------
      Reporter:  vorner       |        Owner:  hanfeng  
          Type:  task         |       Status:  reviewing
      Priority:  major        |    Milestone:           
     Component:  data source  |   Resolution:           
      Keywords:               |    Sensitive:  0        
Estimatedhours:  0.0          |        Hours:  0        
      Billable:  1            |   Totalhours:  0        
      Internal:  0            |  
------------------------------+---------------------------------------------

Comment(by jinmei):

 Hmm, I didn't notice it was assigned to Feng and have looked into the
 code a bit.  I'll stop my review at this point, but dump the comments
 I've had so far:

  - do we need to include <iostream>?

 '''MemoryZoneImpl::add()'''
  - this method does not provide strong exception guarantee (if it
    throws after insert(), the inserted node will remain).  It may be
    okay at least for now, because in the case of exception quite
    likely that the entire zone will be given up.  But it may have a
    tricky implication if/when we want to support dynamic update using
    this implementation (consider the case where we add a new name and
    then allocating the data fails with an exception).  so I'd suggest
    adding (doxygen) comment about this point.
  - I'm not sure why we need the tricks with getRelativeName().  As far
    as I understand it the rbtree should work without trimming the
    origin.  Perhaps it intends to be performance optimization (for
    find())?  If so, I suspect the performance gain wouldn't be much
    substantial because getting the prefix itself is a quite expensive
    operation, and I suspect the possible gain doesn't outweigh the
    additional code complexity.

 '''MemoryZoneImpl::find()'''
  - unfortunately, the delegation case cannot be that simple.  (Using
    the unittest setting) consider the case we have
    ns.subdomain.example.org as RDATA of the subdomain.example.org/NS,
    and an A and/or AAAA glue RRs of ns.subdomain.example.org in the
    example.org zone.  When we ask for ns.subdomain.example.org we
    should return the delegation (unless we really need the glues for
    additional section processing), but the current implementation
    would return an exact match.  So, my suggestion is to leave the
    delegation case open for now and focus on the easier case as
    originally intended for this ticket.

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