[2342] Documentation changes as result of review

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.
   */

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);

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

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

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

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,17 @@ 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.
+///
+/// This class acts as a wrapper around mysql_stmt_fect
+
+
 // MySqlLeaseMgr Methods
 
 MySqlLeaseMgr::MySqlLeaseMgr(const LeaseMgr::ParameterMap& parameters) 
@@ -714,6 +734,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;
@@ -768,10 +790,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;
@@ -836,10 +858,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;
@@ -922,6 +942,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 +992,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;

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
@@ -215,8 +217,10 @@ public:
     ///
     /// @param lease4 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.
@@ -232,8 +236,8 @@ public:
     ///
     /// @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) {

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();