From: Thomas Markwalder Date: Fri, 7 Nov 2025 13:27:12 +0000 (-0500) Subject: [#4190] Address review comments X-Git-Tag: Kea-3.1.4~53 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ac830af76d271978f81721ece4a8bf2ce05fdaf2;p=thirdparty%2Fkea.git [#4190] Address review comments modified: lease_query_impl.cc modified: lease_query_impl.h modified: tests/lease_query_impl4_unittest.cc modified: tests/lease_query_impl6_unittest.cc --- diff --git a/src/hooks/dhcp/lease_query/lease_query_impl.cc b/src/hooks/dhcp/lease_query/lease_query_impl.cc index 7ceb9b0691..d4712f0ca4 100644 --- a/src/hooks/dhcp/lease_query/lease_query_impl.cc +++ b/src/hooks/dhcp/lease_query/lease_query_impl.cc @@ -118,9 +118,9 @@ LeaseQueryImpl::parserRequesters(ConstElementPtr requesters) { IOAddress address(entry_txt); address_list_.insert(address); } catch (const std::exception& ex) { - isc_throw(BadValue, - "'requesters' address entry '" << address_elem->stringValue() - << "' is invalid: " << ex.what()); + isc_throw(BadValue, "'requesters' address entry '" + << address_elem->stringValue() + << "' is invalid: " << ex.what()); } } else { try { @@ -132,7 +132,8 @@ LeaseQueryImpl::parserRequesters(ConstElementPtr requesters) { if ((prefix_len < std::numeric_limits::min()) || (prefix_len > std::numeric_limits::max())) { // This exception will be handled 4 line later! - isc_throw(OutOfRange, "prefix length " << len_txt << " is out of range"); + isc_throw(OutOfRange, "prefix length " + << len_txt << " is out of range"); } pool_set_.insert(prefix, prefix_len); diff --git a/src/hooks/dhcp/lease_query/lease_query_impl.h b/src/hooks/dhcp/lease_query/lease_query_impl.h index 79c04a376b..27850cc28c 100644 --- a/src/hooks/dhcp/lease_query/lease_query_impl.h +++ b/src/hooks/dhcp/lease_query/lease_query_impl.h @@ -75,7 +75,7 @@ private: uint16_t family_; /// @brief Set of unique addresses in the list. - std::unordered_set > addresses_; + std::unordered_set addresses_; }; /// @brief Hash for a Pool based on it's address range. @@ -103,7 +103,9 @@ struct PoolRangeEqual { }; /// @brief Defines an alias for a set of pools hashed by range. -using PoolRangeSet = std::unordered_set; +using PoolRangeSet = std::unordered_set; /// @brief Manages a unique set of Pools of a given protocol family. /// The pools are hashed by their address range. @@ -228,8 +230,8 @@ public: private: /// @brief Parses 'requesters' list element. /// - /// @param requesters pointer to the list element containing requestor - /// entris. Entries may be a mix of IP addresses or subnets in CIDR format. + /// @param requesters pointer to the list element containing requester + /// entries. Entries may be a mix of IP addresses or subnets in CIDR format. /// /// @throw BadValue if the list is empty or if any of the entries are /// not valid addresses or CIDRs. diff --git a/src/hooks/dhcp/lease_query/tests/lease_query_impl4_unittest.cc b/src/hooks/dhcp/lease_query/tests/lease_query_impl4_unittest.cc index 141c807d20..8e0420276d 100644 --- a/src/hooks/dhcp/lease_query/tests/lease_query_impl4_unittest.cc +++ b/src/hooks/dhcp/lease_query/tests/lease_query_impl4_unittest.cc @@ -586,12 +586,15 @@ TEST(LeaseQueryImpl4Test, invalidConfig4) { { "requesters entry not a v4 address", Element::fromJSON("{ \"requesters\" : [ \"2001:db8:1::\" ] }"), - "'requesters' address entry '2001:db8:1::' is invalid: not a IPv4 address" + "'requesters' address entry '2001:db8:1::'" + " is invalid: not a IPv4 address" }, { "requesters entry is a duplicate", - Element::fromJSON("{ \"requesters\" : [ \"192.0.2.1\", \"192.0.2.1\" ] }"), - "'requesters' address entry '192.0.2.1' is invalid: address is already in the list" + Element::fromJSON("{ \"requesters\" : [ \"192.0.2.1\"," + " \"192.0.2.1\" ] }"), + "'requesters' address entry '192.0.2.1' is invalid:" + " address is already in the list" }, { "requesters CIDR entry address is a invalid", @@ -607,8 +610,10 @@ TEST(LeaseQueryImpl4Test, invalidConfig4) { }, { "requesters CIDR entry is a duplicate", - Element::fromJSON("{ \"requesters\" : [ \"192.0.2.0/24\", \"192.0.2.0/24\" ] }"), - "'requesters' CIDR entry '192.0.2.0/24' is invalid: entry already exists" + Element::fromJSON("{ \"requesters\" : [ \"192.0.2.0/24\"," + " \"192.0.2.0/24\" ] }"), + "'requesters' CIDR entry '192.0.2.0/24' is invalid:" + " entry already exists" } }; @@ -646,7 +651,8 @@ TEST(LeaseQueryImpl4Test, validConfig4) { // parses and that requesters can be validated. TEST(LeaseQueryImpl4Test, validConfig4CIDROnly) { // Create an implementation with two requesters. - const std::string json = "{ \"requesters\" : [ \"192.0.2.0/24\", \"192.0.3.0/24\" ] }"; + const std::string json = "{ \"requesters\" : [ \"192.0.2.0/24\"," + " \"192.0.3.0/24\" ] }"; ConstElementPtr config; ASSERT_NO_THROW_LOG(config = Element::fromJSON(json)); @@ -666,7 +672,8 @@ TEST(LeaseQueryImpl4Test, validConfig4CIDROnly) { // parses and that requesters can be validated. TEST(LeaseQueryImpl4Test, validConfig4Mix) { // Create an implementation with two requesters. - const std::string json = "{ \"requesters\" : [ \"192.0.2.0/24\", \"192.0.3.25\" ] }"; + const std::string json = "{ \"requesters\" : [ \"192.0.2.0/24\"," + " \"192.0.3.25\" ] }"; ConstElementPtr config; ASSERT_NO_THROW_LOG(config = Element::fromJSON(json)); @@ -685,7 +692,8 @@ TEST(LeaseQueryImpl4Test, validConfig4Mix) { // and client id) are detected. TEST(LeaseQueryImpl4Test, processQueryInvalidQuery) { // Create an implementation with two requesters. - const std::string json = "{ \"requesters\" : [ \"192.0.2.1\", \"192.0.2.3\" ] }"; + const std::string json = "{ \"requesters\" : [ \"192.0.2.1\"," + " \"192.0.2.3\" ] }"; ConstElementPtr config; ASSERT_NO_THROW_LOG(config = Element::fromJSON(json)); LeaseQueryImpl4Ptr impl; diff --git a/src/hooks/dhcp/lease_query/tests/lease_query_impl6_unittest.cc b/src/hooks/dhcp/lease_query/tests/lease_query_impl6_unittest.cc index 42809ec6e6..9b4b55203c 100644 --- a/src/hooks/dhcp/lease_query/tests/lease_query_impl6_unittest.cc +++ b/src/hooks/dhcp/lease_query/tests/lease_query_impl6_unittest.cc @@ -590,24 +590,29 @@ TEST(LeaseQueryImpl6Test, invalidConfig6) { { "requesters entry not an address", Element::fromJSON("{ \"requesters\" : [ \"foo\" ] }"), - "'requesters' address entry 'foo' is invalid: Failed to convert string" + "'requesters' address entry 'foo' is invalid:" + " Failed to convert string" " to address 'foo': Invalid argument" }, { "requesters entry not a v6 address", Element::fromJSON("{ \"requesters\" : [ \"192.0.2.1\" ] }"), - "'requesters' address entry '192.0.2.1' is invalid: not a IPv6 address" + "'requesters' address entry '192.0.2.1' is invalid:" + " not a IPv6 address" }, { "requesters entry is a duplicate", - Element::fromJSON("{ \"requesters\" : [ \"2001:db8:1::\", \"2001:db8:1::\" ] }"), - "'requesters' address entry '2001:db8:1::' is invalid: address is already in the list" + Element::fromJSON("{ \"requesters\" : [ \"2001:db8:1::\"," + " \"2001:db8:1::\" ] }"), + "'requesters' address entry '2001:db8:1::' is invalid:" + " address is already in the list" }, { "requesters CIDR entry address is invalid", Element::fromJSON("{ \"requesters\" : [ \"2001:db8:1::x/64\" ] }"), "'requesters' CIDR entry '2001:db8:1::x/64' is invalid:" - " Failed to convert string to address '2001:db8:1::x': Invalid argument" + " Failed to convert string to address '2001:db8:1::x':" + " Invalid argument" }, { @@ -618,29 +623,37 @@ TEST(LeaseQueryImpl6Test, invalidConfig6) { }, { "requesters CIDR is a duplicate", - Element::fromJSON("{ \"requesters\" : [ \"2001:db8:1::/64\", \"2001:db8:1::/64\"] }"), - "'requesters' CIDR entry '2001:db8:1::/64' is invalid: entry already exists" + Element::fromJSON("{ \"requesters\" : [ \"2001:db8:1::/64\"," + " \"2001:db8:1::/64\"] }"), + "'requesters' CIDR entry '2001:db8:1::/64' is invalid:" + " entry already exists" }, { "prefix_lengths not a list", - Element::fromJSON("{ \"requesters\" : [ \"2001:db8:1::\" ], \"prefix-lengths\": 77 }"), + Element::fromJSON("{ \"requesters\" : [ \"2001:db8:1::\" ]," + " \"prefix-lengths\": 77 }"), "'prefix-lengths' is not a list" }, { "prefix_lengths entry is not an int", - Element::fromJSON("{ \"requesters\" : [ \"2001:db8:1::\" ], \"prefix-lengths\": [ \"boo\" ] }"), + Element::fromJSON("{ \"requesters\" : [ \"2001:db8:1::\" ]," + " \"prefix-lengths\": [ \"boo\" ] }"), "'prefix-lengths' entry '\"boo\"' is invalid: must be an integer" }, { "prefix_lengths entry is 0", - Element::fromJSON("{ \"requesters\" : [ \"2001:db8:1::\" ], \"prefix-lengths\": [ 0 ] }"), - "'prefix-lengths' entry '0' is invalid: must be greater than 0 and less than or equal to 128" + Element::fromJSON("{ \"requesters\" : [ \"2001:db8:1::\" ]," + " \"prefix-lengths\": [ 0 ] }"), + "'prefix-lengths' entry '0' is invalid: must be greater" + " than 0 and less than or equal to 128" }, { "prefix_lengths entry is 129", - Element::fromJSON("{ \"requesters\" : [ \"2001:db8:1::\" ], \"prefix-lengths\": [ 4, 129 ] }"), - "'prefix-lengths' entry '129' is invalid: must be greater than 0 and less than or equal to 128" + Element::fromJSON("{ \"requesters\" : [ \"2001:db8:1::\" ]," + " \"prefix-lengths\": [ 4, 129 ] }"), + "'prefix-lengths' entry '129' is invalid:" + " must be greater than 0 and less than or equal to 128" } }; @@ -657,7 +670,8 @@ TEST(LeaseQueryImpl6Test, invalidConfig6) { // can be validated. TEST(LeaseQueryImpl6Test, validConfig6) { // Create an implementation with two requesters. - const std::string json = "{ \"requesters\" : [ \"2001:db8:1::1\", \"2001:db8:1::3\" ] }"; + const std::string json = "{ \"requesters\" : [ \"2001:db8:1::1\"," + " \"2001:db8:1::3\" ] }"; ConstElementPtr config; ASSERT_NO_THROW_LOG(config = Element::fromJSON(json)); @@ -691,7 +705,8 @@ class MemfileLeaseQueryImpl6ProcessTest : public // - No D6O_LQ_QUERY option TEST_F(MemfileLeaseQueryImpl6ProcessTest, processQueryInvalidQuery) { // Create an implementation with two requesters. - const std::string json = "{ \"requesters\" : [ \"2001:db8:2::1\", \"2001:db8:3::1\" ] }"; + const std::string json = "{ \"requesters\" : [ \"2001:db8:2::1\"," + " \"2001:db8:3::1\" ] }"; ConstElementPtr config; ASSERT_NO_THROW_LOG(config = Element::fromJSON(json)); LeaseQueryImpl6Ptr impl;