From: Thomas Markwalder Date: Tue, 23 Aug 2016 18:16:46 +0000 (-0400) Subject: [4294] Reverted Lease states back to uin32_t per review comments X-Git-Tag: trac4631_base~6^2~7 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=86411b64adb0de8ce8b1bd6394c475b08afdb8db;p=thirdparty%2Fkea.git [4294] Reverted Lease states back to uin32_t per review comments I had replaced the uint32_t constants for lease states with an enum. Tomek stated that they should remain uin32_t as they may eventually be bitmasks. --- diff --git a/src/lib/dhcpsrv/lease.cc b/src/lib/dhcpsrv/lease.cc index d8d15613d1..5854f06900 100644 --- a/src/lib/dhcpsrv/lease.cc +++ b/src/lib/dhcpsrv/lease.cc @@ -15,6 +15,10 @@ using namespace std; namespace isc { namespace dhcp { +const uint32_t Lease::STATE_DEFAULT = 0x0; +const uint32_t Lease::STATE_DECLINED = 0x1; +const uint32_t Lease::STATE_EXPIRED_RECLAIMED = 0x2; + Lease::Lease(const isc::asiolink::IOAddress& addr, uint32_t t1, uint32_t t2, uint32_t valid_lft, SubnetID subnet_id, time_t cltt, const bool fqdn_fwd, const bool fqdn_rev, diff --git a/src/lib/dhcpsrv/lease.h b/src/lib/dhcpsrv/lease.h index 365cb06f53..c89ad523a3 100644 --- a/src/lib/dhcpsrv/lease.h +++ b/src/lib/dhcpsrv/lease.h @@ -40,19 +40,17 @@ struct Lease { /// @return text decription static std::string typeToText(Type type); - /// @name Enumeration of lease states + /// @name Common lease states constants. //@{ - typedef enum { - /// @brief A lease in the default (assigned) state. - STATE_DEFAULT, - /// @brief Declined lease. - STATE_DECLINED, - /// @brief Expired and reclaimed lease. - STATE_EXPIRED_RECLAIMED, - /// @brief The number of defined lease states. - NUM_LEASE_STATES - } LeaseState; - //@} + /// + /// @brief A lease in the default state. + static const uint32_t STATE_DEFAULT; + + /// @brief Declined lease. + static const uint32_t STATE_DECLINED; + + /// @brief Expired and reclaimed lease. + static const uint32_t STATE_EXPIRED_RECLAIMED; /// @brief Returns name(s) of the basic lease state(s). /// diff --git a/src/lib/dhcpsrv/lease_mgr.cc b/src/lib/dhcpsrv/lease_mgr.cc index b3c34f13ad..de2bf01153 100644 --- a/src/lib/dhcpsrv/lease_mgr.cc +++ b/src/lib/dhcpsrv/lease_mgr.cc @@ -91,29 +91,19 @@ LeaseMgr::recountAddressStats4() { // updating the subnet and global values. AddressStatsRow4 row; while (query->getNextRow(row)) { - switch(row.lease_state_) { - case Lease::STATE_DEFAULT: - // Set subnet level value. - stats_mgr.setValue(StatsMgr::generateName("subnet", - row.subnet_id_, - "assigned-addresses"), - row.state_count_); - break; - - case Lease::STATE_DECLINED: - // Set subnet level value. - stats_mgr.setValue(StatsMgr::generateName("subnet", - row.subnet_id_, - "declined-addresses"), - row.state_count_); - - // Add to the global value. - stats_mgr.addValue("declined-addresses", row.state_count_); - break; - - default: - // Not one we're tracking. - break; + if (row.lease_state_ == Lease::STATE_DEFAULT) { + // Set subnet level value. + stats_mgr.setValue(StatsMgr::generateName("subnet", row.subnet_id_, + "assigned-addresses"), + row.state_count_); + } else if (row.lease_state_ == Lease::STATE_DECLINED) { + // Set subnet level value. + stats_mgr.setValue(StatsMgr::generateName("subnet", row.subnet_id_, + "declined-addresses"), + row.state_count_); + + // Add to the global value. + stats_mgr.addValue("declined-addresses", row.state_count_); } } } @@ -180,46 +170,31 @@ LeaseMgr::recountAddressStats6() { while (query->getNextRow(row)) { switch(row.lease_type_) { case Lease::TYPE_NA: - switch(row.lease_state_) { - case Lease::STATE_DEFAULT: - // Set subnet level value. - stats_mgr.setValue(StatsMgr:: - generateName("subnet", - row.subnet_id_, - "assigned-nas"), - row.state_count_); - break; - case Lease::STATE_DECLINED: - // Set subnet level value. - stats_mgr.setValue(StatsMgr:: - generateName("subnet", - row.subnet_id_, - "declined-addresses"), - row.state_count_); - - // Add to the global value. - stats_mgr.addValue("declined-addresses", - row.state_count_); - break; - default: - // Not one we're tracking. - break; + if (row.lease_state_ == Lease::STATE_DEFAULT) { + // Set subnet level value. + stats_mgr.setValue(StatsMgr:: + generateName("subnet", row.subnet_id_, + "assigned-nas"), + row.state_count_); + } if (row.lease_state_ == Lease::STATE_DECLINED) { + // Set subnet level value. + stats_mgr.setValue(StatsMgr:: + generateName("subnet", row.subnet_id_, + "declined-addresses"), + row.state_count_); + + // Add to the global value. + stats_mgr.addValue("declined-addresses", row.state_count_); } break; case Lease::TYPE_PD: - switch(row.lease_state_) { - case Lease::STATE_DEFAULT: - // Set subnet level value. - stats_mgr.setValue(StatsMgr:: - generateName("subnet", - row.subnet_id_, - "assigned-pds"), - row.state_count_); - break; - default: - // Not one we're tracking. - break; + if (row.lease_state_ == Lease::STATE_DEFAULT) { + // Set subnet level value. + stats_mgr.setValue(StatsMgr:: + generateName("subnet", row.subnet_id_, + "assigned-pds"), + row.state_count_); } break; diff --git a/src/lib/dhcpsrv/lease_mgr.h b/src/lib/dhcpsrv/lease_mgr.h index 679b7b9b6c..456a237f66 100644 --- a/src/lib/dhcpsrv/lease_mgr.h +++ b/src/lib/dhcpsrv/lease_mgr.h @@ -163,7 +163,7 @@ struct AddressStatsRow4 { /// @param lease_state The lease state counted /// @param state_count The count of leases in the lease state AddressStatsRow4(const SubnetID& subnet_id, - const Lease::LeaseState& lease_state, + const uint32_t lease_state, const int64_t state_count) : subnet_id_(subnet_id), lease_state_(lease_state), state_count_(state_count) { @@ -172,7 +172,7 @@ struct AddressStatsRow4 { /// @brief The subnet ID to which this data applies SubnetID subnet_id_; /// @brief The lease_state to which the count applies - Lease::LeaseState lease_state_; + uint32_t lease_state_; /// @brief state_count The count of leases in the lease state int64_t state_count_; }; @@ -228,7 +228,7 @@ struct AddressStatsRow6 { /// @param lease_state The lease state counted /// @param state_count The count of leases in the lease state AddressStatsRow6(const SubnetID& subnet_id, const Lease::Type& lease_type, - const Lease::LeaseState& lease_state, + const uint32_t lease_state, const int64_t state_count) : subnet_id_(subnet_id), lease_type_(lease_type), lease_state_(lease_state), state_count_(state_count) { diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.cc b/src/lib/dhcpsrv/memfile_lease_mgr.cc index 2ffaa7c114..8f5313a997 100644 --- a/src/lib/dhcpsrv/memfile_lease_mgr.cc +++ b/src/lib/dhcpsrv/memfile_lease_mgr.cc @@ -324,16 +324,10 @@ public: } // Bump the appropriate accumulator - switch ((*lease)->state_) { - case Lease::STATE_DEFAULT: + if ((*lease)->state_ == Lease::STATE_DEFAULT) { ++assigned; - break; - case Lease::STATE_DECLINED: + } else if ((*lease)->state_ == Lease::STATE_DECLINED) { ++declined; - break; - default: - // Not one we're tracking. - break; } } @@ -459,8 +453,7 @@ public: } // Bump the appropriate accumulator - switch ((*lease)->state_) { - case Lease::STATE_DEFAULT: + if ((*lease)->state_ == Lease::STATE_DEFAULT) { switch((*lease)->type_) { case Lease::TYPE_NA: ++assigned; @@ -471,16 +464,11 @@ public: default: break; } - break; - case Lease::STATE_DECLINED: + } else if ((*lease)->state_ == Lease::STATE_DECLINED) { // In theory only NAs can be declined if (((*lease)->type_) == Lease::TYPE_NA) { ++declined; } - break; - default: - // Not one we're tracking. - break; } } diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.cc b/src/lib/dhcpsrv/mysql_lease_mgr.cc index 199911767a..7f7c1bf3ca 100644 --- a/src/lib/dhcpsrv/mysql_lease_mgr.cc +++ b/src/lib/dhcpsrv/mysql_lease_mgr.cc @@ -1302,7 +1302,7 @@ public: int status = mysql_stmt_fetch(statement_); if (status == MLM_MYSQL_FETCH_SUCCESS) { row.subnet_id_ = static_cast(subnet_id_); - row.lease_state_ = static_cast(lease_state_); + row.lease_state_ = lease_state_; row.state_count_ = state_count_; have_row = true; } else if (status != MYSQL_NO_DATA) { @@ -1430,7 +1430,7 @@ public: if (status == MLM_MYSQL_FETCH_SUCCESS) { row.subnet_id_ = static_cast(subnet_id_); row.lease_type_ = static_cast(lease_type_); - row.lease_state_ = static_cast(lease_state_); + row.lease_state_ = lease_state_; row.state_count_ = state_count_; have_row = true; } else if (status != MYSQL_NO_DATA) { diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.cc b/src/lib/dhcpsrv/pgsql_lease_mgr.cc index 13c3c2ee4d..2b92e69d6b 100644 --- a/src/lib/dhcpsrv/pgsql_lease_mgr.cc +++ b/src/lib/dhcpsrv/pgsql_lease_mgr.cc @@ -756,8 +756,8 @@ public: // Fetch the lease state. uint32_t state; - PgSqlExchange::getColumnValue(*result_set_, next_row_ , col, state); - row.lease_state_ = static_cast(state); + PgSqlExchange::getColumnValue(*result_set_, next_row_ , col, + row.lease_state_); ++col; // Fetch the state count. @@ -853,8 +853,8 @@ public: // Fetch the lease state. uint32_t state; - PgSqlExchange::getColumnValue(*result_set_, next_row_ , col, state); - row.lease_state_ = static_cast(state); + PgSqlExchange::getColumnValue(*result_set_, next_row_ , col, + row.lease_state_); ++col; // Fetch the state count. diff --git a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc index 3d411db9f0..399533da0e 100644 --- a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc @@ -2431,7 +2431,7 @@ GenericLeaseMgrTest::checkAddressStats(const StatValMapList& expectedStats) { void GenericLeaseMgrTest::makeLease4(const std::string& address, const SubnetID& subnet_id, - const Lease::LeaseState& state) { + const uint32_t state) { Lease4Ptr lease(new Lease4()); // set the address @@ -2455,7 +2455,7 @@ GenericLeaseMgrTest::makeLease6(const Lease::Type& type, const std::string& address, uint8_t prefix_len, const SubnetID& subnet_id, - const Lease::LeaseState& state) { + const uint32_t state) { IOAddress addr(address); // make a DUID from the address diff --git a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h index 6a7ae4d640..b3b1f66550 100644 --- a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h +++ b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h @@ -126,7 +126,7 @@ public: /// @param subnet_id - subnet ID to which the lease belongs /// @param state - the state of the lease void makeLease4(const std::string& address, const SubnetID& subnet_id, - const Lease::LeaseState& state = Lease::STATE_DEFAULT); + const uint32_t state = Lease::STATE_DEFAULT); /// @brief Constructs a minimal IPv6 lease and adds it to the lease storage /// @@ -139,7 +139,7 @@ public: /// @param state - the state of the lease void makeLease6(const Lease::Type& type, const std::string& address, uint8_t prefix_len, const SubnetID& subnet_id, - const Lease::LeaseState& state = Lease::STATE_DEFAULT); + const uint32_t state = Lease::STATE_DEFAULT); /// @brief checks that addLease, getLease4(addr) and deleteLease() works void testBasicLease4();