Browse Source

[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.
Thomas Markwalder 8 years ago
parent
commit
86411b64ad

+ 4 - 0
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,

+ 10 - 12
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).
     ///

+ 34 - 59
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;
 

+ 3 - 3
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) {

+ 4 - 16
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;
             }
         }
 

+ 2 - 2
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<SubnetID>(subnet_id_);
-            row.lease_state_ = static_cast<Lease::LeaseState>(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<SubnetID>(subnet_id_);
             row.lease_type_ = static_cast<Lease::Type>(lease_type_);
-            row.lease_state_ = static_cast<Lease::LeaseState>(lease_state_);
+            row.lease_state_ = lease_state_;
             row.state_count_ = state_count_;
             have_row = true;
         } else if (status != MYSQL_NO_DATA) {

+ 4 - 4
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<Lease::LeaseState>(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<Lease::LeaseState>(state);
+        PgSqlExchange::getColumnValue(*result_set_, next_row_ , col, 
+                                      row.lease_state_);
         ++col;
 
         // Fetch the state count.

+ 2 - 2
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

+ 2 - 2
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();