[2404] Updated comments etc.

src/lib/dhcp/duid.h

@@ -45,13 +45,13 @@ class DUID {
         DUID_MAX          ///< not a real type, just maximum defined value + 1
     } DUIDType;
 
-    /// @brief creates a DUID
+    /// @brief Constructor from vector
     DUID(const std::vector<uint8_t>& duid);
 
-    /// @brief creates a DUID
+    /// @brief Constructor from array and array size
     DUID(const uint8_t* duid, size_t len);
 
-    /// @brief returns a const reference to the actual DUID value
+    /// @brief Returns a const reference to the actual DUID value
     ///
     /// Note: For safety reasons, this method returns a copy of data as
     /// otherwise the reference would be only valid as long as the object that
@@ -60,47 +60,52 @@ class DUID {
     /// (e.g. storeSelf()) that will avoid data copying.
     const std::vector<uint8_t> getDuid() const;
 
-    /// @brief returns DUID type
+    /// @brief Returns the DUID type
     DUIDType getType() const;
 
-    /// returns textual prepresentation (e.g. 00:01:02:03:ff)
+    /// @brief Returns textual representation of a DUID (e.g. 00:01:02:03:ff)
     std::string toText() const;
 
-    /// compares two DUIDs
+    /// @brief Compares two DUIDs for equality
     bool operator==(const DUID& other) const;
 
-    /// compares two DUIDs
+    /// @brief Compares two DUIDs for inequality
     bool operator!=(const DUID& other) const;
 
  protected:
-    /// the actual content of the DUID
+    /// The actual content of the DUID
     std::vector<uint8_t> duid_;
 };
 
+/// @brief Shared pointer to a DUID
 typedef boost::shared_ptr<DUID> DuidPtr;
 
+
+
 /// @brief Holds Client identifier or client IPv4 address
 ///
 /// This class is intended to be a generic IPv4 client identifier. It can hold
 /// a client-id
 class ClientId : DUID {
 public:
+    /// @brief Maximum size of a client ID
+    ///
+    /// This is the same as the maximum size of the underlying DUID.
     static const size_t MAX_CLIENT_ID_LEN = DUID::MAX_DUID_LEN;
 
-    /// constructor based on vector<uint8_t>
+    /// @brief Constructor based on vector<uint8_t>
     ClientId(const std::vector<uint8_t>& clientid);
 
-    /// constructor based on C-style data
+    /// @brief Constructor based on array and array size
     ClientId(const uint8_t* clientid, size_t len);
 
-    /// @brief returns reference to the client-id data
-    ///
+    /// @brief Returns reference to the client-id data
     const std::vector<uint8_t> getClientId() const;
 
-    // compares two client-ids
+    /// @brief Compares two client-ids for equality
     bool operator==(const ClientId& other) const;
 
-    // compares two client-ids
+    /// @brief Compares two client-ids for inequality
     bool operator!=(const ClientId& other) const;
 };
 

src/lib/dhcpsrv/lease_mgr.h

