[2364] More changes in response to review

Also change type of exception thrown by packX() if universe is incorrect.

src/lib/dhcp/option.cc

@@ -63,7 +63,7 @@ void
 Option::check() {
     if ( (universe_ != V4) && (universe_ != V6) ) {
         isc_throw(BadValue, "Invalid universe type specified. "
-                  "Only V4 and V6 are allowed.");
+                  << "Only V4 and V6 are allowed.");
     }
 
     if (universe_ == V4) {
@@ -87,11 +87,13 @@ void Option::pack(isc::util::OutputBuffer& buf) {
     switch (universe_) {
     case V6:
         return (pack6(buf));
+
     case V4:
         return (pack4(buf));
+
     default:
-        isc_throw(BadValue, "Failed to pack " << type_ << " option. Do not "
-                  << "use this method for options other than DHCPv6.");
+        isc_throw(BadValue, "Failed to pack " << type_ << " option as the "
+                  << "universe type is unknown.");
     }
 }
 
@@ -115,7 +117,7 @@ Option::pack4(isc::util::OutputBuffer& buf) {
         LibDHCP::packOptions(buf, options_);
 
     } else {
-        isc_throw(OutOfRange, "Invalid universe type " << universe_);
+        isc_throw(BadValue, "Invalid universe type " << universe_);
     }
 
     return;
@@ -131,7 +133,7 @@ void Option::pack6(isc::util::OutputBuffer& buf) {
 
         LibDHCP::packOptions6(buf, options_);
     } else {
-        isc_throw(OutOfRange, "Invalid universe type " << universe_);
+        isc_throw(BadValue, "Invalid universe type " << universe_);
     }
     return;
 }

src/lib/dhcp/option.h

@@ -119,7 +119,7 @@ public:
     /// This constructor takes vector<uint8_t>& which is used in cases
     /// when content of the option will be copied and stored within
     /// option object. V4 Options follow that approach already.
-    /// TODO Migrate V6 options to that approach.
+    /// @todo Migrate V6 options to that approach.
     ///
     /// @param u specifies universe (V4 or V6)
     /// @param type option type (0-255 for V4 and 0-65535 for V6)
@@ -131,7 +131,7 @@ public:
     /// This contructor is similar to the previous one, but it does not take
     /// the whole vector<uint8_t>, but rather subset of it.
     ///
-    /// TODO: This can be templated to use different containers, not just
+    /// @todo This can be templated to use different containers, not just
     /// vector. Prototype should look like this:
     /// template<typename InputIterator> Option(Universe u, uint16_t type,
     /// InputIterator first, InputIterator last);
@@ -160,19 +160,24 @@ public:
     /// byte after stored option (that is useful for writing options one after
     /// another). Used in DHCPv6 options.
     ///
-    /// TODO: Migrate DHCPv6 code to pack(OutputBuffer& buf) version
+    /// @todo Migrate DHCPv6 code to pack(OutputBuffer& buf) version
     ///
     /// @param buf pointer to a buffer
+    ///
+    /// @throw BadValid Universe of the option is neither V4 nor V6.
     virtual void pack(isc::util::OutputBuffer& buf);
 
     /// @brief Writes option in a wire-format to a buffer.
     ///
     /// Method will throw if option storing fails for some reason.
     ///
-    /// TODO Once old (DHCPv6) implementation is rewritten,
+    /// @todo Once old (DHCPv6) implementation is rewritten,
     /// unify pack4() and pack6() and rename them to just pack().
     ///
     /// @param buf output buffer (option will be stored there)
+    ///
+    /// @throw OutOfRange Option type is greater than 255.
+    /// @throw BadValid Universe is not V4.
     virtual void pack4(isc::util::OutputBuffer& buf);
 
     /// @brief Parses received buffer.
@@ -303,6 +308,8 @@ protected:
     /// defined suboptions. Version for building DHCPv4 options.
     ///
     /// @param buf output buffer (built options will be stored here)
+    ///
+    /// @throw BadValid Universe is not V6.
     virtual void pack6(isc::util::OutputBuffer& buf);
 
     /// @brief A private method used for option correctness.
@@ -324,7 +331,7 @@ protected:
     /// collection for storing suboptions
     OptionCollection options_;
 
-    /// TODO: probably 2 different containers have to be used for v4 (unique
+    /// @todo probably 2 different containers have to be used for v4 (unique
     /// options) and v6 (options with the same type can repeat)
 };