BIND 10 #2237: Pool/Lease Configuration - IPv4

[BIND 10 Development]

#2237: Pool/Lease Configuration - IPv4
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  tomek
  stephen                            |                Status:  reviewing
                       Type:  task   |             Milestone:  Sprint-
                   Priority:         |  DHCP-20121018
  medium                             |            Resolution:
                  Component:         |             Sensitive:  0
  dhcpconf                           |           Sub-Project:  DHCP
                   Keywords:         |  Estimated Difficulty:  0
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by stephen):

 * owner:  stephen => tomek


Comment:

 Reviewed commit 46c03d26e27417bdce132b90eef18686443bd5a7

 '''General'''[[BR]]
 In a lot of calls, "prefix length" is passed as a uint8_t.  Unless there
 is a compelling reason for this, I would have thought that an "unsigned
 int" - using the natural word length of the machine - would be faster.

 '''src/lib/dhcp/addr_utilities.cc'''[[BR]]
 The .h file doesn't seem to have been updated with the function
 definitions.

 Instead of using "static", put masks and bitMask into the anonymous
 namespace.

 firstAddrInPrefix4: require spaces around the operator ">".  Also spaces
 around "-" in "32-len".

 {first,last}AddrInPrefix4: In both cases where "mask" is referenced, it is
 always "mask[32 - len]".  Why not reverse the order of the elements of in
 the initialization of "mask" and just refer to it as "mask[len]"?

 There are no unit tests for the new xxx4 functions.

 '''src/lib/dhcp/cfgmgr.h'''[[BR]]
 There is a definition for removeSubnets4() but not for removeSubnets6().

 Similarly, there is a getSubnet6() by interface ID, but not a getSubnet4()
 by interface ID.

 '''src/lib/dhcp/tests/cfgmgr_unittest.cc'''[[BR]]
 test subnet4: Would add a comment to the test recording (a) adding more
 subnets and checking them and (b) attempting to obtain an address not in
 the subnet.

 test subnet4: Would also test what happens if you attempt to add a subnet
 that is already present.

 (Same comments go for the subnet6 test.)

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