[5457] Addressed review comments.

src/bin/dhcp4/ctrl_dhcp4_srv.cc

@@ -39,6 +39,7 @@ struct CtrlDhcp4Hooks {
     CtrlDhcp4Hooks() {
         hooks_index_dhcp4_srv_configured_ = HooksManager::registerHook("dhcp4_srv_configured");
     }
+
 };
 
 // Declare a Hooks object. As this is outside any function or method, it
@@ -653,7 +654,7 @@ ControlledDhcpv4Srv::processConfig(isc::data::ConstElementPtr config) {
     if (HooksManager::calloutsPresent(Hooks.hooks_index_dhcp4_srv_configured_)) {
         CalloutHandlePtr callout_handle = HooksManager::createCalloutHandle();
 
-        callout_handle->setArgument("io_service", srv->getIOService());
+        callout_handle->setArgument("io_context", srv->getIOService());
         callout_handle->setArgument("json_config", config);
         callout_handle->setArgument("server_config", CfgMgr::instance().getStagingCfg());
 

src/bin/dhcp4/dhcp4_hooks.dox

@@ -48,15 +48,15 @@ to the end of this list.
 
  @subsection dhcp4HooksDhcpv4SrvConfigured dhcp4_srv_configured
  - @b Arguments:
-   - name: @b io_service, type: isc::asiolink::IOServicePtr, direction: <b>in</b>
+   - name: @b io_context, type: isc::asiolink::IOServicePtr, direction: <b>in</b>
    - name: @b json_config, type: isc::data::ConstElementPtr, direction: <b>in</b>
    - name: @b server_config, type: isc::dhcp::SrvConfigPtr, direction: <b>in</b>
 
  - @b Description: this callout is executed when the server has completed
    its (re)configuration. The server provides received and parsed configuration
-   structures to the hook library. It also provides a pointer to the io_service
+   structures to the hook library. It also provides a pointer to the IOService
    object which is used by the server to run asynchronous operations. The hooks
-   libraries can use this IO service object to schedule asynchronous tasks which
+   libraries can use this IOService object to schedule asynchronous tasks which
    are triggered by the DHCP server's main loop. The hook library should hold the
    provided pointer until the library is unloaded.
 

src/bin/dhcp4/dhcp4_srv.cc

@@ -78,6 +78,8 @@ using namespace isc::log;
 using namespace isc::stats;
 using namespace std;
 
+namespace {
+
 /// Structure that holds registered hook indexes
 struct Dhcp4Hooks {
     int hook_index_buffer4_receive_;   ///< index for "buffer4_receive" hook point
@@ -104,12 +106,15 @@ struct Dhcp4Hooks {
     }
 };
 
+} // end of anonymous namespace
+
 // Declare a Hooks object. As this is outside any function or method, it
 // will be instantiated (and the constructor run) when the module is loaded.
 // As a result, the hook indexes will be defined before any method in this
 // module is called.
 Dhcp4Hooks Hooks;
 
+
 namespace isc {
 namespace dhcp {
 
@@ -1062,7 +1067,7 @@ Dhcpv4Srv::processPacket(Pkt4Ptr& query, Pkt4Ptr& rsp, bool allow_packet_park) {
         }
         callout_handle->setArgument("leases4", new_leases);
 
-        Lease4CollectionPtr deleted_leases(new Lease4Collection);
+        Lease4CollectionPtr deleted_leases(new Lease4Collection());
         if (ctx->old_lease_) {
             if ((!ctx->new_lease_) || (ctx->new_lease_->addr_ != ctx->old_lease_->addr_)) {
                 deleted_leases->push_back(ctx->old_lease_);

src/bin/dhcp4/tests/callout_library_1.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2018 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -11,4 +11,4 @@
 /// tests.  See callout_common.cc for details.
 
 static const int LIBRARY_NUMBER = 1;
-#include "callout_library_common.h"
+#include <dhcp4/tests/callout_library_common.h>

src/bin/dhcp4/tests/callout_library_2.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2018 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -11,4 +11,4 @@
 /// tests.  See callout_common.cc for details.
 
 static const int LIBRARY_NUMBER = 2;
-#include "callout_library_common.h"
+#include <dhcp4/tests/callout_library_common.h>

src/bin/dhcp4/tests/hooks_unittest.cc

@@ -2688,7 +2688,7 @@ TEST_F(LoadUnloadDhcpv4SrvTest, Dhcpv4SrvConfigured) {
     // The dhcp4_srv_configured should have been invoked and the provided
     // parameters should be recorded.
     EXPECT_TRUE(checkMarkerFile(SRV_CONFIG_MARKER_FILE,
-                                "3io_servicejson_configserver_config"));
+                                "3io_contextjson_configserver_config"));
 
     // Destroy the server, instance which should unload the libraries.
     srv.reset();
@@ -2698,5 +2698,5 @@ TEST_F(LoadUnloadDhcpv4SrvTest, Dhcpv4SrvConfigured) {
     EXPECT_TRUE(checkMarkerFile(LOAD_MARKER_FILE, "3"));
     EXPECT_TRUE(checkMarkerFile(UNLOAD_MARKER_FILE, "3"));
     EXPECT_TRUE(checkMarkerFile(SRV_CONFIG_MARKER_FILE,
-                                "3io_servicejson_configserver_config"));
+                                "3io_contextjson_configserver_config"));
 }

src/lib/hooks/parking_lots.h

@@ -52,6 +52,11 @@ namespace hooks {
 /// be unparked, but the object will be unparked when all callouts call this
 /// function, i.e. when all callouts signal completion of their respective
 /// asynchronous operations.
+///
+/// The types of the parked objects provided as T parameter of respective
+/// functions are most often shared pointers. One should not use references
+/// to parked objects nor references to shared pointers to avoid premature
+/// destruction of the parked objects.
 class ParkingLot {
 public:
 
@@ -218,6 +223,11 @@ typedef boost::shared_ptr<ParkingLot> ParkingLotPtr;
 /// The handle is provided to the callouts which can reference and unpark
 /// parked objects. The callouts should not park objects, therefore this
 /// operation is not available.
+///
+/// The types of the parked objects provided as T parameter of respective
+/// functions are most often shared pointers. One should not use references
+/// to parked objects nor references to shared pointers to avoid premature
+/// destruction of the parked objects.
 class ParkingLotHandle {
 public:
 

src/lib/hooks/tests/hooks_manager_unittest.cc

@@ -787,7 +787,7 @@ TEST_F(HooksManagerTest, Parking) {
     // got unparked.
     ASSERT_NO_THROW(
         HooksManager::park<std::string>("hookpt_one", "foo",
-        [this, &unparked] {
+        [&unparked] {
             unparked = true;
         })
     );
@@ -848,7 +848,7 @@ TEST_F(HooksManagerTest, ServerUnpark) {
     // can later test the value of this flag to verify when exactly the packet
     // got unparked.
     HooksManager::park<std::string>("hookpt_one", "foo",
-    [this, &unparked] {
+    [&unparked] {
         unparked = true;
     });
 
@@ -890,7 +890,7 @@ TEST_F(HooksManagerTest, ServerDropParked) {
     // can later test the value of this flag to verify when exactly the packet
     // got unparked.
     HooksManager::park<std::string>("hookpt_one", "foo",
-    [this, &unparked] {
+    [&unparked] {
         unparked = true;
     });
 
@@ -938,7 +938,7 @@ TEST_F(HooksManagerTest, UnloadBeforeUnpark) {
     // can later test the value of this flag to verify when exactly the packet
     // got unparked.
     HooksManager::park<std::string>("hookpt_one", "foo",
-    [this, &unparked] {
+    [&unparked] {
         unparked = true;
     });
 

src/lib/hooks/tests/parking_lots_unittest.cc

@@ -43,7 +43,7 @@ TEST(ParkingLotsTest, createGetParkingLot) {
 TEST(ParkingLotTest, parkWithoutReferencing) {
     ParkingLot parking_lot;
     std::string parked_object = "foo";
-    EXPECT_THROW(parking_lot.park(parked_object, [this] {
+    EXPECT_THROW(parking_lot.park(parked_object, [] {
     }), InvalidOperation);
 }
 
@@ -62,7 +62,7 @@ TEST(ParkingLotTest, unpark) {
 
     // This flag will indicate if the callback has been called.
     bool unparked = false;
-    ASSERT_NO_THROW(parking_lot->park(parked_object, [this, &unparked] {
+    ASSERT_NO_THROW(parking_lot->park(parked_object, [&unparked] {
         unparked = true;
     }));
 
@@ -96,7 +96,7 @@ TEST(ParkingLotTest, drop) {
 
     // This flag will indicate if the callback has been called.
     bool unparked = false;
-    ASSERT_NO_THROW(parking_lot->park(parked_object, [this, &unparked] {
+    ASSERT_NO_THROW(parking_lot->park(parked_object, [&unparked] {
         unparked = true;
     }));