BIND 10 trac2342, updated. b0ad16d99eb0bc2148976a31c4dbf7ff0ed5516a [2342] More changes as a result of review

BIND 10 source code commits bind10-changes at lists.isc.org
Thu Nov 8 13:31:34 UTC 2012


The branch, trac2342 has been updated
       via  b0ad16d99eb0bc2148976a31c4dbf7ff0ed5516a (commit)
       via  e68fed5260c3e7f14b596ff8ea897257c9a634d0 (commit)
       via  9fa65c7819dc3b166da2e639214f3905106c3a6b (commit)
      from  3edde39acb177b96c1aaa74f42562c4b7ab0488f (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit b0ad16d99eb0bc2148976a31c4dbf7ff0ed5516a
Author: Stephen Morris <stephen at isc.org>
Date:   Thu Nov 8 13:14:19 2012 +0000

    [2342] More changes as a result of review
    
    a) Corrected some method headers
    b) Added additional diagnostic output should the test fail to
       open the database.

commit e68fed5260c3e7f14b596ff8ea897257c9a634d0
Author: Stephen Morris <stephen at isc.org>
Date:   Thu Nov 8 12:50:34 2012 +0000

    [2342] Encapsulate mysql_stmt_free_result in a temporary object
    
    ... to ensure that it is always called when a method terminates.

commit 9fa65c7819dc3b166da2e639214f3905106c3a6b
Author: Stephen Morris <stephen at isc.org>
Date:   Thu Nov 8 12:14:10 2012 +0000

    [2342] Documentation changes as result of review

-----------------------------------------------------------------------

Summary of changes:
 src/lib/dhcp/database_backends.dox             |   48 +++++----
 src/lib/dhcp/iface_mgr_linux.cc                |    2 +-
 src/lib/dhcp/lease_mgr.h                       |   15 +--
 src/lib/dhcp/lease_mgr_factory.h               |   12 +--
 src/lib/dhcp/libdhcp++.h                       |   10 +-
 src/lib/dhcp/mysql_lease_mgr.cc                |  127 ++++++++++++++++--------
 src/lib/dhcp/mysql_lease_mgr.h                 |   74 +++++++-------
 src/lib/dhcp/option.h                          |   18 ++--
 src/lib/dhcp/tests/mysql_lease_mgr_unittest.cc |   17 +++-
 9 files changed, 196 insertions(+), 127 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/dhcp/database_backends.dox b/src/lib/dhcp/database_backends.dox
index 7736ccb..8eeb5c5 100644
--- a/src/lib/dhcp/database_backends.dox
+++ b/src/lib/dhcp/database_backends.dox
@@ -10,7 +10,7 @@
 
   @section dhcpdb-instantiation Instantiation of Lease Managers
 
-  A lease manager is instantiated through a the LeaseMgrFactory class.  This
+  A lease manager is instantiated through the LeaseMgrFactory class.  This
   has three methods:
 
   - isc::dhcp::LeaseMgrFactory::create - Creates a singleton Lease
@@ -20,9 +20,9 @@
   - isc::dhcp::LeaseMgrFactory::destroy - Destroys the singleton lease manager.
 
   The selection of the Lease Manager (and thus the backend database) is
-  is controlled by the argument passed to isc::dhcp::LeaseMgrfactory::create.
-  This is a string containing a set of "keyword=value" pairs (no embedded
-  spaces), each pair separated by a space from the others, e.g.
+  controlled by the connection string passed to
+  isc::dhcp::LeaseMgrFactory::create.  This is a set of "keyword=value" pairs
+  (no embedded spaces), each pair separated by a space from the others, e.g.
 
   \code
   type=mysql user=keatest password=keatest name=keatest host=localhost
@@ -30,29 +30,35 @@
 
   The following keywords are used for all backends:
 
-  - type - specifies the type of database backend.  The following types are
-    supported:
-       - mysql - Use MySQL as the database
+  - <b>type</b> - specifies the type of database backend.  The following values
+  for the type keyword are supported:
+     - <b>mysql</b> - Use MySQL as the database
 
-  The following keywords apply to the MySQL backend:
+  The following sections list the database-specific keywords:
 
-  - host - host on which the selected database is running.  If not supplied,
-    "localhost" is assumed.
-  - name - name of the MySQL database to access.  There is no default - this
-    must be supplied.
-  - password - password for the selected user ID (see below).  If not specified,
-    no password is used.
-  - user - database user ID under which the database is accessed.  If not
+  @subsection dhcpdb-keywords-mysql MySQL connection string keywords
+
+  - <b>host</b> - host on which the selected database is running.  If not
+  supplied, "localhost" is assumed.
+  - <b>name</b> - name of the MySQL database to access.  There is no default -
+  this must always be supplied.
+  - <b>password</b> - password for the selected user ID (see below).  If not
+  specified, no password is used.
+  - <b>user</b> - database user ID under which the database is accessed.  If not
     specified, no user ID is used - the database is assumed to be open.
 
