BIND 10 #1775: update in-memory getAdditional() to handle wildcard match for additional names

[BIND 10 Development]

#1775: update in-memory getAdditional() to handle wildcard match for additional
names
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:  high   |  Sprint-20120403
                  Component:         |            Resolution:
  Unclassified                       |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  5
Feature Depending on Ticket:  auth   |           Total Hours:  0
  performance                        |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei


Comment:

 Hello

 I have few notes. First, the cppcheck fails here:
 {{{#!c++
 src/lib/datasrc/memory_datasrc.cc:376: check_fail: Possible null pointer
 dereference: node (error,nullPointer)
 }}}

 I believe this probably can't happen to be NULL, but we might want to
 assert or check and throw in case it is.

 Also, it is not obvious why the size of the tree that is created to hold
 the wildcard-constructed additionals can't grow and grow (I thing I found
 a reason after some time of thinking, though). Would it be possible to
 comment on why the size is bounded by the characteristics of the zone?

 And, I do not think this is safe:
 {{{#!c++
         } else {
             // For the tradeoff between safety and performance (of the
             // const version), we discard the constness here.  In practice
             // this should be okay because internally this node was
 retrieved
             // from the zone tree as a mutable pointer anyway.
             *nodep = const_cast<DomainNode*>(result.node);
         }
 }}}

 To be clear, I do not see any specific problem. But every time I see a
 const_cast, I get a bad feeling about it. While reinterpret_cast might be
 completely safe in some circumstances, the const cast probably is never
 safe if we were to take the C/C++ standards literally. The compiler can
 detect something is const and optimise based on it. It is not required to
 check if the const data really is const or if it is broken in some cast
 way. So every time we use it, we risk a future problem when some compiler
 introduces an optimisation that takes advantage of it (like it happened
 with gcc and strict aliasing rules ‒ a lot of code broke and there's the
 `--no-strict-aliasing` parameter since then to „fix“ it). Do you think it
 could be done in a way without that, maybe by reusing the code through a
 template, or something (hmm, that wouldn't look nice either)?

 Thank you

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