BIND 10 #2850: Define and implement ZoneTableSegmentMapped

[BIND 10 Development]

#2850: Define and implement ZoneTableSegmentMapped
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  task          |  jinmei
            Priority:  medium        |                       Status:
           Component:  data source   |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130528
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  5             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |  shared memory data source
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------

Comment (by jinmei):

 '''zone_table_segment_mapped.cc'''

 - `getHeaderHelper()`: I would skip `getNamedAddress` for non writable
   segments.  (I thought I suggested it, but it looks like not that
   clear)  Otherwise we need to do the expensive operation for every
   query lookup.

 '''memory_segment.h'''

 - I'd avoid repeating the same text about valid names for all methods.
 - Likewise, I'd simply say e.g. "name is invalid (see above which
   names are invalid)" than repeating specific cases:
 {{{#!cpp
     /// \throw InvalidParameter name is NULL, empty ("") or begins with
     /// an underscore ('_').
 }}}
 - very minor point, this organization looks a bit awkward:
 {{{#!cpp
         if (!name) {
             isc_throw(InvalidParameter, "NULL is invalid for a name.");
         }
         if (*name == '\0') {
             isc_throw(InvalidParameter, "Empty names are invalid.");
         } else if (*name == '_') {
             isc_throw(InvalidParameter,
                       "Names beginning with '_' are reserved for "
                       "internal use only.");
         }
 }}}
   why is the only one `else if`?

 '''memory_segment_mapped.cc'''

 - `allMemoryDeallocated`: I'd note in comment that reserveMemory
   should succeed as we've already allocated the space.
 - I'd make sure memory relocation doesn't happen in `reserveMemory` if
   called from `allMemoryDeallocated` (it shouldn't happen for the same
   reason as above).  passing an argument to prevent looping, etc.
   note also that if we abort there we shouldn't need try-catch in
   `allMemoryDeallocated`; no other part within the try block can
   throw.

 '''zone_writer.cc'''

 while almost out of scope for this ticket, there's still one more
 glitch in `ZoneWriter::install`: `zone_data_` can be relocated on
 `MemorySegmentGrown`.  Further, in general, we should protect
 `zone_data_` immediately after the call to `load_action_`.  This will
 be beyond a trivial change, and I don't want to delay this task even
 further, but at the same time I don't want to leave the flaw longer.

 So I'm attaching a proposed patch to address this point.  In some
 sense it only does some straightforward thing: maintain a holder
 within the writer and capsule zone_data in the holder as long as
 necessary.  But I wanted to keep the details (especially the holder
 class) hidden in .cc so I moved local members into pimpl struct.  So
 the size of the change is a bit larger.

 If you can review it and agree with it, please incorporate it into the
 branch.  If it becomes controversial, I suggest deferring it to a
 separate urgent task.

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