[2983] Changes after review:

- hooks documentation updated
 - dhcp4 message logs updated
 - long lines wrapped in dhcp4_srv.cc
 - unit-test names now use camel notation
 - Pkt4::bufferOut_ renamed to buffer_out_

src/bin/dhcp4/dhcp4_hooks.dox

@@ -78,16 +78,16 @@ packet processing. Hook points that are not specific to packet processing
    - name: @b query4, type: isc::dhcp::Pkt4Ptr, direction: <b>in/out</b>
 
  - @b Description: this callout is executed when an incoming DHCPv4
-   packet is received and its content is parsed. The sole argument -
+   packet is received and its content has been parsed. The sole
-   query4 - contains a pointer to an isc::dhcp::Pkt4 object that contains
+   argument - query4 - contains a pointer to an isc::dhcp::Pkt4 object
-   all information regarding incoming packet, including its source and
+   that contains all information regarding incoming packet, including
-   destination addresses, interface over which it was received, a list
+   its source and destination addresses, interface over which it was
-   of all options present within and relay information.  All fields of
+   received, a list of all options present within and relay
-   the Pkt4 object can be modified at this time, except data_. (data_
+   information.  All fields of the Pkt4 object can be modified at this
-   contains the incoming packet as raw buffer. By the time this hook is
+   time, except data_. (By the time this hook is reached, the contents
-   reached, that information has already parsed and is available though
+   of the data_ field has been already parsed and stored in other
-   other fields in the Pkt4 object.  For this reason, it doesn't make
+   fields. Therefore, the modification in the data_ field has no
-   sense to modify it.)
+   effect.)
 
  - <b>Skip flag action</b>: If any callout sets the skip flag, the server will
    drop the packet and start processing the next one.  The reason for the drop
@@ -120,18 +120,18 @@ packet processing. Hook points that are not specific to packet processing
    - name: @b lease4, type: isc::dhcp::Lease4Ptr, direction: <b>in/out</b>
 
  - @b Description: this callout is executed after the server engine
-   has selected a lease for client's request but before the lease
+   has selected a lease for client's request but before the lease has
-   has been inserted into the database. Any modifications made to the
+   been inserted into the database. Any modifications made to the
-   isc::dhcp::Lease4 object will be stored in the lease's record in the
+   isc::dhcp::Lease4 object will be stored in the lease's record in
-   database. The callout should make sure that any modifications are
+   the database. The callout should sanity check all modifications as
-   sanity checked as the server will use that data as is with no further
+   the server will use that data as is with no further checking.\n\n
-   checking.\n\n The server processes lease requests for DISCOVER and
+   The server processes lease requests for DISCOVER and REQUEST in a
-   REQUEST in a very similar way. The only major difference is that
+   very similar way. The only major difference is that for DISCOVER
-   for DISCOVER the lease is just selected, but not inserted into
+   the lease is just selected, but not inserted into the database.  It
-   the database.  It is possible to distinguish between DISCOVER and
+   is possible to distinguish between DISCOVER and REQUEST by checking
-   REQUEST by checking value of the fake_allocation flag: a value of true
+   value of the fake_allocation flag: a value of true indicates that the
-   means that the lease won't be inserted into the database (DISCOVER),
+   lease won't be inserted into the database (DISCOVER), a value of
-   a value of false means that it will (REQUEST).
+   false indicates that it will (REQUEST).
 
  - <b>Skip flag action</b>: If any callout installed on 'lease4_select'
    sets the skip flag, the server will not assign any lease. Packet
@@ -178,11 +178,11 @@ packet processing. Hook points that are not specific to packet processing
    - name: @b response4, type: isc::dhcp::Pkt4Ptr, direction: <b>in/out</b>
 
  - @b Description: this callout is executed when server's response
-   is about to be send back to the client. The sole argument - response4 -
+   is about to be sent back to the client. The sole argument - response4 -
    contains a pointer to an isc::dhcp::Pkt4 object that contains the