+
   @section dhcp-backend-unittest Running Unit Tests
 
   With the use of databases requiring separate authorisation, there are
-  certain pre-requisties for scuccessfully running the unit tests.  These are
-  database-specific:
+  certain database-specific pre-requisites for successfully running the unit
+  tests.  These are listed in the following sections.
+
+  @subsection dhcp-mysql-unittest MySQL
 
-  MySQL: a database called keatest needs to be set up.  A database user, also
-  called keatest (with a password keatest) must be given full privileges in
-  that database.  The unit tests create the database schema in the database
-  before each test, and delete it afterwards.
+  A database called <i>keatest</i> needs to be set up using the MySQL
+  <b>CREATE DATABASE</b> command.  A database user, also called <i>keatest</i>
+  (with a password <i>keatest</i>) must be given full privileges in that
+  database.  The unit tests create the schema in the database before each test
+  and delete it afterwards.
   */
diff --git a/src/lib/dhcp/iface_mgr_linux.cc b/src/lib/dhcp/iface_mgr_linux.cc
index 189fe90..d7ebe1a 100644
--- a/src/lib/dhcp/iface_mgr_linux.cc
+++ b/src/lib/dhcp/iface_mgr_linux.cc
@@ -125,7 +125,7 @@ const static size_t RCVBUF_SIZE = 32768;
 
 /// @brief Opens netlink socket and initializes handle structure.
 ///
