BIND 10 #356: Resolver address database

[BIND 10 Development]

#356: Resolver address database
-------------------------------+--------------------------------------------
      Reporter:  stephen       |        Owner:  stephen  
          Type:  enhancement   |       Status:  reviewing
      Priority:  major         |    Milestone:           
     Component:  Unclassified  |   Resolution:           
      Keywords:                |    Sensitive:  0        
Estimatedhours:  0.0           |        Hours:  0        
      Billable:  1             |   Totalhours:  0        
      Internal:  0             |  
-------------------------------+--------------------------------------------
Changes (by ocean):

  * owner:  UnAssigned => stephen


Comment:

 1. zone_entry.h
  * Suggest to remove "using namespace std;" from .h file and put 'std'
 prefix to related type.
  * Suggest to change !ZoneEntry's ctor from

 {{{
 ZoneEntry(AbstractRRset* nsrrset, vector<AbstractRRSet*> additional);
 }}}
 to
 {{{
 ZoneEntry(AbstractRRset* nsrrset, const vector<AbstractRRSet*>&
 additional);
 }}}
 to avoid additional copying.

  * Suggest to change the member variable of '''nameserver_''' to
 '''nameservers_''' to indicate that it may contain multiple items.

 2. nsas_types.h
  * Suggest to change '''!NasAddress''' to '''!NsasAddress''' to keep the
 acronyms consistent.

 3. nameserver_entry.h

  * Suggest to remove "using namespace" statements from header file.
  * Suggest to add "const" to the input pointer variable:

 {{{
 NameserverEntry(AbstractRRset* v4Set, AbstractRRset* v6Set, time_t curtime
 = 0);
 }}}
 to
 {{{
 NameserverEntry(const AbstractRRset* v4Set, const AbstractRRset* v6Set,
 time_t curtime = 0);
 }}}
  * Suggest to add a "virtual destructor" to !NameServer class.
         See. http://www.parashift.com/c++-faq-lite/virtual-
 functions.html#faq-20.7

  * Suggest to add "inline" indicator to the small "accessor" functions.
         I'm not sure whether the compiler will inline the code
 automatically, but add "inline" should do no harm.

  * For class of !AddressSelection, what is the type lf "result_type" ?
         A little confused.

  * Suggest to change member variable "class_" of class !ZoneEntry to
 "classCode_" to keep consistent with the same variable in
 !NameserverEntry.

 4. address_entry.h
  * Suggest to move declaration of "UNREACHABLE" to "private" section,
 since it is used only internally.

  * Suggest to initialize UNREACHABLE in the header file

 {{{
 static const uint32_t UNREACHABLE =UINT32_MAX;  ///< RTT indicating
 unreachable address
 }}}

  * Suggest to change

 {{{
 virtual uint32_t size() {
 }}}
 to
 {{{
 virtual uint32_t size() const  {
 }}}

   BTW, I'm a little confused by the comments:

 {{{
 /// as list.size() may take some time if the list is big.
 }}}

 In my understanding, the std::list should also keep a separate count_
 internally, so std::list::size() should also be O(1) no matter how big the
 list is.

  * Suggest to change

 {{{
 virtual uint32_t getMaxSize() {
 }}}
 to
 {{{
 virtual uint32_t getMaxSize() const {
 }}}

  * Since !LruList is also a virtual class, so I suggest to add a "Virtual
 Destructor" to it.

 5. hash.h
  * Shall this be shared by other component? If so, suggest to move it to a
 more common library such as utility.  otherwise change its name to
 nsas_hash.h
  * Suggest to add "const" to the operator() and mapLower() function, since
 they do not modify any member variable

 {{{
     virtual uint32_t operator()(const char* key, uint32_t keylen, bool
 ignorecase = true) const;
     unsigned char mapLower(unsigned char inchar) const {
 }}}

  * suggest to remove "using namespace std" declaration in header file

 6. hash_table.h
  * The HASHTABLE_SIZE is defined but not used

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