BIND 10 #2414: Use AllocationEngine to actually handle v6 clients' messages

BIND 10 Development do-not-reply at isc.org
Fri Nov 9 15:03:49 UTC 2012


#2414: Use AllocationEngine to actually handle v6 clients' messages
-------------------------------------+-------------------------------------
                   Reporter:  tomek  |                 Owner:  stephen
                       Type:  task   |                Status:  reviewing
                   Priority:         |             Milestone:  Sprint-
  medium                             |  DHCP-20121115
                  Component:  dhcp6  |            Resolution:
                   Keywords:         |             Sensitive:  0
            Defect Severity:  N/A    |           Sub-Project:  DHCP
Feature Depending on Ticket:         |  Estimated Difficulty:  0
        Add Hours to Ticket:  0      |           Total Hours:  0
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by marcin):

 Since there have been a couple of changes since f255... commit I am
 reviewing it since that commit up to 4067c8....

 '''dhcp6_srv_unittest.cc'''
 advertiseOptions: There are some comments needed in the new code that sets
 up the Solicit message. In particular: why do we have to set the link
 local address: fe80::abcd? What are the magic numbers 234, 1500, 3000?
 Also extra space before
 {{{
 Pkt6Ptr sol = Pkt6Ptr(...);
 }}}

 '''dhcp6_srv.cc'''
 Dhcp6Srv: Is there any reason for not initializing the Allocation Engine
 in the constructor's initialization list but in the constructor's body?
 Does it have to be initialized after other objects, i.e. logger, option
 defs etc?

 '''dhcp6_srv.h'''
 Dhcpv6Srv header: There is nothing in Dhcpv6Srv that makes it singleton
 class. Doxygen comment should be changed.

 '''dhcp_srv_unittest.cc'''
 Dhcp6SrvTest constructor: Technically the subnet_ and pool_ could be
 initialized with constructor's initialization list not in the body.

 checkIAAddr: It would be good to comment why we compare:
 {{{
 EXPECT_EQ(expected_addr.toText(), addr->getAddress().toText());
 }}}

 instead of
 {{{
 EXPECT_EQ(expected_addr, addr->getAddress());
 }}}
 I presume this is because the '''==''' operator is not working properly
 for IOAddress because of the presence of uint32 operator?

 checkIAAddr: the addr parameter could be passed by '''const reference'''.

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


More information about the bind10-tickets mailing list