[master] Merge branch 'trac2821'

src/lib/dhcpsrv/mysql_lease_mgr.cc

@@ -289,7 +289,7 @@ public:
         memset(hwaddr_buffer_, 0, sizeof(hwaddr_buffer_));
         memset(client_id_buffer_, 0, sizeof(client_id_buffer_));
         std::fill(&error_[0], &error_[LEASE_COLUMNS], MLM_FALSE);
- 
+
         // Set the column names (for error messages)
         columns_[0] = "address";
         columns_[1] = "hwaddr";
@@ -536,7 +536,7 @@ public:
         memset(addr6_buffer_, 0, sizeof(addr6_buffer_));
         memset(duid_buffer_, 0, sizeof(duid_buffer_));
         std::fill(&error_[0], &error_[LEASE_COLUMNS], MLM_FALSE);
- 
+
         // Set the column names (for error messages)
         columns_[0] = "address";
         columns_[1] = "duid";
@@ -809,7 +809,7 @@ private:
     // schema.
     // Note: arrays are declared fixed length for speed of creation
     std::string     addr6_;             ///< String form of address
-    char            addr6_buffer_[ADDRESS6_TEXT_MAX_LEN + 1];  ///< Character 
+    char            addr6_buffer_[ADDRESS6_TEXT_MAX_LEN + 1];  ///< Character
                                         ///< array form of V6 address
     unsigned long   addr6_length_;      ///< Length of the address
     MYSQL_BIND      bind_[LEASE_COLUMNS]; ///< Bind array
@@ -870,17 +870,10 @@ private:
     MYSQL_STMT*     statement_;     ///< Statement for which results are freed
 };
 
-
 // MySqlLeaseMgr Constructor and Destructor
 
-MySqlLeaseMgr::MySqlLeaseMgr(const LeaseMgr::ParameterMap& parameters) 
-    : LeaseMgr(parameters), mysql_(NULL) {
-
-    // Allocate context for MySQL - it is destroyed in the destructor.
-    mysql_ = mysql_init(NULL);
-    if (mysql_ == NULL) {
-        isc_throw(DbOpenError, "unable to initialize MySQL");
-    }
+MySqlLeaseMgr::MySqlLeaseMgr(const LeaseMgr::ParameterMap& parameters)
+    : LeaseMgr(parameters) {
 
     // Open the database.
     openDatabase();
@@ -916,9 +909,8 @@ MySqlLeaseMgr::~MySqlLeaseMgr() {
         }
     }
 
-    // Close the database
-    mysql_close(mysql_);
-    mysql_ = NULL;
+    // There is no need to close the database in this destructor: it is
+    // closed in the destructor of the mysql_ member variable.
 }
 
 
@@ -970,7 +962,7 @@ 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 
+    expire_tm.tm_isdst = -1;    // Let the system work out about DST
 
     // Convert to local time
     cltt = mktime(&expire_tm) - valid_lifetime;
@@ -1022,7 +1014,7 @@ MySqlLeaseMgr::openDatabase() {
     }
 
     // Set options for the connection:
-    // 
+    //
     // Automatic reconnection: after a period of inactivity, the client will
     // disconnect from the database.  This option causes it to automatically
     // reconnect when another operation is about to be done.
