Parcourir la source

[3965] Lease state is the enum-like value, rather than bitfield.

Marcin Siodelski il y a 9 ans
Parent
commit
457428a354
3 fichiers modifiés avec 22 ajouts et 55 suppressions
  1. 14 33
      src/lib/dhcpsrv/lease.cc
  2. 8 13
      src/lib/dhcpsrv/lease.h
  3. 0 9
      src/lib/dhcpsrv/tests/lease_unittest.cc

+ 14 - 33
src/lib/dhcpsrv/lease.cc

@@ -27,9 +27,6 @@ const uint32_t Lease::STATE_DEFAULT = 0x0;
 const uint32_t Lease::STATE_DECLINED = 0x1;
 const uint32_t Lease::STATE_EXPIRED_RECLAIMED = 0x2;
 
-const unsigned int Lease::BASIC_STATES_NUM = 2;
-
-
 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,
@@ -62,36 +59,20 @@ Lease::typeToText(Lease::Type type) {
 
 std::string
 Lease::basicStatesToText(const uint32_t state) {
-    // Stream will hold comma separated list of states.
-    std::ostringstream s;
-    // Iterate over all defined states.
-    for (int i = 0; i < BASIC_STATES_NUM; ++i) {
-        // Test each bit of the lease state to see if it is set.
-        uint32_t single_state = (state & (static_cast<uint32_t>(1) << i));
-        // Only proceed if the bit is set.
-        if (single_state > 0) {
-            // Comma sign is applied only if any state has been found.
-            if (static_cast<int>(s.tellp() > 0)) {
-                s << ",";
-            }
-            // Check which bit is set and append state name.
-            switch (single_state) {
-            case STATE_DECLINED:
-                s << "declined";
-                break;
-
-            case STATE_EXPIRED_RECLAIMED:
-                s << "expired-reclaimed";
-                break;
-
-            default:
-                // This shouldn't really happen.
-                s << "unknown (" << i << ")";
-            }
-        }
+    switch (state) {
+    case STATE_DEFAULT:
+        return ("default");
+    case STATE_DECLINED:
+        return ("declined");
+    case STATE_EXPIRED_RECLAIMED:
+        return ("expired-reclaimed");
+    default:
+        // The default case will be handled further on
+        ;
     }
-
-    return (s.tellp() > 0 ? s.str() : "default");
+    std::ostringstream s;
+    s << "unknown (" << state << ")";
+    return s.str();
 }
 
 bool
@@ -101,7 +82,7 @@ Lease::expired() const {
 
 bool
 Lease::stateExpiredReclaimed() const {
-    return ((state_ & STATE_EXPIRED_RECLAIMED) != 0);
+    return (state_ == STATE_EXPIRED_RECLAIMED);
 }
 
 int64_t

+ 8 - 13
src/lib/dhcpsrv/lease.h

@@ -60,18 +60,13 @@ struct Lease {
     /// @brief Expired and reclaimed lease.
     static const uint32_t STATE_EXPIRED_RECLAIMED;
 
-    /// @brief Number of common states for DHCPv4 and DHCPv6.
-    ///
-    /// This constants holds the number of states used for both DHCPv4 and
-    /// DHCPv6 leases excluding the default state. If new states are defined,
-    /// this value must be adjusted accordingly.
-    static const unsigned int BASIC_STATES_NUM;
     //@}
 
     /// @brief Returns name(s) of the basic lease state(s).
     ///
-    /// @param state A numeric value holding a state information. Multiple bits
-    /// may be set to indicate that the lease is in multiple states.
+    /// @param state A numeric value holding a state information.
+    /// Some states may be composite, i.e. the single state value
+    /// maps to multiple logical states of the lease.
     ///
     /// @return Comma separated list of state names.
     static std::string basicStatesToText(const uint32_t state);
@@ -156,12 +151,12 @@ struct Lease {
     /// This information may not be available in certain cases.
     HWAddrPtr hwaddr_;
 
-    /// @brief Bitfield holding lease state(s).
+    /// @brief Holds the lease state(s).
     ///
-    /// This is a bitfield which holds the lease state. Typically, a lease
-    /// remains in a single state, but it is possible set multiple states
-    /// for a single lease. In this case, multiple bits of this bitfield
-    /// will be set.
+    /// This is the field that holds the lease state(s). Typically, a
+    /// lease remains in a single states. However, it is posible to
+    /// define a value for state which indicates that the lease remains
+    /// in multiple logical states.
     ///
     /// The defined states are represented by the "STATE_*" constants
     /// belonging to this class.

+ 0 - 9
src/lib/dhcpsrv/tests/lease_unittest.cc

@@ -450,11 +450,6 @@ TEST_F(Lease4Test, stateToText) {
     EXPECT_EQ("default", Lease4::statesToText(Lease::STATE_DEFAULT));
     EXPECT_EQ("declined", Lease4::statesToText(Lease::STATE_DECLINED));
     EXPECT_EQ("expired-reclaimed", Lease4::statesToText(Lease::STATE_EXPIRED_RECLAIMED));
-
-    // Try multiple states.
-    EXPECT_EQ("declined,expired-reclaimed",
-              Lease4::statesToText(Lease::STATE_DECLINED | Lease::STATE_EXPIRED_RECLAIMED));
-
 }
 
 /// @brief Creates an instance of the lease with certain FQDN data.
@@ -811,10 +806,6 @@ TEST(Lease6Test, stateToText) {
     EXPECT_EQ("default", Lease6::statesToText(Lease::STATE_DEFAULT));
     EXPECT_EQ("declined", Lease6::statesToText(Lease::STATE_DECLINED));
     EXPECT_EQ("expired-reclaimed", Lease6::statesToText(Lease::STATE_EXPIRED_RECLAIMED));
-
-    // Try multiple states.
-    EXPECT_EQ("declined,expired-reclaimed",
-              Lease6::statesToText(Lease::STATE_DECLINED | Lease::STATE_EXPIRED_RECLAIMED));
 }