@@ -115,8 +115,8 @@ public:
 /// would be required. As this is a critical part of the code that will be used
 /// extensively, direct access is warranted.
 struct Lease4 {
-    // The following constants definine the size of fields in the database
-    static const size_t HWADDR_MAX = 20;     // Maximum size of hardware address
+    /// @brief Maximum size of a hardware address
+    static const size_t HWADDR_MAX = 20;
 
     /// @brief Constructor
     ///
@@ -392,7 +392,7 @@ struct Lease6 {
 /// @brief Pointer to a Lease6 structure.
 typedef boost::shared_ptr<Lease6> Lease6Ptr;
 
-/// @brief Const pointer to a Lease6 structure.
+/// @brief Pointer to a const Lease6 structure.
 typedef boost::shared_ptr<const Lease6> ConstLease6Ptr;
 
 /// @brief A collection of IPv6 leases.

src/lib/dhcpsrv/mysql_lease_mgr.cc

@@ -15,6 +15,7 @@
 #include <config.h>
 
 #include <asiolink/io_address.h>
+#include <dhcp/duid.h>
 #include <dhcpsrv/mysql_lease_mgr.h>
 
 #include <boost/static_assert.hpp>
@@ -91,18 +92,9 @@ namespace {
 /// colon separators.
 const size_t ADDRESS6_TEXT_MAX_LEN = 39;
 
-/// @brief Maximum size of a DUID.
-const size_t DUID_MAX_LEN = 128;
-
 /// @brief Maximum size of a hardware address.
 const size_t HWADDR_MAX_LEN = 20;
 
-/// @brief Maximum size of a client identification.
-///
-/// Note that the value is arbitrarily chosen: RFC 2131 does not specify an
-/// upper limit, but this seems long enough.
-const size_t CLIENT_ID_MAX_LEN = 128;
-
 /// @brief Number of columns in Lease4 table
 const size_t LEASE4_COLUMNS = 6;
 
@@ -156,7 +148,6 @@ TaggedStatement tagged_statements[] = {
                         "valid_lifetime, expire, subnet_id "
                             "FROM lease4 "
                             "WHERE hwaddr = ?"},
-
     {MySqlLeaseMgr::GET_LEASE4_HWADDR_SUBID,
                     "SELECT address, hwaddr, client_id, "
                         "valid_lifetime, expire, subnet_id "
@@ -496,7 +487,7 @@ private:
                                         ///< Hardware address buffer
     unsigned long   hwaddr_length_;     ///< Hardware address length
     std::vector<uint8_t> client_id_;    ///< Client identification
-    uint8_t         client_id_buffer_[CLIENT_ID_MAX_LEN];
+    uint8_t         client_id_buffer_[ClientId::MAX_CLIENT_ID_LEN];
                                         ///< Client ID buffer
     unsigned long   client_id_length_;  ///< Client ID address length
     MYSQL_TIME      expire_;            ///< Lease expiry time
@@ -812,7 +803,7 @@ private:
     MYSQL_BIND      bind_[LEASE_COLUMNS]; ///< Bind array
     std::string     columns_[LEASE_COLUMNS];///< Column names
     std::vector<uint8_t> duid_;         ///< Client identification
-    uint8_t         duid_buffer_[DUID_MAX_LEN]; ///< Buffer form of DUID
+    uint8_t         duid_buffer_[DUID::MAX_DUID_LEN]; ///< Buffer form of DUID
     unsigned long   duid_length_;       ///< Length of the DUID
     my_bool         error_[LEASE_COLUMNS]; ///< Error indicators
     MYSQL_TIME      expire_;            ///< Lease expiry time
@@ -1222,7 +1213,8 @@ void MySqlLeaseMgr::getLeaseCollection(StatementIndex stindex,
         checkError(status, stindex, "unable to fetch results");
     } else if (status == MYSQL_DATA_TRUNCATED) {
         // Data truncated - throw an exception indicating what was at fault
-        isc_throw(DataTruncated, "returned data truncated column affected: "
+        isc_throw(DataTruncated, text_statements_[stindex]
+                  << " returned truncated data: columns affected are "
                   << exchange->getErrorColumns());
     }
 }

src/lib/dhcpsrv/tests/lease_mgr_unittest.cc

@@ -345,21 +345,21 @@ TEST(Lease4, OperatorEquals) {
     EXPECT_TRUE(lease1 != lease2);
     lease1.addr_ = lease2.addr_;
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
-    EXPECT_FALSE(lease1 != lease2); // ... lease equal
+    EXPECT_FALSE(lease1 != lease2); // ... leases equal
 
     ++lease1.ext_;
     EXPECT_FALSE(lease1 == lease2);
     EXPECT_TRUE(lease1 != lease2);
     lease1.ext_ = lease2.ext_;
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
-    EXPECT_FALSE(lease1 != lease2); // ... lease equal
+    EXPECT_FALSE(lease1 != lease2); // ... leases equal
 
     ++lease1.hwaddr_[0];
     EXPECT_FALSE(lease1 == lease2);
     EXPECT_TRUE(lease1 != lease2);
     lease1.hwaddr_ = lease2.hwaddr_;
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
-    EXPECT_FALSE(lease1 != lease2); // ... lease equal
+    EXPECT_FALSE(lease1 != lease2); // ... leases equal
 
     ++clientid_vec[0];
     lease1.client_id_.reset(new ClientId(clientid_vec));
@@ -368,77 +368,77 @@ TEST(Lease4, OperatorEquals) {
     --clientid_vec[0];
     lease1.client_id_.reset(new ClientId(clientid_vec));
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
-    EXPECT_FALSE(lease1 != lease2); // ... lease equal
+    EXPECT_FALSE(lease1 != lease2); // ... leases equal
 
     ++lease1.t1_;
     EXPECT_FALSE(lease1 == lease2);
     EXPECT_TRUE(lease1 != lease2);
     lease1.t1_ = lease2.t1_;
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
-    EXPECT_FALSE(lease1 != lease2); // ... lease equal
+    EXPECT_FALSE(lease1 != lease2); // ... leases equal
 
     ++lease1.t2_;
     EXPECT_FALSE(lease1 == lease2);
     EXPECT_TRUE(lease1 != lease2);
     lease1.t2_ = lease2.t2_;
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
-    EXPECT_FALSE(lease1 != lease2); // ... lease equal
+    EXPECT_FALSE(lease1 != lease2); // ... leases equal
 
     ++lease1.valid_lft_;
     EXPECT_FALSE(lease1 == lease2);
     EXPECT_TRUE(lease1 != lease2);
     lease1.valid_lft_ = lease2.valid_lft_;
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
-    EXPECT_FALSE(lease1 != lease2); // ... lease equal
+    EXPECT_FALSE(lease1 != lease2); // ... leases equal
 
     ++lease1.cltt_;
     EXPECT_FALSE(lease1 == lease2);
     EXPECT_TRUE(lease1 != lease2);
     lease1.cltt_ = lease2.cltt_;
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
-    EXPECT_FALSE(lease1 != lease2); // ... lease equal
+    EXPECT_FALSE(lease1 != lease2); // ... leases equal
 
     ++lease1.subnet_id_;
     EXPECT_FALSE(lease1 == lease2);
     EXPECT_TRUE(lease1 != lease2);
     lease1.subnet_id_ = lease2.subnet_id_;
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
-    EXPECT_FALSE(lease1 != lease2); // ... lease equal
+    EXPECT_FALSE(lease1 != lease2); // ... leases equal
 
     lease1.fixed_ = !lease1.fixed_;
     EXPECT_FALSE(lease1 == lease2);
     EXPECT_TRUE(lease1 != lease2);
     lease1.fixed_ = lease2.fixed_;
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
-    EXPECT_FALSE(lease1 != lease2); // ... lease equal
+    EXPECT_FALSE(lease1 != lease2); // ... leases equal
 
     lease1.hostname_ += string("Something random");
     EXPECT_FALSE(lease1 == lease2);
     EXPECT_TRUE(lease1 != lease2);
     lease1.hostname_ = lease2.hostname_;
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
-    EXPECT_FALSE(lease1 != lease2); // ... lease equal
+    EXPECT_FALSE(lease1 != lease2); // ... leases equal
 
     lease1.fqdn_fwd_ = !lease1.fqdn_fwd_;
     EXPECT_FALSE(lease1 == lease2);
     EXPECT_TRUE(lease1 != lease2);
     lease1.fqdn_fwd_ = lease2.fqdn_fwd_;
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
-    EXPECT_FALSE(lease1 != lease2); // ... lease equal
+    EXPECT_FALSE(lease1 != lease2); // ... leases equal
 
     lease1.fqdn_rev_ = !lease1.fqdn_rev_;
     EXPECT_FALSE(lease1 == lease2);
     EXPECT_TRUE(lease1 != lease2);
     lease1.fqdn_rev_ = lease2.fqdn_rev_;
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
-    EXPECT_FALSE(lease1 != lease2); // ... lease equal
+    EXPECT_FALSE(lease1 != lease2); // ... leases equal
 
     lease1.comments_ += string("Something random");
     EXPECT_FALSE(lease1 == lease2);
     EXPECT_TRUE(lease1 != lease2);
     lease1.comments_ = lease2.comments_;
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
-    EXPECT_FALSE(lease1 != lease2); // ... lease equal
+    EXPECT_FALSE(lease1 != lease2); // ... leases equal
 }
 
 
@@ -514,28 +514,28 @@ TEST(Lease6, OperatorEquals) {
     EXPECT_TRUE(lease1 != lease2);
     lease1.addr_ = lease2.addr_;
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
-    EXPECT_FALSE(lease1 != lease2); // ... lease equal
+    EXPECT_FALSE(lease1 != lease2); // ... leases equal
 
     lease1.type_ = Lease6::LEASE_IA_PD;
     EXPECT_FALSE(lease1 == lease2);
     EXPECT_TRUE(lease1 != lease2);
     lease1.type_ = lease2.type_;
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
-    EXPECT_FALSE(lease1 != lease2); // ... lease equal
+    EXPECT_FALSE(lease1 != lease2); // ... leases equal
 
     ++lease1.prefixlen_;
     EXPECT_FALSE(lease1 == lease2);
     EXPECT_TRUE(lease1 != lease2);
     lease1.prefixlen_ = lease2.prefixlen_;
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
-    EXPECT_FALSE(lease1 != lease2); // ... lease equal
+    EXPECT_FALSE(lease1 != lease2); // ... leases equal
 
     ++lease1.iaid_;
     EXPECT_FALSE(lease1 == lease2);
     EXPECT_TRUE(lease1 != lease2);
     lease1.iaid_ = lease2.iaid_;
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
-    EXPECT_FALSE(lease1 != lease2); // ... lease equal
+    EXPECT_FALSE(lease1 != lease2); // ... leases equal
 
     ++duid_array[0];
     lease1.duid_.reset(new DUID(duid_array, sizeof(duid_array)));
@@ -544,83 +544,83 @@ TEST(Lease6, OperatorEquals) {
     --duid_array[0];
     lease1.duid_.reset(new DUID(duid_array, sizeof(duid_array)));
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
-    EXPECT_FALSE(lease1 != lease2); // ... lease equal
+    EXPECT_FALSE(lease1 != lease2); // ... leases equal
 
     ++lease1.preferred_lft_;
     EXPECT_FALSE(lease1 == lease2);
     EXPECT_TRUE(lease1 != lease2);
     lease1.preferred_lft_ = lease2.preferred_lft_;
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
-    EXPECT_FALSE(lease1 != lease2); // ... lease equal
+    EXPECT_FALSE(lease1 != lease2); // ... leases equal
 
     ++lease1.valid_lft_;
     EXPECT_FALSE(lease1 == lease2);
     EXPECT_TRUE(lease1 != lease2);
     lease1.valid_lft_ = lease2.valid_lft_;
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
-    EXPECT_FALSE(lease1 != lease2); // ... lease equal
+    EXPECT_FALSE(lease1 != lease2); // ... leases equal
 
     ++lease1.t1_;
     EXPECT_FALSE(lease1 == lease2);
     EXPECT_TRUE(lease1 != lease2);
     lease1.t1_ = lease2.t1_;
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
-    EXPECT_FALSE(lease1 != lease2); // ... lease equal
+    EXPECT_FALSE(lease1 != lease2); // ... leases equal
 
     ++lease1.t2_;
     EXPECT_FALSE(lease1 == lease2);
     EXPECT_TRUE(lease1 != lease2);
     lease1.t2_ = lease2.t2_;
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
-    EXPECT_FALSE(lease1 != lease2); // ... lease equal
+    EXPECT_FALSE(lease1 != lease2); // ... leases equal
 
     ++lease1.cltt_;
     EXPECT_FALSE(lease1 == lease2);
     EXPECT_TRUE(lease1 != lease2);
     lease1.cltt_ = lease2.cltt_;
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
-    EXPECT_FALSE(lease1 != lease2); // ... lease equal
+    EXPECT_FALSE(lease1 != lease2); // ... leases equal
 
     ++lease1.subnet_id_;
     EXPECT_FALSE(lease1 == lease2);
     EXPECT_TRUE(lease1 != lease2);
     lease1.subnet_id_ = lease2.subnet_id_;
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
-    EXPECT_FALSE(lease1 != lease2); // ... lease equal
+    EXPECT_FALSE(lease1 != lease2); // ... leases equal
 
     lease1.fixed_ = !lease1.fixed_;
     EXPECT_FALSE(lease1 == lease2);
     EXPECT_TRUE(lease1 != lease2);
     lease1.fixed_ = lease2.fixed_;
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
-    EXPECT_FALSE(lease1 != lease2); // ... lease equal
+    EXPECT_FALSE(lease1 != lease2); // ... leases equal
 
     lease1.hostname_ += string("Something random");
     EXPECT_FALSE(lease1 == lease2);
     EXPECT_TRUE(lease1 != lease2);
     lease1.hostname_ = lease2.hostname_;
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
-    EXPECT_FALSE(lease1 != lease2); // ... lease equal
+    EXPECT_FALSE(lease1 != lease2); // ... leases equal
 
     lease1.fqdn_fwd_ = !lease1.fqdn_fwd_;
     EXPECT_FALSE(lease1 == lease2);
     EXPECT_TRUE(lease1 != lease2);
     lease1.fqdn_fwd_ = lease2.fqdn_fwd_;
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
-    EXPECT_FALSE(lease1 != lease2); // ... lease equal
+    EXPECT_FALSE(lease1 != lease2); // ... leases equal
 
     lease1.fqdn_rev_ = !lease1.fqdn_rev_;
     EXPECT_FALSE(lease1 == lease2);
     EXPECT_TRUE(lease1 != lease2);
     lease1.fqdn_rev_ = lease2.fqdn_rev_;
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
-    EXPECT_FALSE(lease1 != lease2); // ... lease equal
+    EXPECT_FALSE(lease1 != lease2); // ... leases equal
 
     lease1.comments_ += string("Something random");
     EXPECT_FALSE(lease1 == lease2);
     EXPECT_TRUE(lease1 != lease2);
     lease1.comments_ = lease2.comments_;
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
-    EXPECT_FALSE(lease1 != lease2); // ... lease equal
+    EXPECT_FALSE(lease1 != lease2); // ... leases equal
 }
 }; // end of anonymous namespace