-   packet, with set source and destination addresses, interface over which
+   packet, with source and destination addresses set, interface over which
-   it will be send, list of all options and relay information.  All fields
+   it will be sent, and a list of all options and relay information.  All fields
-   of the Pkt4 object can be modified at this time, except bufferOut_.
+   of the Pkt4 object can be modified at this time, except buffer_out_.
    (This is scratch space used for constructing the packet after all
    pkt4_send callouts are complete, so any changes to that field will
    be overwritten.)
@@ -198,11 +198,11 @@ packet processing. Hook points that are not specific to packet processing
    - name: @b response4, type: isc::dhcp::Pkt4Ptr, direction: <b>in/out</b>
 
  - @b Description: this callout is executed when server's response
-   is about to be send back to the client. The sole argument - response4 -
+   is about to be sent back to the client. The sole argument - response4 -
    contains a pointer to an isc::dhcp::Pkt4 object that contains the
-   packet, with set source and destination addresses, interface over which
+   packet, with source and destination addresses set, interface over which
-   it will be send, list of all options and relay information. The raw
+   it will be sent, and a list of all options and relay information. The raw
-   on-wire form is already prepared in bufferOut_ (see isc::dhcp::Pkt4::getBuffer())
+   on-wire form is already prepared in buffer_out_ (see isc::dhcp::Pkt4::getBuffer())
    It doesn't make any sense to modify packet fields or options content
    at this time, because they were already used to construct on-wire buffer.
 

src/bin/dhcp4/dhcp4_messages.mes

@@ -82,13 +82,11 @@ setting of the flag by a callout instructs the server to drop the packet.
 Server completed all the processing (e.g. may have assigned, updated
 or released leases), but the response will not be send to the client.
 
-% DHCP4_HOOK_LEASE4_RELEASE_SKIP DHCPv6 lease was not released because a callout set the skip flag.
+% DHCP4_HOOK_LEASE4_RELEASE_SKIP DHCPv4 lease was not released because a callout set the skip flag.
 This debug message is printed when a callout installed on lease4_release
 hook point set the skip flag. For this particular hook point, the
 setting of the flag by a callout instructs the server to not release
-a lease. If client requested release of multiples leases (by sending
+a lease.
-multiple IA options), the server will retains this particular lease and
-will proceed with other renewals as usual.
 
 % DHCP4_HOOK_PACKET_RCVD_SKIP received DHCPv4 packet was dropped, because a callout set the skip flag.
 This debug message is printed when a callout installed on the pkt4_receive
@@ -158,8 +156,8 @@ This is a general catch-all message indicating that the processing of a
 received packet failed.  The reason is given in the message.  The server
 will not send a response but will instead ignore the packet.
 
-% DHCP4_PACKET_DROP_NO_TYPE dropped packet received on interface %1: does not have msg-type option
+% DHCP4_PACKET_DROP_NO_TYPE packet received on interface %1 dropped, because of missing msg-type option
-THis is a debug message informing that incoming DHCPv4 packet did not
+This is a debug message informing that incoming DHCPv4 packet did not
 have mandatory DHCP message type option and thus was dropped.
 
 % DHCP4_PACKET_RECEIVED %1 (type %2) packet received on interface %3

src/bin/dhcp4/dhcp4_srv.cc