-/// @exception Unexpected Thrown if socket configuration fails.
+/// @throw isc::Unexpected Thrown if socket configuration fails.
 void Netlink::rtnl_open_socket() {
 
     fd_ = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
diff --git a/src/lib/dhcp/lease_mgr.h b/src/lib/dhcp/lease_mgr.h
index 34e9edc..65c56cc 100644
--- a/src/lib/dhcp/lease_mgr.h
+++ b/src/lib/dhcp/lease_mgr.h
@@ -324,6 +324,10 @@ typedef std::vector<Lease6Ptr> Lease6Collection;
 /// interface to all backends. As this is an abstract class, it should not
 /// be used directly, but rather specialized derived class should be used
 /// instead.
+///
+/// As all methods are virtual, this class throws no exceptions.  However,
+/// methods in concrete implementations of this class may throw exceptions:
+/// see the documentation of those classes for details.
 class LeaseMgr {
 public:
 
@@ -348,8 +352,6 @@ public:
     ///
     /// @result true if the lease was added, false if not (because a lease
     ///         with the same address was already there).
-    ///
-    /// @exception DbOperationError Database function failed
     virtual bool addLease(const Lease4Ptr& lease) = 0;
 
     /// @brief Adds an IPv6 lease.
@@ -358,8 +360,6 @@ public:
     ///
     /// @result true if the lease was added, false if not (because a lease
     ///         with the same address was already there).
-    ///
-    /// @exception DbOperationError Database function failed
     virtual bool addLease(const Lease6Ptr& lease) = 0;
 
     /// @brief Returns IPv4 lease for specified IPv4 address and subnet_id
@@ -484,9 +484,6 @@ public:
     /// @brief Updates IPv4 lease.
     ///
     /// @param lease4 The lease to be updated.
-    ///
-    /// @exception NoSuchLease Attempt to update lease that did not exist.
-    /// @exception DbOperationError Update operation updated multiple leases.
     virtual void updateLease6(const Lease6Ptr& lease6) = 0;
 
     /// @brief Deletes a lease.
@@ -533,16 +530,12 @@ public:
     ///
     /// Commits all pending database operations.  On databases that don't
     /// support transactions, this is a no-op.
-    ///
-    /// @exception DbOperationError if the commit failed.
     virtual void commit() = 0;
 
     /// @brief Rollback Transactions
     ///
     /// Rolls back all pending database operations.  On databases that don't
     /// support transactions, this is a no-op.
-    ///
-    /// @exception DbOperationError if the rollback failed.
     virtual void rollback() = 0;
 
     /// @todo: Add host management here
diff --git a/src/lib/dhcp/lease_mgr_factory.h b/src/lib/dhcp/lease_mgr_factory.h
index e9d73ba..39265c8 100644
--- a/src/lib/dhcp/lease_mgr_factory.h
+++ b/src/lib/dhcp/lease_mgr_factory.h
@@ -75,10 +75,10 @@ public:
     ///        are back-end specific, although must include the "type" keyword
     ///        which gives the backend in use.
     ///
-    /// @exception InvalidParameter dbconfig string does not contain the
-    ///            "type" keyword.
-    /// @exception InvalidType The "type" keyword in dbconfig does not identify
-    ///            a supported backend.
+    /// @throw isc::InvalidParameter dbconfig string does not contain the "type"
+    ///        keyword.
+    /// @throw isc::dhcp::InvalidType The "type" keyword in dbconfig does not
+    ///        identify a supported backend.
     static void create(const std::string& dbconfig);
 
     /// @brief Destroy lease manager
@@ -93,8 +93,8 @@ public:
     /// Returns an instance of the "current" lease manager.  An exception
     /// will be thrown if none is available.
     ///
-    /// @exception NoLeaseManager No lease manager is available: use create()
-    ///            to create one before calling this method.
+    /// @throw isc::dhcp::NoLeaseManager No lease manager is available: use
+    ///        create() to create one before calling this method.
     static LeaseMgr& instance();
 
     /// @brief Parse Database Parameters
diff --git a/src/lib/dhcp/libdhcp++.h b/src/lib/dhcp/libdhcp++.h
index ae90701..53405b4 100644
--- a/src/lib/dhcp/libdhcp++.h
+++ b/src/lib/dhcp/libdhcp++.h
@@ -38,9 +38,11 @@ public:
     /// @param u universe of the option (V4 or V6)
     /// @param type option-type
     /// @param buf option-buffer
-    /// @throw isc::InvalidOperation if there is no factory function
-    /// registered for specified option type.
+    ///
     /// @return instance of option.
+    ///
+    /// @throw isc::InvalidOperation if there is no factory function registered
+    ///        for the specified option type.
     static isc::dhcp::OptionPtr optionFactory(isc::dhcp::Option::Universe u,
                                               uint16_t type,
                                               const OptionBuffer& buf);
@@ -93,8 +95,8 @@ public:
 
     /// Registers factory method that produces options of specific option types.
     ///
-    /// @exception BadValue if provided type is already registered, has too large
-    ///            value or invalid universe is specified
+    /// @throw isc::BadValue if provided the type is already registered, has
+    ///        too large a value or an invalid universe is specified.
     ///
     /// @param u universe of the option (V4 or V6)
     /// @param type option-type
diff --git a/src/lib/dhcp/mysql_lease_mgr.cc b/src/lib/dhcp/mysql_lease_mgr.cc
index 6e3e308..0451fae 100644
--- a/src/lib/dhcp/mysql_lease_mgr.cc
+++ b/src/lib/dhcp/mysql_lease_mgr.cc
@@ -12,16 +12,18 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <iostream>
-#include <iomanip>
-#include <string>
 #include <config.h>
-#include <time.h>
-#include <mysql/mysqld_error.h>
 
 #include <dhcp/mysql_lease_mgr.h>
 #include <asiolink/io_address.h>
 
+#include <mysql/mysqld_error.h>
+
+#include <iostream>
+#include <iomanip>
+#include <string>
+#include <time.h>
+
 using namespace isc;
 using namespace isc::dhcp;
 using namespace std;
@@ -108,17 +110,18 @@ namespace dhcp {
 /// Owing to the MySQL API, the process requires some intermediate variables
 /// to hold things like length etc.  This object holds the intermediate
 /// variables as well.
-
-// Note - there are no unit tests for this class.  It is tested indirectly
-// in all MySqlLeaseMgr::xxx6() calls where it is used.
+///
+/// @note There are no unit tests for this class.  It is tested indirectly
+/// in all MySqlLeaseMgr::xxx6() calls where it is used.
 
 class MySqlLease6Exchange {
 public:
     /// @brief Constructor
     ///
-    /// Apart from the initialization of false_ and true_, the other
-    /// initializations are to satisfy cppcheck: none are really needed, as all
-    /// variables are initialized/set in the methods.
+    /// Apart from the initialization of false_ and true_, the initialization
+    /// of addr6_length_, duid_length_, addr6_buffer_ and duid_buffer_ are
+    /// to satisfy cppcheck: none are really needed, as all variables are
+    /// initialized/set in the methods.
     MySqlLease6Exchange() : addr6_length_(0), duid_length_(0),
                             false_(0), true_(1) {
         memset(addr6_buffer_, 0, sizeof(addr6_buffer_));
@@ -144,6 +147,12 @@ public:
         addr6_ = lease_->addr_.toText();
         addr6_length_ = addr6_.size();
 
+        // In the following statement, the string is being read.  However, the
+        // MySQL C interface does not use "const", so the "buffer" element
+        // is declared as "char*" instead of "const char*".  To resolve this,
+        // the "const" is discarded.  Note that the address of addr6_.c_str()
+        // is guaranteed to be valid until the next non-const operation on
+        // addr6_.
         bind_[0].buffer_type = MYSQL_TYPE_STRING;
         bind_[0].buffer = const_cast<char*>(addr6_.c_str());
         bind_[0].buffer_length = addr6_length_;
@@ -302,7 +311,7 @@ public:
     /// @return Lease6Ptr Pointer to a Lease6 object holding the relevant
     ///         data.
     ///
-    /// @exception BadValue Unable to convert Lease Type value in database
+    /// @throw isc::BadValue Unable to convert Lease Type value in database
     Lease6Ptr getLeaseData() {
 
         // Create the object to be returned.
@@ -375,6 +384,47 @@ private:
 };
 
 
+/// @brief Fetch and Release MySQL Results
+///
+/// When a MySQL statement is exected, to fetch the results the function
+/// mysql_stmt_fetch() must be called.  As well as getting data, this
+/// allocated internal state.  Subsequent calls to mysql_stmt_fetch
+/// can be made, but when all the data is retrieved, mysql_stmt_free_result
+/// must be called to free up the resources allocated.
+///
+/// Created prior to the first fetch, this class's destructor calls
+/// mysql_stmt_free_result, so eliminating the need for an explicit release
+/// in the method using mysql_stmt_free_result.  In this way, it guarantees
+/// that the resources are released even if the MySqlLeaseMgr method concerned
+/// exits via an exception.
+class MySqlFreeResult {
+public:
+
+    /// @brief Constructor
+    ///
+    /// Store the pointer to the statement for which data is being fetched.
+    ///
+    /// Note that according to the MySQL documentation, mysql_stmt_free_result
+    /// only releases resources if a cursor has been allocated for the
+    /// statement.  This implies that it is a no-op if none have been.  Either
+    /// way, any error from mysql_stmt_free_result is ignored. (Generating
+    /// an exception is not much help, as it will only confuse things if the
+    /// method calling mysql_stmt_fetch is exiting via an exception.)
+    MySqlFreeResult(MYSQL_STMT* statement) : statement_(statement)
+    {}
+
+    /// @brief Destructor
+    ///
+    /// Frees up fetch context if a fetch has been successfully executed.
+    ~MySqlFreeResult() {
+        (void) mysql_stmt_free_result(statement_);
+    }
+
+private:
+    MYSQL_STMT*     statement_;     ///< Statement for which results are freed
+};
+
+
 // MySqlLeaseMgr Methods
 
 MySqlLeaseMgr::MySqlLeaseMgr(const LeaseMgr::ParameterMap& parameters) 
@@ -714,6 +764,8 @@ MySqlLeaseMgr::getLease6(const isc::asiolink::IOAddress& addr) const {
     std::string addr6 = addr.toText();
     unsigned long addr6_length = addr6.size();
 
+    // See the earlier description of the use of "const_cast" when accessing
+    // the address for an explanation of the reason.
     inbind[0].buffer_type = MYSQL_TYPE_STRING;
     inbind[0].buffer = const_cast<char*>(addr6.c_str());
     inbind[0].buffer_length = addr6_length;
@@ -723,7 +775,9 @@ MySqlLeaseMgr::getLease6(const isc::asiolink::IOAddress& addr) const {
     // to fields in the exchange object, then execute the prepared statement.
     bind6AndExecute(stindex, inbind);
 
-    // Fetch the data.
+    // Fetch the data and set up the "release" object to release associated
+    // resources when this method exits.
+    MySqlFreeResult fetch_release(statements_[stindex]);
     int status = mysql_stmt_fetch(statements_[stindex]);
 
     Lease6Ptr result;
@@ -731,9 +785,6 @@ MySqlLeaseMgr::getLease6(const isc::asiolink::IOAddress& addr) const {
         try {
             result = exchange6_->getLeaseData();
         } catch (const isc::BadValue& ex) {
-            // Free up result set.
-
-            (void) mysql_stmt_free_result(statements_[stindex]);
             // Lease type is returned, to rethrow the exception with a bit
             // more data.
             isc_throw(BadValue, ex.what() << ". Statement is <" <<
@@ -754,8 +805,6 @@ MySqlLeaseMgr::getLease6(const isc::asiolink::IOAddress& addr) const {
         // This has already been set, so no action is needed.
     }
 
-    // Free data structures associated with information returned.
-    (void) mysql_stmt_free_result(statements_[stindex]);
     return (result);
 }
 
@@ -768,10 +817,10 @@ MySqlLeaseMgr::getLease6(const DUID& duid, uint32_t iaid) const {
     MYSQL_BIND inbind[2];
     memset(inbind, 0, sizeof(inbind));
 
-    // DUID.  The complex casting is needed to obtain the "const" vector of
-    // uint8_t from the DUID, point to the start of it (discarding the
-    // "const"ness) and finally casting it to "char*" for the MySQL buffer
-    // element.
+    // In the following statement, the DUID is being read.  However, the
+    // MySQL C interface does not use "const", so the "buffer" element
+    // is declared as "char*" instead of "const char*".  To resolve this,
+    // the "const" is discarded before the uint8_t* is cast to char*.
     const vector<uint8_t>& duid_vector = duid.getDuid();
     unsigned long duid_length = duid_vector.size();
     inbind[0].buffer_type = MYSQL_TYPE_BLOB;
@@ -797,15 +846,17 @@ MySqlLeaseMgr::getLease6(const DUID& duid, uint32_t iaid) const {
     // Fetch the data.  There could be multiple rows, so we need to iterate
     // until all data has been retrieved.
     Lease6Collection result;
+
+    // Set up the fetch "release" object to release resources associated
+    // with the call to mysql_stmt_fetch when this method exits, then
+    // retrieve the data.
+    MySqlFreeResult fetch_release(statements_[stindex]);
     while ((status = mysql_stmt_fetch(statements_[stindex])) == 0) {
         try {
             Lease6Ptr lease = exchange6_->getLeaseData();
             result.push_back(lease);
 
         } catch (const isc::BadValue& ex) {
-            // Free up result set.
-            (void) mysql_stmt_free_result(statements_[stindex]);
-
             // Rethrow the exception with a bit more data.
             isc_throw(BadValue, ex.what() << ". Statement is <" <<
                       text_statements_[stindex] << ">");
@@ -821,8 +872,6 @@ MySqlLeaseMgr::getLease6(const DUID& duid, uint32_t iaid) const {
         ;
     }
 
-    // Free up resources assoicated with the fetched data.
-    (void) mysql_stmt_free_result(statements_[stindex]);
     return (result);
 }
 
@@ -836,10 +885,8 @@ MySqlLeaseMgr::getLease6(const DUID& duid, uint32_t iaid,
     MYSQL_BIND inbind[3];
     memset(inbind, 0, sizeof(inbind));
 
-    // DUID.  The complex casting is needed to obtain the "const" vector of
-    // uint8_t from the DUID, point to the start of it (discarding the
-    // "const"ness) and finally casing it to "char*" for the MySQL buffer
-    // element.
+    // See the earlier description of the use of "const_cast" when accessing
+    // the DUID for an explanation of the reason.
     const vector<uint8_t>& duid_vector = duid.getDuid();
     unsigned long duid_length = duid_vector.size();
     inbind[0].buffer_type = MYSQL_TYPE_BLOB;
@@ -862,7 +909,10 @@ MySqlLeaseMgr::getLease6(const DUID& duid, uint32_t iaid,
     // to fields in the exchange object, then execute the prepared statement.
     bind6AndExecute(stindex, inbind);
 
+    // Fetch the data and set up the "release" object to release associated
+    // resources when this method exits then retrieve the data.
     Lease6Ptr result;
+    MySqlFreeResult fetch_release(statements_[stindex]);
     int status = mysql_stmt_fetch(statements_[stindex]);
     if (status == 0) {
         try {
@@ -872,9 +922,6 @@ MySqlLeaseMgr::getLease6(const DUID& duid, uint32_t iaid,
             // ignore the excess and take the first.
 
         } catch (const isc::BadValue& ex) {
-            // Free up result set.
-
-            (void) mysql_stmt_free_result(statements_[stindex]);
             // Lease type is returned, to rethrow the exception with a bit
             // more data.
             isc_throw(BadValue, ex.what() << ". Statement is <" <<
@@ -895,8 +942,6 @@ MySqlLeaseMgr::getLease6(const DUID& duid, uint32_t iaid,
         // This has already been set, so the action is a no-op.
     }
 
-    // Free data structures associated with information returned.
-    (void) mysql_stmt_free_result(statements_[stindex]);
     return (result);
 }
 
@@ -922,6 +967,8 @@ MySqlLeaseMgr::updateLease6(const Lease6Ptr& lease) {
     std::string addr6 = lease->addr_.toText();
     unsigned long addr6_length = addr6.size();
 
+    // See the earlier description of the use of "const_cast" when accessing
+    // the address for an explanation of the reason.
     where.buffer_type = MYSQL_TYPE_STRING;
     where.buffer = const_cast<char*>(addr6.c_str());
     where.buffer_length = addr6_length;
@@ -970,6 +1017,8 @@ MySqlLeaseMgr::deleteLease6(const isc::asiolink::IOAddress& addr) {
     std::string addr6 = addr.toText();
     unsigned long addr6_length = addr6.size();
 
+    // See the earlier description of the use of "const_cast" when accessing
+    // the address for an explanation of the reason.
     inbind[0].buffer_type = MYSQL_TYPE_STRING;
     inbind[0].buffer = const_cast<char*>(addr6.c_str());
     inbind[0].buffer_length = addr6_length;
@@ -1042,15 +1091,15 @@ MySqlLeaseMgr::getVersion() const {
                   mysql_error(mysql_));
     }
 
-    // Get the result
+    // Fetch the data and set up the "release" object to release associated
+    // resources when this method exits then retrieve the data.
+    MySqlFreeResult fetch_release(statements_[stindex]);
     status = mysql_stmt_fetch(statements_[stindex]);
     if (status != 0) {
-        (void) mysql_stmt_free_result(statements_[stindex]);
         isc_throw(DbOperationError, "unable to obtain result set: " <<
                   mysql_error(mysql_));
     }
 
-    (void) mysql_stmt_free_result(statements_[stindex]);
     return (std::make_pair(major, minor));
 }
 
diff --git a/src/lib/dhcp/mysql_lease_mgr.h b/src/lib/dhcp/mysql_lease_mgr.h
index 6b5773b..5ebeadd 100644
--- a/src/lib/dhcp/mysql_lease_mgr.h
+++ b/src/lib/dhcp/mysql_lease_mgr.h
@@ -57,10 +57,10 @@ public:
     /// @param parameters A data structure relating keywords and values
     ///        concerned with the database.
     ///
-    /// @exception NoDatabaseName Mandatory database name not given
-    /// @exception DbOpenError Error opening the database
-    /// @exception DbOperationError An operation on the open database has
-    ///            failed.
+    /// @throw isc::dhcp::NoDatabaseName Mandatory database name not given
+    /// @throw isc::dhcp::DbOpenError Error opening the database
+    /// @throw isc::dhcp::DbOperationError An operation on the open database has
+    ///        failed.
     MySqlLeaseMgr(const ParameterMap& parameters);
 
     /// @brief Destructor (closes database)
@@ -73,7 +73,8 @@ public:
     /// @result true if the lease was added, false if not (because a lease
     ///         with the same address was already there).
     ///
-    /// @exception DbOperationError Database function failed
+    /// @throw isc::dhcp::DbOperationError An operation on the open database has
+    ///        failed.
     virtual bool addLease(const Lease4Ptr& lease);
 
     /// @brief Adds an IPv6 lease.
@@ -83,7 +84,8 @@ public:
     /// @result true if the lease was added, false if not (because a lease
     ///         with the same address was already there).
     ///
-    /// @exception DbOperationError Database function failed
+    /// @throw isc::dhcp::DbOperationError An operation on the open database has
+    ///        failed.
     virtual bool addLease(const Lease6Ptr& lease);
 
     /// @brief Return IPv4 lease for specified IPv4 address and subnet_id
@@ -174,10 +176,10 @@ public:
     ///
     /// @return smart pointer to the lease (or NULL if a lease is not found)
     ///
-    /// @exception BadValue record retrieved from database had an invalid
-    ///            lease type field.
-    /// @exception DbOperationError MySQL operation failed, exception will give
-    ///            text indicating the reason.
+    /// @throw isc::BadValue record retrieved from database had an invalid
+    ///        lease type field.
+    /// @throw isc::dhcp::DbOperationError An operation on the open database has
+    ///        failed.
     virtual Lease6Ptr getLease6(const isc::asiolink::IOAddress& addr) const;
 
     /// @brief Returns existing IPv6 leases for a given DUID+IA combination
@@ -211,29 +213,31 @@ public:
     /// If no such lease is present, an exception will be thrown.
     virtual void updateLease4(const Lease4Ptr& lease4);
 
-    /// @brief Updates IPv4 lease.
+    /// @brief Updates IPv6 lease.
     ///
-    /// @param lease4 The lease to be updated.
+    /// @param lease6 The lease to be updated.
     ///
-    /// @exception NoSuchLease Attempt to update lease that did not exist.
-    /// @exception DbOperationError Update operation updated multiple leases.
+    /// @throw isc::dhcp::NoSuchLease Attempt to update a lease that did not
+    ///        exist.
+    /// @throw isc::dhcp::DbOperationError An operation on the open database has
+    ///        failed.
     virtual void updateLease6(const Lease6Ptr& lease6);
 
-    /// @brief Deletes a lease.
+    /// @brief Deletes an IPv4 lease.
     ///
     /// @param addr IPv4 address of the lease to be deleted.
     ///
     /// @return true if deletion was successful, false if no such lease exists
     virtual bool deleteLease4(const isc::asiolink::IOAddress& addr);
 
-    /// @brief Deletes a lease.
+    /// @brief Deletes an IPv6 lease.
     ///
-    /// @param addr IPv4 address of the lease to be deleted.
+    /// @param addr IPv6 address of the lease to be deleted.
     ///
     /// @return true if deletion was successful, false if no such lease exists
     ///
-    /// @exception DbOperationError MySQL operation failed, exception will give
-    ///            text indicating the reason.
+    /// @throw isc::dhcp::DbOperationError An operation on the open database has
+    ///        failed.
     virtual bool deleteLease6(const isc::asiolink::IOAddress& addr);
 
     /// @brief Returns backend name.
@@ -261,8 +265,8 @@ public:
     /// compatible)
     /// Also if B>C, some database upgrade procedure may be triggered
     ///
-    /// @exception DbOperationError MySQL operation failed, exception will give
-    ///            text indicating the reason.
+    /// @throw isc::dhcp::DbOperationError An operation on the open database has
+    ///        failed.
     virtual std::pair<uint32_t, uint32_t> getVersion() const;
 
     /// @brief Commit Transactions
@@ -270,7 +274,7 @@ public:
     /// Commits all pending database operations.  On databases that don't
     /// support transactions, this is a no-op.
     ///
-    /// @exception DbOperationError if the commit failed.
+    /// @throw DbOperationError Iif the commit failed.
     virtual void commit();
 
     /// @brief Rollback Transactions
@@ -278,7 +282,7 @@ public:
     /// Rolls back all pending database operations.  On databases that don't
     /// support transactions, this is a no-op.
     ///
-    /// @exception DbOperationError if the rollback failed.
+    /// @throw DbOperationError If the rollback failed.
     virtual void rollback();
 
     ///@{
@@ -356,9 +360,9 @@ private:
     ///        to be valid, else an exception will be thrown.
     /// @param text Text of the SQL statement to be prepared.
     ///
-    /// @exception DbOperationError MySQL operation failed, exception will give
-    ///            text indicating the reason.
-    /// @exception InvalidParameter 'index' is not valid for the vector.
+    /// @throw isc::dhcp::DbOperationError An operation on the open database has
+    ///        failed.
+    /// @throw isc::InvalidParameter 'index' is not valid for the vector.
     void prepareStatement(StatementIndex index, const char* text);
 
     /// @brief Prepare statements
@@ -366,10 +370,10 @@ private:
     /// Creates the prepared statements for all of the SQL statements used
     /// by the MySQL backend.
     ///
-    /// @exception DbOperationError MySQL operation failed, exception will give
-    ///            text indicating the reason.
-    /// @exception InvalidParameter 'index' is not valid for the vector.  This
-    ///            represents an internal error within the code.
+    /// @throw isc::dhcp::DbOperationError An operation on the open database has
+    ///        failed.
+    /// @throw isc::InvalidParameter 'index' is not valid for the vector.  This
+    ///        represents an internal error within the code.
     void prepareStatements();
 
     /// @brief Open Database
@@ -377,8 +381,8 @@ private:
     /// Opens the database using the information supplied in the parameters
     /// passed to the constructor.
     ///
-    /// @exception NoDatabaseName Mandatory database name not given
-    /// @exception DbOpenError Error opening the database
+    /// @throw NoDatabaseName Mandatory database name not given
+    /// @throw DbOpenError Error opening the database
     void openDatabase();
 
     /// @brief Binds Parameters and Executes
@@ -397,7 +401,8 @@ private:
     ///        (Note that the number is determined by the number of parameters
     ///        in the statement.)
     ///
-    /// @exception DbOperationError Error doing a database operation.
+    /// @throw isc::dhcp::DbOperationError An operation on the open database has
+    ///        failed.
     void bind6AndExecute(StatementIndex stindex, MYSQL_BIND* inbind) const;
 
     /// @brief Check Error and Throw Exception
@@ -410,7 +415,8 @@ private:
     /// @param index Index of statement that caused the error
     /// @param what High-level description of the error
     ///
-    /// @exception DbOperationError Error doing a database operation
+    /// @throw isc::dhcp::DbOperationError An operation on the open database has
+    ///        failed.
     inline void checkError(int status, StatementIndex index,
                            const char* what) const {
         if (status != 0) {
diff --git a/src/lib/dhcp/option.h b/src/lib/dhcp/option.h
index 080a869..4617c26 100644
--- a/src/lib/dhcp/option.h
+++ b/src/lib/dhcp/option.h
@@ -78,9 +78,11 @@ public:
     /// @param u universe of the option (V4 or V6)
     /// @param type option-type
     /// @param buf option-buffer
-    /// @throw isc::InvalidOperation if there is no factory function
-    /// registered for specified option type.
+    ///
     /// @return instance of option.
+    ///
+    /// @throw isc::InvalidOperation if there is no factory function
+    ///        registered for specified option type.
     static OptionPtr factory(Option::Universe u,
                              uint16_t type,
                              const OptionBuffer& buf);
@@ -95,9 +97,11 @@ public:
     ///
     /// @param u universe of the option (V4 or V6)
     /// @param type option-type
-    /// @throw isc::InvalidOperation if there is no factory function
-    /// registered for specified option type.
+    ///
     /// @return instance of option.
+    ///
+    /// @throw isc::InvalidOperation if there is no factory function
+    ///        registered for specified option type.
     static OptionPtr factory(Option::Universe u, uint16_t type) {
         return factory(u, type, OptionBuffer());
     }
@@ -240,21 +244,21 @@ public:
 
     /// @brief Returns content of first byte.
     ///
-    /// @exception OutOfRange Thrown if the option has a length of 0.
+    /// @throw isc::OutOfRange Thrown if the option has a length of 0.
     ///
     /// @return value of the first byte
     uint8_t getUint8();
 
     /// @brief Returns content of first word.
     ///
-    /// @exception OutOfRange Thrown if the option has a length less than 2.
+    /// @throw isc::OutOfRange Thrown if the option has a length less than 2.
     ///
     /// @return uint16_t value stored on first two bytes
     uint16_t getUint16();
 
     /// @brief Returns content of first double word.
     ///
-    /// @exception OutOfRange Thrown if the option has a length less than 4.
+    /// @throw isc::OutOfRange Thrown if the option has a length less than 4.
     ///
     /// @return uint32_t value stored on first four bytes
     uint32_t getUint32();
diff --git a/src/lib/dhcp/tests/mysql_lease_mgr_unittest.cc b/src/lib/dhcp/tests/mysql_lease_mgr_unittest.cc
index 6c473bc..a9b09ff 100644
--- a/src/lib/dhcp/tests/mysql_lease_mgr_unittest.cc
+++ b/src/lib/dhcp/tests/mysql_lease_mgr_unittest.cc
@@ -174,11 +174,20 @@ public:
         L4_ADDRESS(ADDRESS_4), L4_IOADDRESS(L4_ADDRESS), 
         L5_ADDRESS(ADDRESS_5), L5_IOADDRESS(L5_ADDRESS), 
         L6_ADDRESS(ADDRESS_6), L6_IOADDRESS(L6_ADDRESS), 
-        L7_ADDRESS(ADDRESS_7), L7_IOADDRESS(L7_ADDRESS)
-        {
+        L7_ADDRESS(ADDRESS_7), L7_IOADDRESS(L7_ADDRESS) {
+
         destroySchema();
         createSchema();
-        LeaseMgrFactory::create(validConnectionString());
+        try {
+            LeaseMgrFactory::create(validConnectionString());
+        } catch (...) {
+            std::cerr << "*** ERROR: unable to open database. The test\n"
+                         "*** environment is broken and must be fixed before\n"
+                         "*** the MySQL tests will run correctly.\n"
+                         "*** The reason for the problem is described in the\n"
+                         "*** accompanying exception output.\n";
+            throw;
+        }
         lmptr_ = &(LeaseMgrFactory::instance());
     }
 
@@ -419,7 +428,7 @@ TEST(MySqlOpenTest, OpenDatabase) {
         LeaseMgrFactory::destroy();
     } catch (const isc::Exception& ex) {
         FAIL() << "*** ERROR: unable to open database, reason:\n"
-               << "    " << ex.what()
+               << "    " << ex.what() << "\n"
                << "*** The test environment is broken and must be fixed\n"
                << "*** before the MySQL tests will run correctly.\n";
     }



More information about the bind10-changes mailing list