BIND 10 #992: DHCP IPv4 server as a dummy BIND 10 component

[BIND 10 Development]

#992: DHCP IPv4 server as a dummy BIND 10 component
-------------------------------------+-------------------------------------
                   Reporter:  shane  |                 Owner:  tomek
                       Type:         |                Status:  reviewing
  enhancement                        |             Milestone:  Sprint-
                   Priority:  major  |  DHCP-20111221
                  Component:  dhcp4  |            Resolution:
                   Keywords:         |             Sensitive:  0
            Defect Severity:  N/A    |           Sub-Project:  DHCP
Feature Depending on Ticket:         |  Estimated Difficulty:  0.0
        Add Hours to Ticket:  0      |           Total Hours:  0
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by stephen):

 * owner:  stephen => tomek


Comment:

 Reviewed commit d8cd199a66645341270081a7f409c557e596099b

 '''src/bin/dhcp4/dhcp4.spec'''[[BR]]
 Just a comment that we will eventually need a list of interfaces.  However
 it is fine for now.

 '''src/bin/dhcp4/dhcp4_srv.h'''[[BR]]
 Dhcp4Srv contructor: port argument needs Doxygen entry in the header.

 processXxx(): I feel that the arguments should be passed by reference to
 avoid the overhead of a reference count increase during the class.
 (Although I know Jinmei disagrees with me on this one.)

 shutdown member variable should be shutdown_

 '''src/bin/dhcp4/dhcp4_srv.cc'''[[BR]]
 run(): In the longer term we need to address how to shutdown the server if
 it is in the receive4() call when the shutdown request is received.

 '''src/bin/dhcp4/tests/dhcp4_srv_unittest.cc'''[[BR]]
 The comment should be '"naked" DHCP4 server', not '"naked" Interface
 Manager'

 In NakedDhcpv4Srv(), if you don't change the function signatures you don't
 need to duplicate base class methods.  (In this case, the derived class
 receives the parameters by value and passes them to the base class by
 reference; if it were to received the parameters by reference the methods
 would be no-op and could be omitted.)

 "basic" test: there is no need to check is srv is non-null before
 attempting the deletion.

 processXxx tests: Do we want to do an ASSERT_NO_THROW around the "delete
 src"?

 '''src/bin/dhcp4/tests/dhcp4_unittests.cc'''[[BR]]
 "return result;" should be "return (result);" according to the coding
 guidelines.

 '''src/lib/dhcp/iface_mgr.*'''[[BR]]
 I didn't review these files or tests as they receintly appeared in another
 ticket that I reviewed and I understand there is a lot of cross-over
 between tickets at the moment.

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