@@ -1088,7 +1080,7 @@ MySqlLeaseMgr::prepareStatements() {
     // Allocate space for all statements
     statements_.clear();
     statements_.resize(NUM_STATEMENTS, NULL);
-    
+
     text_statements_.clear();
     text_statements_.resize(NUM_STATEMENTS, std::string(""));
 

src/lib/dhcpsrv/mysql_lease_mgr.h

@@ -19,6 +19,7 @@
 #include <dhcpsrv/lease_mgr.h>
 
 #include <boost/scoped_ptr.hpp>
+#include <boost/utility.hpp>
 #include <mysql/mysql.h>
 
 #include <time.h>
@@ -26,6 +27,54 @@
 namespace isc {
 namespace dhcp {
 
+/// @brief MySQL Handle Holder
+///
+/// Small RAII object for safer initialization, will close the database
+/// connection upon destruction.  This means that if an exception is thrown
+/// during database initialization, resources allocated to the database are
+/// guaranteed to be freed.
+///
+/// It makes no sense to copy an object of this class.  After the copy, both
+/// objects would contain pointers to the same MySql context object.  The
+/// destruction of one would invalid the context in the remaining object.
+/// For this reason, the class is declared noncopyable.
+class MySqlHolder : public boost::noncopyable {
+public:
+
+    /// @brief Constructor
+    ///
+    /// Initialize MySql and store the associated context object.
+    ///
+    /// @throw DbOpenError Unable to initialize MySql handle.
+    MySqlHolder() : mysql_(mysql_init(NULL)) {
+        if (mysql_ == NULL) {
+            isc_throw(DbOpenError, "unable to initialize MySQL");
+        }
+    }
+
+    /// @brief Destructor
+    ///
+    /// Frees up resources allocated by the initialization of MySql.
+    ~MySqlHolder() {
+        if (mysql_ != NULL) {
+            mysql_close(mysql_);
+        }
+        // The library itself shouldn't be needed anymore
+        mysql_library_end();
+    }
+
+    /// @brief Conversion Operator
+    ///
+    /// Allows the MySqlHolder object to be passed as the context argument to
+    /// mysql_xxx functions.
+    operator MYSQL*() const {
+        return (mysql_);
+    }
+
+private:
+    MYSQL* mysql_;      ///< Initialization context
+};
+
 // Define the current database schema values
 
 const uint32_t CURRENT_VERSION_VERSION = 1;
@@ -379,7 +428,7 @@ public:
     /// @param cltt Reference to location where client last transmit time
     ///        is put.
     static
-    void convertFromDatabaseTime(const MYSQL_TIME& expire, 
+    void convertFromDatabaseTime(const MYSQL_TIME& expire,
                                  uint32_t valid_lifetime, time_t& cltt);
     ///@}
 
@@ -616,7 +665,7 @@ private:
     /// declare them as "mutable".)
     boost::scoped_ptr<MySqlLease4Exchange> exchange4_; ///< Exchange object
     boost::scoped_ptr<MySqlLease6Exchange> exchange6_; ///< Exchange object
-    MYSQL*              mysql_;                 ///< MySQL context object
+    MySqlHolder mysql_;
     std::vector<MYSQL_STMT*> statements_;       ///< Prepared statements
     std::vector<std::string> text_statements_;  ///< Raw text of statements
 };

src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc

@@ -118,21 +118,16 @@ validConnectionString() {
 // There is no error checking in this code: if something fails, one of the
 // tests will (should) fall over.
 void destroySchema() {
-    // Initialise
-    MYSQL handle;
-    (void) mysql_init(&handle);
+    MySqlHolder mysql;
 
     // Open database
-    (void) mysql_real_connect(&handle, "localhost", "keatest",
+    (void) mysql_real_connect(mysql, "localhost", "keatest",
                               "keatest", "keatest", 0, NULL, 0);
 
     // Get rid of everything in it.
     for (int i = 0; destroy_statement[i] != NULL; ++i) {
-        (void) mysql_query(&handle, destroy_statement[i]);
+        (void) mysql_query(mysql, destroy_statement[i]);
     }
-
-    // ... and close
-    (void) mysql_close(&handle);
 }
 
 // @brief Create the Schema
@@ -142,21 +137,16 @@ void destroySchema() {
 // There is no error checking in this code: if it fails, one of the tests
 // will fall over.
 void createSchema() {
-    // Initialise
-    MYSQL handle;
-    (void) mysql_init(&handle);
+    MySqlHolder mysql;
 
     // Open database
-    (void) mysql_real_connect(&handle, "localhost", "keatest",
+    (void) mysql_real_connect(mysql, "localhost", "keatest",
                               "keatest", "keatest", 0, NULL, 0);
 
     // Execute creation statements.
     for (int i = 0; create_statement[i] != NULL; ++i) {
-        (void) mysql_query(&handle, create_statement[i]);
+        (void) mysql_query(mysql, create_statement[i]);
     }
-
-    // ... and close
-    (void) mysql_close(&handle);
 }
 
 /// @brief Test fixture class for testing MySQL Lease Manager

src/lib/dhcpsrv/tests/run_unittests.cc

@@ -12,6 +12,7 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <config.h>
 #include <log/logger_support.h>
 
 #include <gtest/gtest.h>