From: Stephen Morris Date: Wed, 24 Oct 2012 14:36:54 +0000 (+0100) Subject: [2342] Fix problems with time conversion X-Git-Tag: trac2487_base~1^2~52 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=3e52b2fec4e3fed3898964ad0ae10d899b31c8bb;p=thirdparty%2Fkea.git [2342] Fix problems with time conversion ... traced to not initializing a "struct" properly. --- diff --git a/src/lib/dhcp/dhcpdb_create.mysql b/src/lib/dhcp/dhcpdb_create.mysql index 02cc374926..b888b599d5 100644 --- a/src/lib/dhcp/dhcpdb_create.mysql +++ b/src/lib/dhcp/dhcpdb_create.mysql @@ -46,7 +46,7 @@ CREATE TABLE lease6 ( address VARCHAR(40) PRIMARY KEY NOT NULL, # IPv6 address hwaddr VARBINARY(20), # Hardware address duid VARBINARY(128), # DUID - lease_time INT UNSIGNED, # Length of the lease (seconds) + valid_lifetime INT UNSIGNED, # Length of the lease (seconds) expire TIMESTAMP, # Expiration time of the lease subnet_id INT UNSIGNED, # Subnet identification pref_lifetime INT UNSIGNED, # Preferred lifetime diff --git a/src/lib/dhcp/mysql_lease_mgr.cc b/src/lib/dhcp/mysql_lease_mgr.cc index 7479bf8895..0d7a4edd76 100644 --- a/src/lib/dhcp/mysql_lease_mgr.cc +++ b/src/lib/dhcp/mysql_lease_mgr.cc @@ -111,23 +111,19 @@ public: bind_[2].buffer_length = duid_length_; bind_[2].length = &duid_length_; - // The lease structure holds the client last transmission time (cltt_) - // and the valid lifetime (valid_lft_). For convenience for external - // tools, the data stored in the database is expiry time (expire) and - // lease time (lease+time). The relationship is given by: - // - // lease_time - valid_lft_ - // expire = cltt_ + valid_lft_ - // - MySqlLeaseMgr::convertToDatabaseTime(lease_->cltt_, lease_->valid_lft_, - expire_, lease_time_); - // lease_time: unsigned int bind_[3].buffer_type = MYSQL_TYPE_LONG; - bind_[3].buffer = reinterpret_cast(&lease_time_); + bind_[3].buffer = reinterpret_cast(&lease->valid_lft_); bind_[3].is_unsigned = true_; // expire: timestamp + // The lease structure holds the client last transmission time (cltt_) + // For convenience for external tools, this is converted to lease + /// expiry time (expire). The relationship is given by: + // + // expire = cltt_ + valid_lft_ + MySqlLeaseMgr::convertToDatabaseTime(lease_->cltt_, lease_->valid_lft_, + expire_); bind_[4].buffer_type = MYSQL_TYPE_TIMESTAMP; bind_[4].buffer = reinterpret_cast(&expire_); bind_[4].buffer_length = sizeof(expire_); @@ -210,7 +206,7 @@ public: // lease_time: unsigned int bind_[3].buffer_type = MYSQL_TYPE_LONG; - bind_[3].buffer = reinterpret_cast(&lease_time_); + bind_[3].buffer = reinterpret_cast(&valid_lifetime_); bind_[3].is_unsigned = true_; bind_[3].error = &error_[3]; @@ -269,16 +265,20 @@ public: Lease6Ptr result(new Lease6()); // Success - put the data in the lease object + + // The address buffer is declared larger than the buffer size passed + // to the access function so that we can always append a null byte. addr6_buffer_[addr6_length_] = '\0'; std::string address = addr6_buffer_; - result->addr_ = isc::asiolink::IOAddress(address); + // Set the other data, converting time as needed. + result->addr_ = isc::asiolink::IOAddress(address); result->hwaddr_ = vector(&hwaddr_buffer_[0], &hwaddr_buffer_[hwaddr_length_]); result->duid_.reset(new DUID(duid_buffer_, duid_length_)); - MySqlLeaseMgr::convertFromDatabaseTime(expire_, lease_time_, - result->cltt_, - result->valid_lft_); + MySqlLeaseMgr::convertFromDatabaseTime(expire_, valid_lifetime_, + result->cltt_); + result->valid_lft_ = valid_lifetime_; result->subnet_id_ = subnet_id_; result->preferred_lft_ = pref_lifetime_; @@ -323,7 +323,7 @@ private: unsigned long hwaddr_length_; ///< Length of hardware address uint32_t iaid_; ///< Identity association ID Lease6Ptr lease_; ///< Pointer to lease object - uint32_t lease_time_; ///< Lease time + uint32_t valid_lifetime_; ///< Lease time uint8_t lease_type_; ///< Lease type uint8_t prefixlen_; ///< Prefix length uint32_t pref_lifetime_; ///< Preferred lifetime @@ -337,15 +337,21 @@ private: // // Note that the MySQL TIMESTAMP data type (used for "expire") converts data // from the current timezone to UTC for storage, and from UTC to the current -// timezone for retrieval. This means that the external interface - cltt - -// must be local time. +// timezone for retrieval. +// +// This causes no problems providing that: +// a) cltt is given in local time +// b) We let the system take care of timezone conversion when converting +// from a time read from the database into a local time. void -MySqlLeaseMgr::convertToDatabaseTime(time_t cltt, uint32_t valid_lft, - MYSQL_TIME& expire, uint32_t& lease_time) { +MySqlLeaseMgr::convertToDatabaseTime(time_t cltt, uint32_t valid_lifetime, + MYSQL_TIME& expire) { // Calculate expiry time and convert to various date/time fields. - time_t expire_time = cltt + valid_lft; + time_t expire_time = cltt + valid_lifetime; + + // Convert to broken-out time struct tm expire_tm; (void) localtime_r(&expire_time, &expire_tm); @@ -358,19 +364,15 @@ MySqlLeaseMgr::convertToDatabaseTime(time_t cltt, uint32_t valid_lft, expire.second = expire_tm.tm_sec; expire.second_part = 0; // No fractional seconds expire.neg = static_cast(0); // Not negative - - // Set the lease time. - lease_time = valid_lft; } void MySqlLeaseMgr::convertFromDatabaseTime(const MYSQL_TIME& expire, - uint32_t lease_time, time_t& cltt, - uint32_t& valid_lft) { - valid_lft = lease_time; + uint32_t valid_lifetime, time_t& cltt) { // Copy across fields from MYSQL_TIME structure. struct tm expire_tm; + memset(&expire_tm, 0, sizeof(expire_tm)); expire_tm.tm_year = expire.year - 1900; expire_tm.tm_mon = expire.month - 1; @@ -378,9 +380,10 @@ MySqlLeaseMgr::convertFromDatabaseTime(const MYSQL_TIME& expire, expire_tm.tm_hour = expire.hour; expire_tm.tm_min = expire.minute; expire_tm.tm_sec = expire.second; + expire_tm.tm_isdst = -1; // Let the system work out about DST // Convert to local time - cltt = mktime(&expire_tm) - valid_lft; + cltt = mktime(&expire_tm) - valid_lifetime; } void @@ -474,14 +477,14 @@ MySqlLeaseMgr::prepareStatements() { "DELETE FROM lease6 WHERE address = ?"); prepareStatement(GET_LEASE6, "SELECT address, hwaddr, duid, " - "lease_time, expire, subnet_id, pref_lifetime, " + "valid_lifetime, expire, subnet_id, pref_lifetime, " "lease_type, iaid, prefix_len " "FROM lease6 WHERE address = ?"); prepareStatement(GET_VERSION, "SELECT version, minor FROM schema_version"); prepareStatement(INSERT_LEASE6, "INSERT INTO lease6(address, hwaddr, duid, " - "lease_time, expire, subnet_id, pref_lifetime, " + "valid_lifetime, expire, subnet_id, pref_lifetime, " "lease_type, iaid, prefix_len) " "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"); } diff --git a/src/lib/dhcp/mysql_lease_mgr.h b/src/lib/dhcp/mysql_lease_mgr.h index 84a6ac7ca3..375a0eec78 100644 --- a/src/lib/dhcp/mysql_lease_mgr.h +++ b/src/lib/dhcp/mysql_lease_mgr.h @@ -262,36 +262,32 @@ public: /// @brief Convert Lease Time to Database Times /// - /// Within the DHCP servers, times are stored as cltt (client last transmit - /// time) and valid_lft (valid lifetime). In the database, the information - /// is stored as lease_time (lease time) and expire (time of expiry of the - /// lease). They are related by the equations: + /// Within the DHCP servers, times are stored as client last transmit time + /// and valid lifetime. In the database, the information is stored as + /// valid lifetime and "expire" (time of expiry of the lease). They are + /// related by the equation: /// - /// - lease_time = valid_lft - /// - expire = cltt + valid_lft + /// - expire = client last transmit time + valid lifetime /// /// This method converts from the times in the lease object into times /// able to be added to the database. /// /// @param cltt Client last transmit time - /// @param valid_lft Valid lifetime + /// @param valid_lifetime Valid lifetime /// @param expire Reference to MYSQL_TIME object where the expiry time of /// the lease will be put. - /// @param lease_time Reference to the time_t object where the lease time - /// will be put. static - void convertToDatabaseTime(time_t cltt, uint32_t valid_lft, - MYSQL_TIME& expire, uint32_t& lease_time); + void convertToDatabaseTime(time_t cltt, uint32_t valid_lifetime, + MYSQL_TIME& expire); /// @brief Convert Database Time to Lease Times /// - /// Within the database, time is stored as lease_time (lease time) and - /// expire (time of expiry of the lease). In the DHCP server, the - /// information is stored as cltt (client last transmit time) and - /// valid_lft (valid lifetime). These are related by the equations: + /// Within the database, time is stored as "expire" (time of expiry of the + /// lease) and valid lifetime. In the DHCP server, the information is + /// stored client last transmit time and valid lifetime. These are related + /// by the equation: /// - /// - valid_lft = lease_time - /// - cltt = expire - lease_time + /// - client last transmit time = expire - valid_lifetime /// /// This method converts from the times in the database into times /// able to be inserted into the lease object. @@ -301,10 +297,9 @@ public: /// @param lease_time lifetime of the lease. /// @param cltt Reference to location where client last transmit time /// is put. - /// @param valid_lft Reference to location where valid lifetime is put. static - void convertFromDatabaseTime(const MYSQL_TIME& expire, uint32_t lease_time, - time_t& cltt, uint32_t& valid_lft); + void convertFromDatabaseTime(const MYSQL_TIME& expire, + uint32_t valid_lifetime, time_t& cltt); ///@} diff --git a/src/lib/dhcp/tests/mysql_lease_mgr_unittest.cc b/src/lib/dhcp/tests/mysql_lease_mgr_unittest.cc index 2739a90bb7..74d769aa5f 100644 --- a/src/lib/dhcp/tests/mysql_lease_mgr_unittest.cc +++ b/src/lib/dhcp/tests/mysql_lease_mgr_unittest.cc @@ -132,21 +132,16 @@ TEST_F(MySqlLeaseMgrTest, CheckTimeConversion) { const time_t cltt = time(NULL); const uint32_t valid_lft = 86400; // 1 day MYSQL_TIME expire; - uint32_t lease_time; - MySqlLeaseMgr::convertToDatabaseTime(cltt, valid_lft, expire, lease_time); - EXPECT_EQ(valid_lft, lease_time); + MySqlLeaseMgr::convertToDatabaseTime(cltt, valid_lft, expire); EXPECT_LE(2012, expire.year); // Code was written in 2012 EXPECT_EQ(0, expire.second_part); EXPECT_EQ(0, expire.neg); // Convert back time_t converted_cltt = 0; - uint32_t converted_valid_lft = 0; - MySqlLeaseMgr::convertFromDatabaseTime(expire, lease_time, converted_cltt, - converted_valid_lft); + MySqlLeaseMgr::convertFromDatabaseTime(expire, valid_lft, converted_cltt); EXPECT_EQ(cltt, converted_cltt); - EXPECT_EQ(valid_lft, converted_valid_lft); } /// @brief Check that getVersion() works