BIND 10 #2559: Obtain DHCP lease database parameters from configuration database

[BIND 10 Development]

#2559: Obtain DHCP lease database parameters from configuration database
-------------------------------------+-------------------------------------
            Reporter:  stephen       |                        Owner:
                Type:  defect        |  stephen
            Priority:  medium        |                       Status:
           Component:  dhcpdb        |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-DHCP-20130122
         Sub-Project:  DHCP          |                   Resolution:
Estimated Difficulty:  0             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by marcin):

 * owner:  marcin => stephen


Comment:

 Reviewed commit 79f0d62ef03b3774a0b0c6237eff78af821bdbc4

 General: the modified and new files should have copyright headers updated
 to 2013:
 - src/bin/dhcp6/ctrl_dhcp6_srv.cc
 - src/bin/dhcp6/ctrl_dhcp6_srv.h
 - src/bin/dhcp6/dhcp6_srv.h
 - src/bin/dhcp6/main.cc
 - src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
 - src/lib/dhcpsrv/dbaccess_parser.cc
 - src/lib/dhcpsrv/lease_mgr_factory.cc
 - src/lib/dhcpsrv/memfile_lease_mgr.cc
 - src/lib/dhcpsrv/dbaccess_parser.h
 - src/lib/dhcpsrv/tests/dbaccess_parser_unittest.cc

 '''src/bin/dhcp4/config_parser.cc'''
 Line 1386: since the '''current_parser''' variable has been introduced it
 makes sense to use it further in the code rather than
 '''config_pair.first''', e.g.
 {{{
 ParserPtr parser(createGlobalDhcp4ConfigParser(config_pair.first));
 }}}
 can be replaced with
 {{{
 ParserPtr parser(createGlobalDhcp4ConfigParser(current_parser));
 }}}

 The same applies to line 1402.

 '''src/bin/dhcp6/config_parser.cc'''
 Line 1421 and line 1437: the same comments as for
 '''src/bin/dhcp4/config_parser.cc'''

 DbAccessParser::build: suggest that you consider using std::swap() to
 assign contents of the values_copy to values_ when the parsing is
 successful. In theory it should be faster than using the assignment
 operator.

 DbAccessParser::commit: spurious space before '''dbaccess.empty()''' in
 line 91

 '''src/lib/dhcpsrv/dbaccess_parser.h'''

 !ConfigPair definition (line 48): this type could be declared private
 within this class as it is not used externally. Alternatively, could this
 be declared in the dhcp_config_parser.h and become common for all parsers?

 In the header of '''getDbAccessParameters''' function:
 s/corrected/correctly.

 '''src/lib/dhcpsrv/dhcpsrv_messages.mes'''
 Suggest that the following warning message:
 {{{
 % DHCPSRV_MEMFILE_WARNING using early version of memfile - leases will be
 lost after a restart
 }}}
 is modified to
 {{{
 % DHCPSRV_MEMFILE_WARNING using early version of memfile lease database -
 leases will be lost after a restart
 }}}
 as the bare word !''memfile!'' is vague.

 '''src/lib/dhcpsrv/tests/dbaccess_parser_unittest.cc'''

 There are no tests for commit() function.

 Do you need to:
 {{{
 #include <fstream>
 #include <iostream>
 #include <sstream>
 #include <arpa/inet.h>
 }}}
 ?

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