[#4492] Addressed Minor review comments

modified:   ../../../../doc/sphinx/arm/hooks-lease-cmds.rst
modified:   libloadtests/sflq_cmds_unittests.cc
modified:   sflq_cmds.cc
modified:   ../../../lib/cc/simple_parser.h
modified:   ../../../lib/dhcpsrv/lease_mgr.h

diff --git a/doc/sphinx/arm/hooks-lease-cmds.rst b/doc/sphinx/arm/hooks-lease-cmds.rst
index 342c04288c4c493ba4c1418c386bc77c5dfb38c0..c2e1cda0869ee067bb030f4a64475234f1ca9e31 100644 (file)
--- a/doc/sphinx/arm/hooks-lease-cmds.rst
+++ b/doc/sphinx/arm/hooks-lease-cmds.rst
@@ -1312,7 +1312,7 @@ The ``sflq-pool4-get-all``, ``sflq-pool6-get-all`` Commands
 -----------------------------------------------------------
 
 :isccmd:`sflq-pool4-get-all` and :isccmd:`sflq-pool6-get-all` can be used
-to fetch a list of all IPv4 or IPv6 SFLQ pools.  It takes no arguments.
+to fetch a list of all IPv4 or IPv6 SFLQ pools.  They take no arguments.
 
 An example :isccmd:`sflq-pool4-get-all` command for fetching IPv4 SFLQ
 pools:
@@ -1594,7 +1594,7 @@ The ``sflq-pool4-del``, ``sflq-pool6-del`` Commands
 ---------------------------------------------------
 
 :isccmd:`sflq-pool4-del` and :isccmd:`sflq-pool6-del` can be used to delete
-an IPv4 or IPv6 SFLQ pool. It requires two arguments, ``start-address`` and
+an IPv4 or IPv6 SFLQ pool. They require two arguments, ``start-address`` and
 and ``end-address``.  Only a pool whose range matches these values will be
 deleted and only if no other SFLQ pools overlap the range described by these
 values.  If any overlapping pools are detected the delete will fail with an

diff --git a/src/hooks/dhcp/lease_cmds/libloadtests/sflq_cmds_unittests.cc b/src/hooks/dhcp/lease_cmds/libloadtests/sflq_cmds_unittests.cc
index 0dab1d542026254e90a467481be61a0a3dfb2975..5b42d3ec968264ba50486897deb7eb03d591e62f 100644 (file)
--- a/src/hooks/dhcp/lease_cmds/libloadtests/sflq_cmds_unittests.cc
+++ b/src/hooks/dhcp/lease_cmds/libloadtests/sflq_cmds_unittests.cc
@@ -417,6 +417,14 @@ void SflqCmds4Test::sflqPool4GetBySubnetBadParams() {
             "bogus" : "fluff"
         )",
         "spurious 'bogus' parameter"
+    },
+    {
+        __LINE__,
+        R"(
+            "subnet-id": -1
+        )",
+        "'subnet-id' -1 is invalid, must be greater than zero"
+        " and less than 4294967295"
     }
     };
 
@@ -997,6 +1005,14 @@ void SflqCmds6Test::sflqPool6GetBySubnetBadParams() {
             "bogus" : "fluff"
         )",
         "spurious 'bogus' parameter"
+    },
+    {
+        __LINE__,
+        R"(
+            "subnet-id": -1
+        )",
+        "'subnet-id' -1 is invalid, must be greater than zero"
+        " and less than 4294967295"
     }
     };
 

diff --git a/src/hooks/dhcp/lease_cmds/sflq_cmds.cc b/src/hooks/dhcp/lease_cmds/sflq_cmds.cc
index 3141f51538f8945f91bc7bfcdc4522068520e32a..a19d5f0238f5a5b71fec04c8e93a1c04304f3161 100644 (file)
--- a/src/hooks/dhcp/lease_cmds/sflq_cmds.cc
+++ b/src/hooks/dhcp/lease_cmds/sflq_cmds.cc
@@ -254,7 +254,14 @@ SflqCmdsImpl::sflqPoolGetBySubnet(CalloutHandle& handle, uint16_t family) {
 
         SimpleParser::checkRequired(required_keywords, cmd_args_);
         SimpleParser::checkKeywords(keywords, cmd_args_);
-        SubnetID subnet_id = SimpleParser::getInteger(cmd_args_, "subnet-id");
+
+        // Extract subnet-id.
+        int64_t subnet_id = SimpleParser::getInteger(cmd_args_, "subnet-id");
+        if ((subnet_id < 1) || (subnet_id >= SUBNET_ID_UNUSED)) {
+            isc_throw(isc::BadValue, "'subnet-id' " << subnet_id << " is invalid,"
+                                     " must be greater than zero and less than "
+                                     << SUBNET_ID_UNUSED);
+        }
 
         // Invoke the pool get by subnet function.
         auto pools = (family == AF_INET ? LeaseMgrFactory::instance().sflqPool4Get(subnet_id)

diff --git a/src/lib/cc/simple_parser.h b/src/lib/cc/simple_parser.h
index cb788028fda548844fe3612de74be7c61d8c5b0e..5a15392e936f798571a064fd9b30abcc9430d356 100644 (file)
--- a/src/lib/cc/simple_parser.h
+++ b/src/lib/cc/simple_parser.h
@@ -290,7 +290,7 @@ public:
     /// @return an uint32_t value
     /// @throw isc::dhcp::DhcpConfigError when it is not an uint32_t
     uint32_t getUint32(isc::data::ConstElementPtr scope,
-                       const std::string& name) {
+                              const std::string& name) {
         return (getIntType<uint32_t>(scope, name));
     }
 

diff --git a/src/lib/dhcpsrv/lease_mgr.h b/src/lib/dhcpsrv/lease_mgr.h
index 28455ae7113417f51f8c41a948d74c06a676c018..f4a128342de4b5a238fe8315da3b93b76278c36e 100644 (file)
--- a/src/lib/dhcpsrv/lease_mgr.h
+++ b/src/lib/dhcpsrv/lease_mgr.h
@@ -235,11 +235,6 @@ typedef boost::shared_ptr<LeaseStatsQuery> LeaseStatsQueryPtr;
 /// @brief Defines a pointer to a LeaseStatsRow.
 typedef boost::shared_ptr<LeaseStatsRow> LeaseStatsRowPtr;
 
-// Forward declaration.
-class SflqPoolInfo;
-/// @brief A pointer to a SFLQPoolInfo instance.
-typedef boost::shared_ptr<SflqPoolInfo> SflqPoolInfoPtr;
-
 /// @brief Describes a SFLQ pool.
 class SflqPoolInfo {
 public:
@@ -259,6 +254,9 @@ public:
     data::ElementPtr toElement() const;
 };
 
+/// @brief A pointer to a SFLQPoolInfo instance.
+typedef boost::shared_ptr<SflqPoolInfo> SflqPoolInfoPtr;
+
 /// @brief A collection of SFLQPoolInfo structures.
 typedef std::vector<SflqPoolInfoPtr> SflqPoolInfoCollection;
 typedef boost::shared_ptr<SflqPoolInfoCollection> SflqPoolInfoCollectionPtr;