[5457] Addressed review comments.

diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc
index ae0dc5f9bdc2bbef43ec10389f53e638b013f8b2..3cf0f0bd4690bf5f136bb6bd999ceee789888ea8 100644 (file)
--- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc
+++ b/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());
 

diff --git a/src/bin/dhcp4/dhcp4_hooks.dox b/src/bin/dhcp4/dhcp4_hooks.dox
index 0d804f2a9117e5afef5b7d472ae124a276e39632..2ae0289aa09802ee2647f6c7a3a9c75fbe191c5d 100644 (file)
--- a/src/bin/dhcp4/dhcp4_hooks.dox
+++ b/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.
 

diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 2ca899530fd1d5da75696cb6fb631eeb91aa9a1c..74ddc3a667112e723cfd36e1471eb57562329157 100644 (file)
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/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_);

diff --git a/src/bin/dhcp4/tests/callout_library_1.cc b/src/bin/dhcp4/tests/callout_library_1.cc
index 72045f28d05eea26a36430a715d063df16cf0e5f..af66942ae95f5f3758c0c27d484506080da0e3cc 100644 (file)
--- a/src/bin/dhcp4/tests/callout_library_1.cc
+++ b/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>

diff --git a/src/bin/dhcp4/tests/callout_library_2.cc b/src/bin/dhcp4/tests/callout_library_2.cc
index 3c7bbe00ff633a1b889b76ba3a93bcde3a97e71e..8135e2b1cd8ffe83194c06dad7d6a59d2159dc1d 100644 (file)
--- a/src/bin/dhcp4/tests/callout_library_2.cc
+++ b/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>

diff --git a/src/bin/dhcp4/tests/hooks_unittest.cc b/src/bin/dhcp4/tests/hooks_unittest.cc
index 2bf7428e4e261478aadd06cbb9a574dc1f58e201..b43d813425d8c9efc727a5344660cf8c259dfed3 100644 (file)
--- a/src/bin/dhcp4/tests/hooks_unittest.cc
+++ b/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"));
 }

diff --git a/src/lib/hooks/parking_lots.h b/src/lib/hooks/parking_lots.h
index 991fe3d323f503d3a32b408ff371063c92fa9d38..7ee9ec0eaeaea0adccd886a6e284ea817d7bc9ea 100644 (file)
--- a/src/lib/hooks/parking_lots.h
+++ b/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:
 

diff --git a/src/lib/hooks/tests/hooks_manager_unittest.cc b/src/lib/hooks/tests/hooks_manager_unittest.cc
index ff985a22e7dd6208e5b7bb641f0901cc215d1ccd..089c894f4af6ec489c52942285e9ef531d68c9e2 100644 (file)
--- a/src/lib/hooks/tests/hooks_manager_unittest.cc
+++ b/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;
     });
 

diff --git a/src/lib/hooks/tests/parking_lots_unittest.cc b/src/lib/hooks/tests/parking_lots_unittest.cc
index fe7a2039dbb7bcef7f3e89154377baab0cb56f5c..99b62ca83ffac576ad15b1973c82ed5da4c66be7 100644 (file)
--- a/src/lib/hooks/tests/parking_lots_unittest.cc
+++ b/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;
     }));