BIND 10 #1957: Implement perfdhcp Interface Manager

BIND 10 Development do-not-reply at isc.org
Mon Jun 25 12:39:43 UTC 2012


#1957: Implement perfdhcp Interface Manager
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  marcin
  stephen                            |                Status:  reviewing
                       Type:         |             Milestone:  Sprint-
  enhancement                        |  DHCP-20120703
                   Priority:         |            Resolution:
  medium                             |             Sensitive:  0
                  Component:         |           Sub-Project:  DHCP
  perfdhcp                           |  Estimated Difficulty:  24
                   Keywords:         |           Total Hours:  25
            Defect Severity:  N/A    |
Feature Depending on Ticket:         |
        Add Hours to Ticket:  25     |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by stephen):

 * owner:  UnAssigned => marcin


Comment:

 Reviewed commit 0e1996af4d06ab528f76e13a3ce7c4c29b12d046.

 Some minor changes to spacing around operators ands header comments have
 been made and pushed.

 '''src/lib/dhcp/iface_mgr.{h,cc}'''[[BR]]
 openSocketFromAddress()/openSocketFromIface(): the description says that
 the method will return the socket descriptor if the operation was
 successful.  However, if neither the interface nor address is found, the
 methods will return a value of zero.  Two options:

 a) Any failure - including not finding the interface or not finding the
 address - should result in an exception.

 b) If a return value is required in the case of not finding an interface
 or address, I would suggest a value of -1, as 0 is a valid descriptor
 number.

 (In both cases, the variable "sock" can be declared at the point of first
 use.  In case (b), the return status can be hard-coded - there is no need
 to use "sock".)

 getLocalAddress(): as remote_endpoint is not being shared outside this
 method, a shared_ptr is not needed - a scoped_ptr eliminates the overhead
 of reference counting.


 '''src/lib/dhcp/tests/iface_mgr_unittest.cc'''[[BR]]
 As a general rule, I suggest that the pointer to !NakedIfaceMgr be
 encapsulated in a scoped_ptr and the "delete" at the end of the method
 removed.  Although OK now, if the code is subsequenly edited to include an
 ASSERT_XXX test, a memory leak could ensue.

 socketsFromIface: the comment says that the v4 sicket ''(sic)'' be bound
 to the 10548 port, yet the code binds it to 10547.

 Suggest that the "magic" port numbers 10547 and 10548 be coded as symbolic
 constants.

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


More information about the bind10-tickets mailing list