From: Tomek Mrugalski Date: Wed, 18 Feb 2015 11:29:00 +0000 (+0100) Subject: [3711] Changes after review: X-Git-Tag: trac3723_base~18^2~1 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=fd015b6ef1f6e1773377e0f20188418b5fa092a8;p=thirdparty%2Fkea.git [3711] Changes after review: - IOAddress::increase() simplified - getLeasesCount renamed to getPoolCapacity - removed several intermediate variables --- diff --git a/src/lib/asiolink/io_address.cc b/src/lib/asiolink/io_address.cc index 08229fda6e..827793bf1e 100644 --- a/src/lib/asiolink/io_address.cc +++ b/src/lib/asiolink/io_address.cc @@ -143,8 +143,8 @@ IOAddress::subtract(const IOAddress& a, const IOAddress& b) { // v6 is more involved. // Let's extract the raw data first. - const vector& a_vec(a.toBytes()); - const vector& b_vec(b.toBytes()); + vector a_vec = a.toBytes(); + vector b_vec = b.toBytes(); // ... and prepare the result vector result(V6ADDRESS_LEN,0); @@ -156,9 +156,14 @@ IOAddress::subtract(const IOAddress& a, const IOAddress& b) { uint8_t carry = 0; // Now perform subtraction with borrow. - for (int i = a_vec.size(); i >= 0; --i) { - result[i] = a_vec[i] - b_vec[i] - carry; - carry = (a_vec[i] < b_vec[i] + carry); + for (int i = 15; i >=0; --i) { + if (a_vec[i] >= (b_vec[i] + carry) ) { + result[i] = a_vec[i] - b_vec[i] - carry; + carry = 0; + } else { + result[i] = a_vec[i] - b_vec[i] - carry; + carry = 1; + } } return (fromBytes(AF_INET6, &result[0])); @@ -167,29 +172,21 @@ IOAddress::subtract(const IOAddress& a, const IOAddress& b) { IOAddress IOAddress::increase(const IOAddress& addr) { - // Get a buffer holding an address. - const std::vector& vec = addr.toBytes(); - // Get the address length. - const int len = vec.size(); - // Since the same array will be used to hold the IPv4 and IPv6 // address we have to make sure that the size of the array // we allocate will work for both types of address. BOOST_STATIC_ASSERT(V4ADDRESS_LEN <= V6ADDRESS_LEN); - uint8_t packed[V6ADDRESS_LEN]; - - // Copy the address. It can be either V4 or V6. - std::memcpy(packed, &vec[0], len); + std::vector packed(addr.toBytes()); // Start increasing the least significant byte - for (int i = len - 1; i >= 0; --i) { + for (int i = packed.size() - 1; i >= 0; --i) { // if we haven't overflowed (0xff -> 0x0), than we are done if (++packed[i] != 0) { break; } } - return (IOAddress::fromBytes(addr.getFamily(), packed)); + return (IOAddress::fromBytes(addr.getFamily(), &packed[0])); } diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc index 927ed91971..81f27bcc86 100644 --- a/src/lib/dhcpsrv/alloc_engine.cc +++ b/src/lib/dhcpsrv/alloc_engine.cc @@ -543,7 +543,7 @@ AllocEngine::allocateUnreservedLeases6(ClientContext6& ctx) { /// try that number of times at most. It would be useful to that value if /// attempts_, specified by the user could override that value (and keep /// dynamic if they're set to 0). - uint32_t max_attempts = ctx.subnet_->getLeasesCount(ctx.type_); + uint32_t max_attempts = ctx.subnet_->getPoolCapacity(ctx.type_); for (uint32_t i = 0; i < max_attempts; ++i) { IOAddress candidate = allocator->pickAddress(ctx.subnet_, ctx.duid_, hint); @@ -1007,7 +1007,7 @@ AllocEngine::allocateLease4(const SubnetPtr& subnet, const ClientIdPtr& clientid /// try that number of times at most. It would be useful to that value if /// attempts_, specified by the user could override that value (and keep /// dynamic if they're set to 0). - uint64_t i = subnet->getLeasesCount(Lease::TYPE_V4); + uint64_t i = subnet->getPoolCapacity(Lease::TYPE_V4); do { // Decrease the number of remaining attempts here so as we guarantee // that it is decreased when the code below uses "continue". diff --git a/src/lib/dhcpsrv/pool.cc b/src/lib/dhcpsrv/pool.cc index 8af0fe2447..854c32d9a8 100644 --- a/src/lib/dhcpsrv/pool.cc +++ b/src/lib/dhcpsrv/pool.cc @@ -24,7 +24,8 @@ namespace dhcp { Pool::Pool(Lease::Type type, const isc::asiolink::IOAddress& first, const isc::asiolink::IOAddress& last) - :id_(getNextID()), first_(first), last_(last), type_(type) { + :id_(getNextID()), first_(first), last_(last), type_(type), + capacity_(0) { } bool Pool::inRange(const isc::asiolink::IOAddress& addr) const { @@ -55,7 +56,7 @@ Pool4::Pool4(const isc::asiolink::IOAddress& first, // the number of theoretically possible leases in it. As there's 2^32 // possible IPv4 addresses, we'll be able to accurately store that // info. - leases_count_ = addrsInRange(first, last); + capacity_ = addrsInRange(first, last); } Pool4::Pool4( const isc::asiolink::IOAddress& prefix, uint8_t prefix_len) @@ -78,7 +79,7 @@ Pool4::Pool4( const isc::asiolink::IOAddress& prefix, uint8_t prefix_len) // the number of theoretically possible leases in it. As there's 2^32 // possible IPv4 addresses, we'll be able to accurately store that // info. - leases_count_ = addrsInRange(prefix, last_); + capacity_ = addrsInRange(prefix, last_); } Pool6::Pool6(Lease::Type type, const isc::asiolink::IOAddress& first, @@ -120,7 +121,7 @@ Pool6::Pool6(Lease::Type type, const isc::asiolink::IOAddress& first, // Let's calculate the theoretical number of leases in this pool. // If the pool is extremely large (i.e. contains more than 2^64 addresses, // we'll just cap it at max value of uint64_t). - leases_count_ = addrsInRange(first, last); + capacity_ = addrsInRange(first, last); } Pool6::Pool6(Lease::Type type, const isc::asiolink::IOAddress& prefix, @@ -158,7 +159,7 @@ Pool6::Pool6(Lease::Type type, const isc::asiolink::IOAddress& prefix, // Let's calculate the theoretical number of leases in this pool. // For addresses, we could use addrsInRange(prefix, last_), but it's // much faster to do calculations on prefix lengths. - leases_count_ = prefixesInRange(prefix_len, delegated_len); + capacity_ = prefixesInRange(prefix_len, delegated_len); } std::string diff --git a/src/lib/dhcpsrv/pool.h b/src/lib/dhcpsrv/pool.h index 1d4d0a84ac..218c98a98d 100644 --- a/src/lib/dhcpsrv/pool.h +++ b/src/lib/dhcpsrv/pool.h @@ -84,8 +84,8 @@ public: /// Note that this is the upper bound, assuming that no leases are used /// and there are no host reservations. This is just a theoretical calculation. /// @return number of possible leases in this pool - uint64_t getLeasesCount() const { - return (leases_count_); + uint64_t getCapacity() const { + return (capacity_); } protected: @@ -135,7 +135,7 @@ protected: /// involved, so it is more efficient to calculate it once and just store /// the result. Note that for very large pools, the number is capped at /// max value of uint64_t. - uint64_t leases_count_; + uint64_t capacity_; }; /// @brief Pool information for IPv4 addresses diff --git a/src/lib/dhcpsrv/subnet.cc b/src/lib/dhcpsrv/subnet.cc index aa96921711..63b24fedd2 100644 --- a/src/lib/dhcpsrv/subnet.cc +++ b/src/lib/dhcpsrv/subnet.cc @@ -134,15 +134,15 @@ Subnet::toText() const { } uint64_t -Subnet::getLeasesCount(Lease::Type type) const { +Subnet::getPoolCapacity(Lease::Type type) const { switch (type) { case Lease::TYPE_V4: case Lease::TYPE_NA: - return sumLeasesCount(pools_); + return sumPoolCapacity(pools_); case Lease::TYPE_TA: - return sumLeasesCount(pools_ta_); + return sumPoolCapacity(pools_ta_); case Lease::TYPE_PD: - return sumLeasesCount(pools_pd_); + return sumPoolCapacity(pools_pd_); default: isc_throw(BadValue, "Unsupported pool type: " << static_cast(type)); @@ -150,10 +150,10 @@ Subnet::getLeasesCount(Lease::Type type) const { } uint64_t -Subnet::sumLeasesCount(const PoolCollection& pools) const { +Subnet::sumPoolCapacity(const PoolCollection& pools) const { uint64_t sum = 0; for (PoolCollection::const_iterator p = pools.begin(); p != pools.end(); ++p) { - uint64_t x = (*p)->getLeasesCount(); + uint64_t x = (*p)->getCapacity(); // Check if we can add it. If sum + x > uint64::max, then we would have // overflown if we tried to add it. diff --git a/src/lib/dhcpsrv/subnet.h b/src/lib/dhcpsrv/subnet.h index 9d5f1182b5..ed4758e523 100644 --- a/src/lib/dhcpsrv/subnet.h +++ b/src/lib/dhcpsrv/subnet.h @@ -236,7 +236,7 @@ public: /// @brief Returns the number of possible leases for specified lease type /// /// @param type type of the lease - uint64_t getLeasesCount(Lease::Type type) const; + uint64_t getPoolCapacity(Lease::Type type) const; /// @brief Sets name of the network interface for directly attached networks /// @@ -408,11 +408,10 @@ protected: /// @throw BadValue if invalid value is used virtual void checkType(Lease::Type type) const = 0; - /// @brief returns a sum of possible leases in all pools /// @param pools list of pools /// @return sum of possible leases - uint64_t sumLeasesCount(const PoolCollection& pools) const; + uint64_t sumPoolCapacity(const PoolCollection& pools) const; /// @brief subnet-id /// diff --git a/src/lib/dhcpsrv/tests/addr_utilities_unittest.cc b/src/lib/dhcpsrv/tests/addr_utilities_unittest.cc index af742007fa..0b2343fe9c 100644 --- a/src/lib/dhcpsrv/tests/addr_utilities_unittest.cc +++ b/src/lib/dhcpsrv/tests/addr_utilities_unittest.cc @@ -213,7 +213,7 @@ TEST(AddrUtilitiesTest, addrsInRange4) { EXPECT_EQ(3, addrsInRange(IOAddress("10.0.0.255"), IOAddress("10.0.1.1"))); // Let's go a bit overboard with this! How many addresses are there in - // IPv4 address space? That's a slightly tricku question, as the answer + // IPv4 address space? That's a slightly tricky question, as the answer // cannot be stored in uint32_t. EXPECT_EQ(uint64_t(std::numeric_limits::max()) + 1, addrsInRange(IOAddress("0.0.0.0"), IOAddress("255.255.255.255"))); diff --git a/src/lib/dhcpsrv/tests/pool_unittest.cc b/src/lib/dhcpsrv/tests/pool_unittest.cc index d480c441ef..4cf9ced815 100644 --- a/src/lib/dhcpsrv/tests/pool_unittest.cc +++ b/src/lib/dhcpsrv/tests/pool_unittest.cc @@ -84,16 +84,16 @@ TEST(Pool4Test, in_range) { // Checks if the number of possible leases in range is reported correctly. TEST(Pool4Test, leasesCount) { Pool4 pool1(IOAddress("192.0.2.10"), IOAddress("192.0.2.20")); - EXPECT_EQ(11, pool1.getLeasesCount()); + EXPECT_EQ(11, pool1.getCapacity()); Pool4 pool2(IOAddress("192.0.2.0"), IOAddress("192.0.2.255")); - EXPECT_EQ(256, pool2.getLeasesCount()); + EXPECT_EQ(256, pool2.getCapacity()); Pool4 pool3(IOAddress("192.168.0.0"), IOAddress("192.168.255.255")); - EXPECT_EQ(65536, pool3.getLeasesCount()); + EXPECT_EQ(65536, pool3.getCapacity()); Pool4 pool4(IOAddress("10.0.0.0"), IOAddress("10.255.255.255")); - EXPECT_EQ(16777216, pool4.getLeasesCount()); + EXPECT_EQ(16777216, pool4.getCapacity()); } // This test creates 100 pools and verifies that their IDs are unique. @@ -282,10 +282,10 @@ TEST(Poo6Test,toText) { TEST(Pool6Test, leasesCount) { Pool6 pool1(Lease::TYPE_NA, IOAddress("2001:db8::1"), IOAddress("2001:db8::2")); - EXPECT_EQ(2, pool1.getLeasesCount()); + EXPECT_EQ(2, pool1.getCapacity()); Pool6 pool2(Lease::TYPE_PD, IOAddress("2001:db8:1::"), 96, 112); - EXPECT_EQ(65536, pool2.getLeasesCount()); + EXPECT_EQ(65536, pool2.getCapacity()); } diff --git a/src/lib/dhcpsrv/tests/subnet_unittest.cc b/src/lib/dhcpsrv/tests/subnet_unittest.cc index b88d1460a2..9a80b664e2 100644 --- a/src/lib/dhcpsrv/tests/subnet_unittest.cc +++ b/src/lib/dhcpsrv/tests/subnet_unittest.cc @@ -150,28 +150,28 @@ TEST(Subnet4Test, Pool4InSubnet4) { // Check if it's possible to get specified number of possible leases for // an IPv4 subnet. -TEST(Subnet4Test, getLeasesCount) { +TEST(Subnet4Test, getCapacity) { // There's one /24 pool. Subnet4Ptr subnet(new Subnet4(IOAddress("192.1.2.0"), 24, 1, 2, 3)); // There are no pools defined, so the total number of available addrs is 0. - EXPECT_EQ(0, subnet->getLeasesCount(Lease::TYPE_V4)); + EXPECT_EQ(0, subnet->getPoolCapacity(Lease::TYPE_V4)); // Let's add a /25 pool. That's 128 addresses. PoolPtr pool1(new Pool4(IOAddress("192.1.2.0"), 25)); subnet->addPool(pool1); - EXPECT_EQ(128, subnet->getLeasesCount(Lease::TYPE_V4)); + EXPECT_EQ(128, subnet->getPoolCapacity(Lease::TYPE_V4)); // Let's add another /26 pool. That's extra 64 addresses. PoolPtr pool2(new Pool4(IOAddress("192.1.2.128"), 26)); subnet->addPool(pool2); - EXPECT_EQ(192, subnet->getLeasesCount(Lease::TYPE_V4)); + EXPECT_EQ(192, subnet->getPoolCapacity(Lease::TYPE_V4)); // Let's add a third pool /30. This one has 4 addresses. PoolPtr pool3(new Pool4(IOAddress("192.1.2.192"), 30)); subnet->addPool(pool3); - EXPECT_EQ(196, subnet->getLeasesCount(Lease::TYPE_V4)); + EXPECT_EQ(196, subnet->getPoolCapacity(Lease::TYPE_V4)); } TEST(Subnet4Test, Subnet4_Pool4_checks) { @@ -463,7 +463,7 @@ TEST(Subnet6Test, relay) { // Test checks whether the number of addresses available in the pools are // calculated properly. -TEST(Subnet6Test, Pool6LeasesCount) { +TEST(Subnet6Test, Pool6getCapacity) { Subnet6Ptr subnet(new Subnet6(IOAddress("2001:db8:1::"), 56, 1, 2, 3, 4)); @@ -474,35 +474,35 @@ TEST(Subnet6Test, Pool6LeasesCount) { PoolPtr pool2(new Pool6(Lease::TYPE_NA, IOAddress("2001:db8:1:2::"), 96)); PoolPtr pool3(new Pool6(Lease::TYPE_NA, IOAddress("2001:db8:1:3::"), 96)); - EXPECT_EQ(0, subnet->getLeasesCount(Lease::TYPE_NA)); - EXPECT_EQ(0, subnet->getLeasesCount(Lease::TYPE_TA)); - EXPECT_EQ(0, subnet->getLeasesCount(Lease::TYPE_PD)); + EXPECT_EQ(0, subnet->getPoolCapacity(Lease::TYPE_NA)); + EXPECT_EQ(0, subnet->getPoolCapacity(Lease::TYPE_TA)); + EXPECT_EQ(0, subnet->getPoolCapacity(Lease::TYPE_PD)); subnet->addPool(pool1); - EXPECT_EQ(65536, subnet->getLeasesCount(Lease::TYPE_NA)); + EXPECT_EQ(65536, subnet->getPoolCapacity(Lease::TYPE_NA)); subnet->addPool(pool2); - EXPECT_EQ(uint64_t(4294967296 + 65536), subnet->getLeasesCount(Lease::TYPE_NA)); + EXPECT_EQ(uint64_t(4294967296 + 65536), subnet->getPoolCapacity(Lease::TYPE_NA)); subnet->addPool(pool3); EXPECT_EQ(uint64_t(4294967296 + 4294967296 + 65536), - subnet->getLeasesCount(Lease::TYPE_NA)); + subnet->getPoolCapacity(Lease::TYPE_NA)); // This is 2^64 prefixes. We're overflown uint64_t. PoolPtr pool4(new Pool6(Lease::TYPE_NA, IOAddress("2001:db8:1:4::"), 64)); subnet->addPool(pool4); EXPECT_EQ(std::numeric_limits::max(), - subnet->getLeasesCount(Lease::TYPE_NA)); + subnet->getPoolCapacity(Lease::TYPE_NA)); PoolPtr pool5(new Pool6(Lease::TYPE_NA, IOAddress("2001:db8:1:5::"), 64)); subnet->addPool(pool5); EXPECT_EQ(std::numeric_limits::max(), - subnet->getLeasesCount(Lease::TYPE_NA)); + subnet->getPoolCapacity(Lease::TYPE_NA)); } // Test checks whether the number of prefixes available in the pools are // calculated properly. -TEST(Subnet6Test, Pool6PdLeasesCount) { +TEST(Subnet6Test, Pool6PdgetPoolCapacity) { Subnet6Ptr subnet(new Subnet6(IOAddress("2001:db8::"), 32, 1, 2, 3, 4)); @@ -513,30 +513,30 @@ TEST(Subnet6Test, Pool6PdLeasesCount) { PoolPtr pool2(new Pool6(Lease::TYPE_PD, IOAddress("2001:db8:2::"), 48, 80)); PoolPtr pool3(new Pool6(Lease::TYPE_PD, IOAddress("2001:db8:3::"), 48, 80)); - EXPECT_EQ(0, subnet->getLeasesCount(Lease::TYPE_NA)); - EXPECT_EQ(0, subnet->getLeasesCount(Lease::TYPE_TA)); - EXPECT_EQ(0, subnet->getLeasesCount(Lease::TYPE_PD)); + EXPECT_EQ(0, subnet->getPoolCapacity(Lease::TYPE_NA)); + EXPECT_EQ(0, subnet->getPoolCapacity(Lease::TYPE_TA)); + EXPECT_EQ(0, subnet->getPoolCapacity(Lease::TYPE_PD)); subnet->addPool(pool1); - EXPECT_EQ(65536, subnet->getLeasesCount(Lease::TYPE_PD)); + EXPECT_EQ(65536, subnet->getPoolCapacity(Lease::TYPE_PD)); subnet->addPool(pool2); - EXPECT_EQ(uint64_t(4294967296 + 65536), subnet->getLeasesCount(Lease::TYPE_PD)); + EXPECT_EQ(uint64_t(4294967296 + 65536), subnet->getPoolCapacity(Lease::TYPE_PD)); subnet->addPool(pool3); EXPECT_EQ(uint64_t(4294967296 + 4294967296 + 65536), - subnet->getLeasesCount(Lease::TYPE_PD)); + subnet->getPoolCapacity(Lease::TYPE_PD)); // This is 2^64. PoolPtr pool4(new Pool6(Lease::TYPE_PD, IOAddress("2001:db8:4::"), 48, 112)); subnet->addPool(pool4); EXPECT_EQ(std::numeric_limits::max(), - subnet->getLeasesCount(Lease::TYPE_PD)); + subnet->getPoolCapacity(Lease::TYPE_PD)); PoolPtr pool5(new Pool6(Lease::TYPE_PD, IOAddress("2001:db8:5::"), 48, 112)); subnet->addPool(pool5); EXPECT_EQ(std::numeric_limits::max(), - subnet->getLeasesCount(Lease::TYPE_PD)); + subnet->getPoolCapacity(Lease::TYPE_PD)); } TEST(Subnet6Test, Pool6InSubnet6) {