@@ -199,7 +199,8 @@ Dhcpv4Srv::run() {
 
         // The packet has just been received so contains the uninterpreted wire
         // data; execute callouts registered for buffer4_receive.
-        if (HooksManager::getHooksManager().calloutsPresent(Hooks.hook_index_buffer4_receive_)) {
+        if (HooksManager::getHooksManager()
+            .calloutsPresent(Hooks.hook_index_buffer4_receive_)) {
             CalloutHandlePtr callout_handle = getCalloutHandle(query);
 
             // Delete previously set arguments
@@ -209,7 +210,8 @@ Dhcpv4Srv::run() {
             callout_handle->setArgument("query4", query);
 
             // Call callouts
-            HooksManager::callCallouts(Hooks.hook_index_buffer4_receive_, *callout_handle);
+            HooksManager::callCallouts(Hooks.hook_index_buffer4_receive_,
+                                       *callout_handle);
 
             // Callouts decided to skip the next processing step. The next
             // processing step would to parse the packet, so skip at this
@@ -255,7 +257,7 @@ Dhcpv4Srv::run() {
             .arg(type)
             .arg(query->toText());
 
-        // Let's execute all callouts registered for packet_received
+        // Let's execute all callouts registered for pkt4_receive
         if (HooksManager::calloutsPresent(hook_index_pkt4_receive_)) {
             CalloutHandlePtr callout_handle = getCalloutHandle(query);
 
@@ -352,7 +354,7 @@ Dhcpv4Srv::run() {
         // Specifies if server should do the packing
         bool skip_pack = false;
 
-        // Execute all callouts registered for packet6_send
+        // Execute all callouts registered for pkt4_send
         if (HooksManager::calloutsPresent(hook_index_pkt4_send_)) {
             CalloutHandlePtr callout_handle = getCalloutHandle(query);
 
@@ -391,8 +393,9 @@ Dhcpv4Srv::run() {
             // Now all fields and options are constructed into output wire buffer.
             // Option objects modification does not make sense anymore. Hooks
             // can only manipulate wire buffer at this stage.
-            // Let's execute all callouts registered for buffer6_send
+            // Let's execute all callouts registered for buffer4_send
-            if (HooksManager::getHooksManager().calloutsPresent(Hooks.hook_index_buffer4_send_)) {
+            if (HooksManager::getHooksManager()
+                .calloutsPresent(Hooks.hook_index_buffer4_send_)) {
                 CalloutHandlePtr callout_handle = getCalloutHandle(query);
 
                 // Delete previously set arguments
@@ -402,13 +405,15 @@ Dhcpv4Srv::run() {
                 callout_handle->setArgument("response4", rsp);
 
                 // Call callouts
-                HooksManager::callCallouts(Hooks.hook_index_buffer4_send_, *callout_handle);
+                HooksManager::callCallouts(Hooks.hook_index_buffer4_send_,
+                                           *callout_handle);
 
                 // Callouts decided to skip the next processing step. The next
                 // processing step would to parse the packet, so skip at this
                 // stage means drop.
                 if (callout_handle->getSkip()) {
-                    LOG_DEBUG(dhcp4_logger, DBG_DHCP4_HOOKS, DHCP4_HOOK_BUFFER_SEND_SKIP);
+                    LOG_DEBUG(dhcp4_logger, DBG_DHCP4_HOOKS,
+                              DHCP4_HOOK_BUFFER_SEND_SKIP);
                     continue;
                 }
 
@@ -847,7 +852,7 @@ Dhcpv4Srv::processDiscover(Pkt4Ptr& discover) {
 Pkt4Ptr
 Dhcpv4Srv::processRequest(Pkt4Ptr& request) {
 
-    /// @todo Uncomment this
+    /// @todo Uncomment this (see ticket #3116)
     // sanityCheck(request, MANDATORY);
 
     Pkt4Ptr ack = Pkt4Ptr
@@ -873,7 +878,7 @@ Dhcpv4Srv::processRequest(Pkt4Ptr& request) {
 void
 Dhcpv4Srv::processRelease(Pkt4Ptr& release) {
 
-    /// @todo Uncomment this
+    /// @todo Uncomment this (see ticket #3116)
     // sanityCheck(release, MANDATORY);
 
     // Try to find client-id
@@ -919,8 +924,9 @@ Dhcpv4Srv::processRelease(Pkt4Ptr& release) {
 
         bool skip = false;
 
-        // Execute all callouts registered for packet6_send
+        // Execute all callouts registered for lease4_release
-        if (HooksManager::getHooksManager().calloutsPresent(Hooks.hook_index_lease4_release_)) {
+        if (HooksManager::getHooksManager()
+            .calloutsPresent(Hooks.hook_index_lease4_release_)) {
             CalloutHandlePtr callout_handle = getCalloutHandle(release);
 
             // Delete all previous arguments
@@ -933,14 +939,16 @@ Dhcpv4Srv::processRelease(Pkt4Ptr& release) {
             callout_handle->setArgument("lease4", lease);
 
             // Call all installed callouts
-            HooksManager::callCallouts(Hooks.hook_index_lease4_release_, *callout_handle);
+            HooksManager::callCallouts(Hooks.hook_index_lease4_release_,
+                                       *callout_handle);
 
             // Callouts decided to skip the next processing step. The next
             // processing step would to send the packet, so skip at this
             // stage means "drop response".
             if (callout_handle->getSkip()) {
                 skip = true;
-                LOG_DEBUG(dhcp4_logger, DBG_DHCP4_HOOKS, DHCP4_HOOK_LEASE4_RELEASE_SKIP);
+                LOG_DEBUG(dhcp4_logger, DBG_DHCP4_HOOKS,
+                          DHCP4_HOOK_LEASE4_RELEASE_SKIP);
             }
         }
 
@@ -976,13 +984,13 @@ Dhcpv4Srv::processRelease(Pkt4Ptr& release) {
 
 void
 Dhcpv4Srv::processDecline(Pkt4Ptr& /* decline */) {
-    /// @todo Implement this.
+    /// @todo Implement this (also see ticket #3116)
 }
 
 Pkt4Ptr
 Dhcpv4Srv::processInform(Pkt4Ptr& inform) {
 
-    /// @todo Implement this for real.
+    /// @todo Implement this for real. (also see ticket #3116)
     return (inform);
 }
 
@@ -1036,7 +1044,7 @@ Dhcpv4Srv::selectSubnet(const Pkt4Ptr& question) {
 
     /// @todo Implement getSubnet4(interface-name)
 
-    // Let's execute all callouts registered for packet_received
+    // Let's execute all callouts registered for subnet4_select
     if (HooksManager::calloutsPresent(hook_index_subnet4_select_)) {
         CalloutHandlePtr callout_handle = getCalloutHandle(question);
 
@@ -1046,16 +1054,19 @@ Dhcpv4Srv::selectSubnet(const Pkt4Ptr& question) {
         // Set new arguments
         callout_handle->setArgument("query4", question);
         callout_handle->setArgument("subnet4", subnet);
-        callout_handle->setArgument("subnet4collection", CfgMgr::instance().getSubnets4());
+        callout_handle->setArgument("subnet4collection",
+                                    CfgMgr::instance().getSubnets4());
 
         // Call user (and server-side) callouts
-        HooksManager::callCallouts(hook_index_subnet4_select_, *callout_handle);
+        HooksManager::callCallouts(hook_index_subnet4_select_,
+                                   *callout_handle);
 
         // Callouts decided to skip this step. This means that no subnet will be
-        // selected. Packet processing will continue, but it will be severly limited
+        // selected. Packet processing will continue, but it will be severly
-        // (i.e. only global options will be assigned)
+        // limited (i.e. only global options will be assigned)
         if (callout_handle->getSkip()) {
-            LOG_DEBUG(dhcp4_logger, DBG_DHCP4_HOOKS, DHCP4_HOOK_SUBNET4_SELECT_SKIP);
+            LOG_DEBUG(dhcp4_logger, DBG_DHCP4_HOOKS,
+                      DHCP4_HOOK_SUBNET4_SELECT_SKIP);
             return (Subnet4Ptr());
         }
 

src/bin/dhcp4/tests/dhcp4_srv_unittest.cc

@@ -2138,7 +2138,7 @@ vector<string> HooksDhcpv4SrvTest::callback_argument_names_;
 //
 // Note that the test name does not follow test naming convention,
 // but the proper hook name is "buffer4_receive".
-TEST_F(HooksDhcpv4SrvTest, simple_buffer4_receive) {
+TEST_F(HooksDhcpv4SrvTest, Buffer4ReceiveSimple) {
 
     // Install pkt6_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
@@ -2171,7 +2171,7 @@ TEST_F(HooksDhcpv4SrvTest, simple_buffer4_receive) {
 
 // Checks if callouts installed on buffer4_receive is able to change
 // the values and the parameters are indeed used by the server.
-TEST_F(HooksDhcpv4SrvTest, valueChange_buffer4_receive) {
+TEST_F(HooksDhcpv4SrvTest, buffer4RreceiveValueChange) {
 
     // Install callback that modifies MAC addr of incoming packet
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
@@ -2210,7 +2210,7 @@ TEST_F(HooksDhcpv4SrvTest, valueChange_buffer4_receive) {
 // will cause the server to not parse the packet. Even though the packet is valid,
 // the server should eventually drop it, because there won't be mandatory options
 // (or rather option objects) in it.
-TEST_F(HooksDhcpv4SrvTest, skip_buffer4_receive) {
+TEST_F(HooksDhcpv4SrvTest, buffer4ReceiveSkip) {
 
     // Install pkt6_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
@@ -2237,7 +2237,7 @@ TEST_F(HooksDhcpv4SrvTest, skip_buffer4_receive) {
 //
 // Note that the test name does not follow test naming convention,
 // but the proper hook name is "pkt4_receive".
-TEST_F(HooksDhcpv4SrvTest, simple_pkt4_receive) {
+TEST_F(HooksDhcpv4SrvTest, pkt4ReceiveSimple) {
 
     // Install pkt4_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
@@ -2306,7 +2306,7 @@ TEST_F(HooksDhcpv4SrvTest, valueChange_pkt4_receive) {
 // Checks if callouts installed on pkt4_received is able to delete
 // existing options and that change impacts server processing (mandatory
 // client-id option is deleted, so the packet is expected to be dropped)
-TEST_F(HooksDhcpv4SrvTest, deleteClientId_pkt4_receive) {
+TEST_F(HooksDhcpv4SrvTest, pkt4ReceiveDeleteClientId) {
 
     // Install pkt4_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
@@ -2330,7 +2330,7 @@ TEST_F(HooksDhcpv4SrvTest, deleteClientId_pkt4_receive) {
 
 // Checks if callouts installed on pkt4_received is able to set skip flag that
 // will cause the server to not process the packet (drop), even though it is valid.
-TEST_F(HooksDhcpv4SrvTest, skip_pkt4_receive) {
+TEST_F(HooksDhcpv4SrvTest, pkt4ReceiveSkip) {
 
     // Install pkt4_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
@@ -2355,7 +2355,7 @@ TEST_F(HooksDhcpv4SrvTest, skip_pkt4_receive) {
 
 // Checks if callouts installed on pkt4_send are indeed called and the
 // all necessary parameters are passed.
-TEST_F(HooksDhcpv4SrvTest, simple_pkt4_send) {
+TEST_F(HooksDhcpv4SrvTest, pkt4SendSimple) {
 
     // Install pkt4_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
@@ -2391,7 +2391,7 @@ TEST_F(HooksDhcpv4SrvTest, simple_pkt4_send) {
 
 // Checks if callouts installed on pkt4_send is able to change
 // the values and the packet sent contains those changes
-TEST_F(HooksDhcpv4SrvTest, valueChange_pkt4_send) {
+TEST_F(HooksDhcpv4SrvTest, pkt4SendValueChange) {
 
     // Install pkt4_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
@@ -2428,7 +2428,7 @@ TEST_F(HooksDhcpv4SrvTest, valueChange_pkt4_send) {
 // existing options and that server applies those changes. In particular,
 // we are trying to send a packet without server-id. The packet should
 // be sent
-TEST_F(HooksDhcpv4SrvTest, deleteServerId_pkt4_send) {
+TEST_F(HooksDhcpv4SrvTest, pkt4SendDeleteServerId) {
 
     // Install pkt4_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
@@ -2488,7 +2488,7 @@ TEST_F(HooksDhcpv4SrvTest, skip_pkt4_send) {
 
 // Checks if callouts installed on buffer4_send are indeed called and the
 // all necessary parameters are passed.
-TEST_F(HooksDhcpv4SrvTest, simple_buffer4_send) {
+TEST_F(HooksDhcpv4SrvTest, buffer4SendSimple) {
 
     // Install pkt4_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
@@ -2524,7 +2524,7 @@ TEST_F(HooksDhcpv4SrvTest, simple_buffer4_send) {
 
 // Checks if callouts installed on buffer4_send are indeed called and that
 // the output buffer can be changed.
-TEST_F(HooksDhcpv4SrvTest, change_buffer4_send) {
+TEST_F(HooksDhcpv4SrvTest, buffer4Send) {
 
     // Install pkt4_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
@@ -2553,7 +2553,7 @@ TEST_F(HooksDhcpv4SrvTest, change_buffer4_send) {
 
 // Checks if callouts installed on buffer4_send can set skip flag and that flag
 // causes the packet to not be sent
-TEST_F(HooksDhcpv4SrvTest, skip_buffer4_send) {
+TEST_F(HooksDhcpv4SrvTest, buffer4SendSkip) {
 
     // Install pkt4_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
@@ -2578,7 +2578,7 @@ TEST_F(HooksDhcpv4SrvTest, skip_buffer4_send) {
 
 // This test checks if subnet4_select callout is triggered and reports
 // valid parameters
-TEST_F(HooksDhcpv4SrvTest, subnet4_select) {
+TEST_F(HooksDhcpv4SrvTest, subnet4SelectSimple) {
 
     // Install pkt4_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
@@ -2644,7 +2644,7 @@ TEST_F(HooksDhcpv4SrvTest, subnet4_select) {
 
 // This test checks if callout installed on subnet4_select hook point can pick
 // a different subnet.
-TEST_F(HooksDhcpv4SrvTest, subnet_select_change) {
+TEST_F(HooksDhcpv4SrvTest, subnet4SelectChange) {
 
     // Install a callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
@@ -2704,7 +2704,7 @@ TEST_F(HooksDhcpv4SrvTest, subnet_select_change) {
 // This test verifies that incoming (positive) REQUEST/Renewing can be handled
 // properly and that callout installed on lease4_renew is triggered with
 // expected parameters.
-TEST_F(HooksDhcpv4SrvTest, basic_lease4_renew) {
+TEST_F(HooksDhcpv4SrvTest, lease4RenewSimple) {
 
     const IOAddress addr("192.0.2.106");
     const uint32_t temp_t1 = 50;
@@ -2788,7 +2788,7 @@ TEST_F(HooksDhcpv4SrvTest, basic_lease4_renew) {
 
 // This test verifies that a callout installed on lease4_renew can trigger
 // the server to not renew a lease.
-TEST_F(HooksDhcpv4SrvTest, skip_lease4_renew) {
+TEST_F(HooksDhcpv4SrvTest, lease4RenewSkip) {
 
     const IOAddress addr("192.0.2.106");
     const uint32_t temp_t1 = 50;
@@ -2852,7 +2852,7 @@ TEST_F(HooksDhcpv4SrvTest, skip_lease4_renew) {
 }
 
 // This test verifies that valid RELEASE triggers lease4_release callouts
-TEST_F(HooksDhcpv4SrvTest, basic_lease4_release) {
+TEST_F(HooksDhcpv4SrvTest, lease4ReleaseSimple) {
 
     const IOAddress addr("192.0.2.106");
     const uint32_t temp_t1 = 50;
@@ -2937,7 +2937,7 @@ TEST_F(HooksDhcpv4SrvTest, basic_lease4_release) {
 
 // This test verifies that skip flag returned by a callout installed on the
 // lease4_release hook point will keep the lease
-TEST_F(HooksDhcpv4SrvTest, skip_lease4_release) {
+TEST_F(HooksDhcpv4SrvTest, lease4ReleaseSkip) {
 
     const IOAddress addr("192.0.2.106");
     const uint32_t temp_t1 = 50;
@@ -2998,9 +2998,6 @@ TEST_F(HooksDhcpv4SrvTest, skip_lease4_release) {
     // @todo: Uncomment this once trac2592 is implemented
     //Lease4Collection leases = LeaseMgrFactory::instance().getLease4(*client_id_);
     //EXPECT_EQ(leases.size(), 1);
-
 }
 
-
-
 } // end of anonymous namespace

src/lib/dhcp/pkt4.cc

@@ -33,7 +33,7 @@ namespace dhcp {
 const IOAddress DEFAULT_ADDRESS("0.0.0.0");
 
 Pkt4::Pkt4(uint8_t msg_type, uint32_t transid)
-     :bufferOut_(DHCPV4_PKT_HDR_LEN),
+     :buffer_out_(DHCPV4_PKT_HDR_LEN),
       local_addr_(DEFAULT_ADDRESS),
       remote_addr_(DEFAULT_ADDRESS),
       iface_(""),
@@ -58,7 +58,7 @@ Pkt4::Pkt4(uint8_t msg_type, uint32_t transid)
 }
 
 Pkt4::Pkt4(const uint8_t* data, size_t len)
-     :bufferOut_(0), // not used, this is RX packet
+     :buffer_out_(0), // not used, this is RX packet
       local_addr_(DEFAULT_ADDRESS),
       remote_addr_(DEFAULT_ADDRESS),
       iface_(""),
@@ -111,25 +111,25 @@ Pkt4::pack() {
     try {
         size_t hw_len = hwaddr_->hwaddr_.size();
 
-        bufferOut_.writeUint8(op_);
+        buffer_out_.writeUint8(op_);
-        bufferOut_.writeUint8(hwaddr_->htype_);
+        buffer_out_.writeUint8(hwaddr_->htype_);
-        bufferOut_.writeUint8(hw_len < MAX_CHADDR_LEN ? 
+        buffer_out_.writeUint8(hw_len < MAX_CHADDR_LEN ?
                               hw_len : MAX_CHADDR_LEN);
-        bufferOut_.writeUint8(hops_);
+        buffer_out_.writeUint8(hops_);
-        bufferOut_.writeUint32(transid_);
+        buffer_out_.writeUint32(transid_);
-        bufferOut_.writeUint16(secs_);
+        buffer_out_.writeUint16(secs_);
-        bufferOut_.writeUint16(flags_);
+        buffer_out_.writeUint16(flags_);
-        bufferOut_.writeUint32(ciaddr_);
+        buffer_out_.writeUint32(ciaddr_);
-        bufferOut_.writeUint32(yiaddr_);
+        buffer_out_.writeUint32(yiaddr_);
-        bufferOut_.writeUint32(siaddr_);
+        buffer_out_.writeUint32(siaddr_);
-        bufferOut_.writeUint32(giaddr_);
+        buffer_out_.writeUint32(giaddr_);
 
 
         if (hw_len <= MAX_CHADDR_LEN) {
             // write up to 16 bytes of the hardware address (CHADDR field is 16
             // bytes long in DHCPv4 message).
-            bufferOut_.writeData(&hwaddr_->hwaddr_[0], 
+            buffer_out_.writeData(&hwaddr_->hwaddr_[0],
-                                 (hw_len < MAX_CHADDR_LEN ? 
+                                 (hw_len < MAX_CHADDR_LEN ?
                                   hw_len : MAX_CHADDR_LEN) );
             hw_len = MAX_CHADDR_LEN - hw_len;
         } else {
@@ -138,20 +138,20 @@ Pkt4::pack() {
 
         // write (len) bytes of padding
         vector<uint8_t> zeros(hw_len, 0);
-        bufferOut_.writeData(&zeros[0], hw_len);
+        buffer_out_.writeData(&zeros[0], hw_len);
-        // bufferOut_.writeData(chaddr_, MAX_CHADDR_LEN);
+        // buffer_out_.writeData(chaddr_, MAX_CHADDR_LEN);
 
-        bufferOut_.writeData(sname_, MAX_SNAME_LEN);
+        buffer_out_.writeData(sname_, MAX_SNAME_LEN);
-        bufferOut_.writeData(file_, MAX_FILE_LEN);
+        buffer_out_.writeData(file_, MAX_FILE_LEN);
 
         // write DHCP magic cookie
-        bufferOut_.writeUint32(DHCP_OPTIONS_COOKIE);
+        buffer_out_.writeUint32(DHCP_OPTIONS_COOKIE);
 
-        LibDHCP::packOptions(bufferOut_, options_);
+        LibDHCP::packOptions(buffer_out_, options_);
 
         // add END option that indicates end of options
         // (End option is very simple, just a 255 octet)
-         bufferOut_.writeUint8(DHO_END);
+         buffer_out_.writeUint8(DHO_END);
      } catch(const Exception& e) {
         // An exception is thrown and message will be written to Logger
          isc_throw(InvalidOperation, e.what());
@@ -259,7 +259,7 @@ void Pkt4::setType(uint8_t dhcp_type) {
 }
 
 void Pkt4::repack() {
-    bufferOut_.writeData(&data_[0], data_.size());
+    buffer_out_.writeData(&data_[0], data_.size());
 }
 
 std::string

src/lib/dhcp/pkt4.h

@@ -70,7 +70,7 @@ public:
     ///
     /// Prepares on-wire format of message and all its options.
     /// Options must be stored in options_ field.
-    /// Output buffer will be stored in bufferOut_.
+    /// Output buffer will be stored in buffer_out_.
     ///
     /// @throw InvalidOperation if packing fails
     void
@@ -103,7 +103,7 @@ public:
     ///
     /// This is mostly a diagnostic function. It is being used for sending
     /// received packet. Received packet is stored in bufferIn_, but
-    /// transmitted data is stored in bufferOut_. If we want to send packet
+    /// transmitted data is stored in buffer_out_. If we want to send packet
     /// that we just received, a copy between those two buffers is necessary.
     void repack();
 
@@ -312,7 +312,7 @@ public:
     ///
     /// @return reference to output buffer
     isc::util::OutputBuffer&
-    getBuffer() { return (bufferOut_); };
+    getBuffer() { return (buffer_out_); };
 
     /// @brief Add an option.
     ///
@@ -490,7 +490,7 @@ public:
     /// @throw isc::Unexpected if timestamp update failed
     void updateTimestamp();
 
-    /// output buffer (used during message transmission)
+    /// Output buffer (used during message transmission)
     ///
     /// @warning This public member is accessed by derived
     /// classes directly. One of such derived classes is
@@ -502,12 +502,13 @@ public:
     /// but we expect to also have them in Python, so any accesibility
     /// methods would overly complicate things here and degrade
     /// performance).
-    isc::util::OutputBuffer bufferOut_;
+    isc::util::OutputBuffer buffer_out_;
 
-    /// that's the data of input buffer used in RX packet. Note that
+    /// @brief That's the data of input buffer used in RX packet.
-    /// InputBuffer does not store the data itself, but just expects that
+    ///
-    /// data will be valid for the whole life of InputBuffer. Therefore we
+    /// @note Note that InputBuffer does not store the data itself, but just
-    /// need to keep the data around.
+    /// expects that data will be valid for the whole life of InputBuffer.
+    /// Therefore we need to keep the data around.
     ///
     /// @warning This public member is accessed by derived
     /// classes directly. One of such derived classes is

src/lib/dhcpsrv/alloc_engine.cc

@@ -464,6 +464,7 @@ Lease4Ptr AllocEngine::renewLease4(const SubnetPtr& subnet,
     // Let's keep the old data. This is essential if we are using memfile
     // (the lease returned points directly to the lease4 object in the database)
     // We'll need it if we want to skip update (i.e. roll back renewal)
+    /// @todo: remove this once #3083 is implemented
     Lease4 old_values = *lease;
 
     lease->subnet_id_ = subnet->getID();
@@ -513,6 +514,7 @@ Lease4Ptr AllocEngine::renewLease4(const SubnetPtr& subnet,
     }
     if (skip) {
         // Rollback changes (really useful only for memfile)
+        /// @todo: remove this once #3083 is implemented
         *lease = old_values;
     }
 

tests/tools/perfdhcp/perf_pkt4.cc

@@ -40,7 +40,7 @@ PerfPkt4::rawPack() {
                                options_,
                                getTransidOffset(),
                                getTransid(),
-                               bufferOut_));
+                               buffer_out_));
 }
 
 bool