BIND 10 #3191: Kea4: siaddr field must be configurable

[BIND 10 Development]

#3191: Kea4: siaddr field must be configurable
-------------------------------------+-------------------------------------
            Reporter:  tomek         |                        Owner:  tomek
                Type:  defect        |                       Status:
            Priority:  medium        |  reviewing
           Component:  dhcp4         |                    Milestone:
            Keywords:                |  Sprint-DHCP-20131016
           Sensitive:  0             |                   Resolution:
         Sub-Project:  DHCP          |                 CVSS Scoring:
Estimated Difficulty:  0             |              Defect Severity:  N/A
         Total Hours:  0             |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by marcin):

 * owner:  marcin => tomek


Comment:

 Reviewed commit: e98ccfe63db14be0d9a027797ffd75885b296969

 '''doc/guide/bind10-guide.xml'''
 Typo: !''form the TFTP server!''. It should be !''from the TFTP server!''.

 '''src/bin/dhcp4/config_parser.cc'''
 Why do you use a raw pointer (subnet4) instead shared_ptr (subnet_) in
 invocations to setSiaddr?
 {{{
         Subnet4* subnet4 = new Subnet4(addr, len, t1, t2, valid);
         subnet_.reset(subnet4);

         // Try global value first
         try {
             string next_server = globalContext()->string_values_->getParam
 ("next-server");
             subnet4->setSiaddr(IOAddress(next_server));
         } catch (const DhcpConfigError&) {
             // don't care. next_server is optional. We can live without it
         }

         // Try subnet specific value if it's available
         try {
             string next_server = string_values_->getParam("next-server");
             subnet4->setSiaddr(IOAddress(next_server));
         } catch (const DhcpConfigError&) {
             // don't care. next_server is optional. We can live without it
         }

 }}}


 '''src/bin/dhcp4/dhcp4.spec'''
 Trivial: Line 229-231 - odd alignment of lines

 '''src/bin/dhcp4/tests/config_parser_unittest.cc'''
 There are no negative tests for the siaddr. If you think it is too much
 burden, please add a todo comment. In particular I am thinking that
 invalid next-server (say abc) should result in configuration failure.

 '''src/lib/dhcpsrv/subnet.cc'''
 setSiaddr: Can you replace !''addr!'' with !''address!'' in the exception
 string.
 Spurious commentary in lines 163 and 164

 '''src/lib/dhcpsrv/subnet.h'''
 Capital letters are encouraged. :-)

 '''src/lib/dhcpsrv/tests/subnet_unittest.cc'''
 I think that the description of the siaddr test is misleading. We barely
 check that we can set and get the value of the siaddr. Also typo
 !''handle!''. Should be !''handled!''.

 That change deserves a !ChangeLog.

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