[2304] Fixes for the code review.

Includes adding missing tests for signed integer values

src/lib/dhcp/option6_int.h

@@ -47,7 +47,7 @@ public:
     Option6Int(uint16_t type, T value)
         : Option(Option::V6, type), value_(value) {
         if (!OptionDataTypes<T>::valid) {
-            isc_throw(dhcp::InvalidDataType, "non-numeric type");
+            isc_throw(dhcp::InvalidDataType, "non-integer type");
         }
     }
 
@@ -66,7 +66,7 @@ public:
                OptionBufferConstIter end)
         : Option(Option::V6, type) {
         if (!OptionDataTypes<T>::valid) {
-            isc_throw(dhcp::InvalidDataType, "non-numeric type");
+            isc_throw(dhcp::InvalidDataType, "non-integer type");
         }
         unpack(begin, end);
     }
@@ -80,6 +80,11 @@ public:
     void pack(isc::util::OutputBuffer& buf) {
         buf.writeUint16(type_);
         buf.writeUint16(len() - OPTION6_HDR_LEN);
+        // Depending on the data type length we use different utility functions
+        // writeUint16 or writeUint32 which write the data in the network byte
+        // order to the provided buffer. The same functions can be safely used
+        // for either unsiged or signed integers so there is not need to create
+        // special cases for intX_t types.
         switch (OptionDataTypes<T>::len) {
         case 1:
             buf.writeUint8(value_);
@@ -91,7 +96,7 @@ public:
             buf.writeUint32(value_);
             break;
         default:
-            isc_throw(dhcp::InvalidDataType, "non-numeric type");
+            isc_throw(dhcp::InvalidDataType, "non-integer type");
         }
         LibDHCP::packOptions6(buf, options_);
     }
@@ -105,8 +110,13 @@ public:
     /// @param end iterator to end of option data (first byte after option end)
     virtual void unpack(OptionBufferConstIter begin, OptionBufferConstIter end) {
         if (distance(begin, end) < sizeof(T)) {
-            isc_throw(OutOfRange, "Option " << type_ << " truncated");
+            isc_throw(OutOfRange, "Option " << getType() << " truncated");
         }
+        // Depending on the data type length we use different utility functions
+        // readUint16 or readUint32 which read the data laid in the network byte
+        // order from the provided buffer. The same functions can be safely used
+        // for either unsiged or signed integers so there is not need to create
+        // special cases for intX_t types.
         switch (OptionDataTypes<T>::len) {
         case 1:
             value_ = *begin;
@@ -118,7 +128,7 @@ public:
             value_ = isc::util::readUint32(&(*begin));
             break;
         default:
-            isc_throw(dhcp::InvalidDataType, "non-numeric type");
+            isc_throw(dhcp::InvalidDataType, "non-integer type");
         }
         begin += OptionDataTypes<T>::len;
         LibDHCP::unpackOptions6(OptionBuffer(begin, end), options_);
@@ -152,7 +162,7 @@ public:
 
 private:
 
-    T value_;  ///< Value cabveyed by the option.
+    T value_;  ///< Value conveyed by the option.
 };
 
 } // isc::dhcp namespace

src/lib/dhcp/option6_int_array.h

@@ -36,6 +36,12 @@ namespace dhcp {
 /// - int16_t,
 /// - int32_t.
 ///
+/// @warning Since this option may convey variable number of integer
+/// values, sub-options are should not be added in this option as
+/// there is no way to distinguish them from other data. The API will
+/// allow addition of sub-options but they will be ignored during
+/// packing and unpacking option data.
+///
 /// @param T data field type (see above).
 template<typename T>
 class Option6IntArray: public Option {
@@ -51,18 +57,21 @@ public:
         : Option(Option::V6, type),
           values_(0) {
         if (!OptionDataTypes<T>::valid) {
-            isc_throw(dhcp::InvalidDataType, "non-numeric type");
+            isc_throw(dhcp::InvalidDataType, "non-integer type");
         }
     }
 
     /// @brief Constructor.
     ///
     /// @param type option type.
-    /// @param buf buffer with option data.
+    /// @param buf buffer with option data (must not be empty).
+    ///
+    /// @throw isc::OutOfRange if provided buffer is empty or its length
+    /// is not multiple of size of the data type in bytes.
     Option6IntArray(uint16_t type, const OptionBuffer& buf)
         : Option(Option::V6, type) {
         if (!OptionDataTypes<T>::valid) {
-            isc_throw(dhcp::InvalidDataType, "non-numeric type");
+            isc_throw(dhcp::InvalidDataType, "non-integer type");
         }
         unpack(buf.begin(), buf.end());
     }
@@ -77,12 +86,13 @@ public:
     /// @param begin iterator to first byte of option data.
     /// @param end iterator to end of option data (first byte after option end).
     ///
-    /// @todo mention here what it throws.
+    /// @throw isc::OutOfRange if provided buffer is empty or its length
+    /// is not multiple of size of the data type in bytes.
     Option6IntArray(uint16_t type, OptionBufferConstIter begin,
                     OptionBufferConstIter end)
         : Option(Option::V6, type) {
         if (!OptionDataTypes<T>::valid) {
-            isc_throw(dhcp::InvalidDataType, "non-numeric type");
+            isc_throw(dhcp::InvalidDataType, "non-integer type");
         }
         unpack(begin, end);
     }
@@ -97,6 +107,11 @@ public:
         buf.writeUint16(type_);
         buf.writeUint16(len() - OPTION6_HDR_LEN);
         for (int i = 0; i < values_.size(); ++i) {
+            // Depending on the data type length we use different utility functions
+            // writeUint16 or writeUint32 which write the data in the network byte
+            // order to the provided buffer. The same functions can be safely used
+            // for either unsiged or signed integers so there is not need to create
+            // special cases for intX_t types.
             switch (OptionDataTypes<T>::len) {
             case 1:
                 buf.writeUint8(values_[i]);
@@ -108,9 +123,12 @@ public:
                 buf.writeUint32(values_[i]);
                 break;
             default:
-                isc_throw(dhcp::InvalidDataType, "non-numeric type");
+                isc_throw(dhcp::InvalidDataType, "non-integer type");
             }
         }
+        // We don't pack sub-options here because we have array-type option.
+        // We don't allow sub-options in array-type options as there is no
+        // way to distinguish them from the data fields on option reception.
     }
 
     /// @brief Parses received buffer
@@ -121,11 +139,19 @@ public:
     /// @param begin iterator to first byte of option data
     /// @param end iterator to end of option data (first byte after option end)
     virtual void unpack(OptionBufferConstIter begin, OptionBufferConstIter end) {
+        if (distance(begin, end) == 0) {
+            isc_throw(OutOfRange, "option " << getType() << " empty");
+        }
         if (distance(begin, end) % sizeof(T) != 0) {
-            isc_throw(OutOfRange, "Option " << type_ << " truncated");
+            isc_throw(OutOfRange, "option " << getType() << " truncated");
         }
         values_.clear();
         while (begin != end) {
+            // Depending on the data type length we use different utility functions
+            // readUint16 or readUint32 which read the data laid in the network byte
+            // order from the provided buffer. The same functions can be safely used
+            // for either unsiged or signed integers so there is not need to create
+            // special cases for intX_t types.
             switch (OptionDataTypes<T>::len) {
             case 1:
                 values_.push_back(*begin);
@@ -137,10 +163,13 @@ public:
                 values_.push_back(isc::util::readUint32(&(*begin)));
                 break;
             default:
-                isc_throw(dhcp::InvalidDataType, "non-numeric type");
+                isc_throw(dhcp::InvalidDataType, "non-integer type");
             }
             begin += sizeof(T);
         }
+        // We do not unpack sub-options here because we have array-type option.
+        // Such option have variable number of data fields, thus there is no
+        // way to assess where sub-options start.
     }
 
     /// @brief Return collection of option values.

src/lib/dhcp/option_definition.cc

@@ -164,39 +164,28 @@ OptionDefinition::validate() const {
 }
 
 bool
-OptionDefinition::haveIA6Format() const {
+OptionDefinition::haveIAx6Format(OptionDefinition::DataType first_type) const {
     // Expect that IA_NA option format is defined as record.
     // Although it consists of 3 elements of the same (uint32)
     // type it can't be defined as array of uint32 elements because
     // arrays do not impose limitations on number of elements in
     // the array while this limitation is needed for IA_NA - need
     // exactly 3 elements.
-    if (haveType(RECORD_TYPE)) {
-        if (record_fields_.size() == 3) {
-            for (RecordFieldsConstIter it = record_fields_.begin();
-                 it != record_fields_.end(); ++it) {
-                if (*it != UINT32_TYPE) {
-                    return (false);
-                }
-            }
-            return (true);
-        }
-    }
-    return (false);
+   return (haveType(RECORD_TYPE) &&
+           record_fields_.size() == 3 &&
+           record_fields_[0] == first_type &&
+           record_fields_[1] == UINT32_TYPE &&
+           record_fields_[2] == UINT32_TYPE);
+}
+
+bool
+OptionDefinition::haveIA6Format() const {
+    return (haveIAx6Format(UINT32_TYPE));
 }
 
 bool
 OptionDefinition::haveIAAddr6Format() const {
-    if (haveType(RECORD_TYPE)) {
-        if (record_fields_.size() == 3) {
-            if (record_fields_[0] == IPV6_ADDRESS_TYPE &&
-                record_fields_[1] == UINT32_TYPE &&
-                record_fields_[2] == UINT32_TYPE) {
-                return (true);
-            }
-        }
-    }
-    return (false);
+    return (haveIAx6Format(IPV6_ADDRESS_TYPE));
 }
 
 OptionPtr

src/lib/dhcp/option_definition.h

@@ -96,7 +96,7 @@ public:
         STRING_TYPE = 10,
         FQDN_TYPE = 11,
         RECORD_TYPE = 12,
-        UNKNOWN_TYPE = RECORD_TYPE + 1
+        UNKNOWN_TYPE = 13
     };
 
     /// List of fields within the record.
@@ -133,10 +133,13 @@ private:
         DataType getDataType(const std::string& data_type_name);
 
     private:
-        /// @brief Constructor.
+        /// @brief Private constructor.
         ///
         /// Constructor initializes the internal data structures, e.g.
         /// mapping between data type name and the corresponding enum.
+        /// This constructor is private to ensure that exactly one
+        /// instance of this class can be created using \ref instance
+        /// function.
         DataTypeUtil();
 
         /// Map of data types, maps name of the type to enum value.
@@ -334,6 +337,17 @@ public:
 
 private:
 
+    /// @brief Check if specified option format is a record with 3 fields
+    /// where first one is custom, and two others are uint32.
+    ///
+    /// This is a helper function for functions that detect IA_NA and IAAddr
+    /// option formats.
+    ///
+    /// @param first_type type of the first data field.
+    ///
+    /// @return true if actual option format matches expected format.
+    bool haveIAx6Format(const OptionDefinition::DataType first_type) const;
+
     /// @brief Check if specified type matches option definition type.
     ///
     /// @return true if specified type matches option definition type.

src/lib/dhcp/tests/option6_int_array_unittest.cc

@@ -42,6 +42,223 @@ public:
         }
     }
 
+    /// @brief Test parsing buffer into array of int8_t or uint8_t values.
+    ///
+    /// @warning this function does not perform type check. Make
+    /// sure that only int8_t or uint8_t type is used.
+    ///
+    /// @tparam T int8_t or uint8_t.
+    template<typename T>
+    void bufferToIntTest8() {
+        // Create option that conveys array of multiple uint8_t or int8_t values.
+        // In fact there is no need to use this template class for array
+        // of uint8_t values because Option class is sufficient - it
+        // returns the buffer which is actually the array of uint8_t.
+        // However, since we allow using uint8_t types with this template
+        // class we have to test it here.
+        boost::shared_ptr<Option6IntArray<T> > opt;
+        const int opt_len = 10;
+        const uint16_t opt_code = 80;
+
+        // Constructor throws exception if provided buffer is empty.
+        EXPECT_THROW(
+            Option6IntArray<T>(opt_code, buf_.begin(), buf_.begin()),
+            isc::OutOfRange
+        );
+
+        // Provided buffer is not empty so it should not throw exception.
+        ASSERT_NO_THROW(
+            opt = boost::shared_ptr<
+                Option6IntArray<T> >(new Option6IntArray<T>(opt_code, buf_.begin(),
+                                                            buf_.begin() + opt_len))
+        );
+
+        EXPECT_EQ(Option::V6, opt->getUniverse());
+        EXPECT_EQ(opt_code, opt->getType());
+        // Option should return the collection of int8_t or uint8_t values that
+        // we can match with the buffer we used to create the option.
+        std::vector<T> values = opt->getValues();
+        // We need to copy values from the buffer to apply sign if signed
+        // type is used.
+        std::vector<T> reference_values;
+        for (int i = 0; i < opt_len; ++i) {
+            // Values have been read from the buffer in network
+            // byte order. We put them back in the same order here.
+            reference_values.push_back(static_cast<T>(buf_[i]));
+        }
+
+        // Compare the values against the reference buffer.
+        ASSERT_EQ(opt_len, values.size());
+        EXPECT_TRUE(std::equal(reference_values.begin(), reference_values.begin()
+                               + opt_len, values.begin()));
+
+        // test for pack()
+        opt->pack(out_buf_);
+
+        // Data length is 10 bytes.
+        EXPECT_EQ(10, opt->len() - opt->getHeaderLen());
+        EXPECT_EQ(opt_code, opt->getType());
+        // The total length is 10 bytes for data and 4 bytes for header.
+        ASSERT_EQ(14, out_buf_.getLength());
+
+        // Check if pack worked properly:
+        InputBuffer out(out_buf_.getData(), out_buf_.getLength());
+        // if option type is correct
+        EXPECT_EQ(opt_code, out.readUint16());
+        // if option length is correct
+        EXPECT_EQ(10, out.readUint16());
+        // if data is correct
+        std::vector<uint8_t> out_data;
+        ASSERT_NO_THROW(out.readVector(out_data, opt_len));
+        ASSERT_EQ(opt_len, out_data.size());
+        EXPECT_TRUE(std::equal(buf_.begin(), buf_.begin() + opt_len, out_data.begin()));;
+    }
+
+    /// @brief Test parsing buffer into array of int16_t or uint16_t values.
+    ///
+    /// @warning this function does not perform type check. Make
+    /// sure that only int16_t or uint16_t type is used.
+    ///
+    /// @tparam T int16_t or uint16_t.
+    template<typename T>
+    void bufferToIntTest16() {
+        // Create option that conveys array of multiple uint16_t or int16_t values.
+        boost::shared_ptr<Option6IntArray<T> > opt;
+        const int opt_len = 20;
+        const uint16_t opt_code = 81;
+
+        // Constructor throws exception if provided buffer is empty.
+        EXPECT_THROW(
+            Option6IntArray<T>(opt_code, buf_.begin(), buf_.begin()),
+            isc::OutOfRange
+        );
+
+        // Constructor throws exception if provided buffer's length is not
+        // multiple of 2-bytes.
+        EXPECT_THROW(
+            Option6IntArray<T>(opt_code, buf_.begin(), buf_.begin() + 5),
+            isc::OutOfRange
+        );
+
+        // Now the buffer length is correct.
+        ASSERT_NO_THROW(
+            opt = boost::shared_ptr<
+                Option6IntArray<T> >(new Option6IntArray<T>(opt_code, buf_.begin(),
+                                                            buf_.begin() + opt_len))
+        );
+
+        EXPECT_EQ(Option::V6, opt->getUniverse());
+        EXPECT_EQ(opt_code, opt->getType());
+        // Option should return vector of uint16_t values which should be
+        // constructed from the buffer we provided.
+        std::vector<T> values = opt->getValues();
+        ASSERT_EQ(opt_len, values.size() * sizeof(T));
+        // Create reference values from the buffer so as we can
+        // simply compare two vectors.
+        std::vector<T> reference_values;
+        for (int i = 0; i < opt_len; i += 2) {
+            reference_values.push_back((buf_[i] << 8) |
+                                       buf_[i + 1]);
+        }
+        EXPECT_TRUE(std::equal(reference_values.begin(), reference_values.end(),
+                               values.begin()));
+
+        // Test for pack()
+        opt->pack(out_buf_);
+
+        // Data length is 20 bytes.
+        EXPECT_EQ(20, opt->len() - opt->getHeaderLen());
+        EXPECT_EQ(opt_code, opt->getType());
+        // The total length is 20 bytes for data and 4 bytes for header.
+        ASSERT_EQ(24, out_buf_.getLength());
+
+        // Check if pack worked properly:
+        InputBuffer out(out_buf_.getData(), out_buf_.getLength());
+        // if option type is correct
+        EXPECT_EQ(opt_code, out.readUint16());
+        // if option length is correct
+        EXPECT_EQ(20, out.readUint16());
+        // if data is correct
+        std::vector<uint8_t> out_data;
+        ASSERT_NO_THROW(out.readVector(out_data, opt_len));
+        ASSERT_EQ(opt_len, out_data.size());
+        EXPECT_TRUE(std::equal(buf_.begin(), buf_.begin() + opt_len, out_data.begin()));;
+    }
+
+    /// @brief Test parsing buffer into array of int32_t or uint32_t values.
+    ///
+    /// @warning this function does not perform type check. Make
+    /// sure that only int32_t or uint32_t type is used.
+    ///
+    /// @tparam T int32_t or uint32_t.
+    template<typename T>
+    void bufferToIntTest32() {
+        // Create option that conveys array of multiple uint16_t values.
+        boost::shared_ptr<Option6IntArray<T> > opt;
+        const int opt_len = 40;
+        const uint16_t opt_code = 82;
+
+        // Constructor throws exception if provided buffer is empty.
+        EXPECT_THROW(
+            Option6IntArray<T>(opt_code, buf_.begin(), buf_.begin()),
+            isc::OutOfRange
+        );
+
+        // Constructor throws exception if provided buffer's length is not
+        // multiple of 4-bytes.
+        EXPECT_THROW(
+            Option6IntArray<T>(opt_code, buf_.begin(), buf_.begin() + 9),
+            isc::OutOfRange
+        );
+
+        // Now the buffer length is correct.
+        ASSERT_NO_THROW(
+            opt = boost::shared_ptr<
+                Option6IntArray<T> >(new Option6IntArray<T>(opt_code, buf_.begin(),
+                                                            buf_.begin() + opt_len))
+        );
+
+        EXPECT_EQ(Option::V6, opt->getUniverse());
+        EXPECT_EQ(opt_code, opt->getType());
+        // Option should return vector of uint32_t values which should be
+        // constructed from the buffer we provided.
+        std::vector<T> values = opt->getValues();
+        ASSERT_EQ(opt_len, values.size() * sizeof(T));
+        // Create reference values from the buffer so as we can
+        // simply compare two vectors.
+        std::vector<T> reference_values;
+        for (int i = 0; i < opt_len; i += 4) {
+            reference_values.push_back((buf_[i] << 24) |
+                                       (buf_[i + 1] << 16 & 0x00FF0000) |
+                                       (buf_[i + 2] << 8 & 0xFF00) |
+                                       (buf_[i + 3] & 0xFF));
+        }
+        EXPECT_TRUE(std::equal(reference_values.begin(), reference_values.end(),
+                               values.begin()));
+
+        // Test for pack()
+        opt->pack(out_buf_);
+
+        // Data length is 40 bytes.
+        EXPECT_EQ(40, opt->len() - opt->getHeaderLen());
+        EXPECT_EQ(opt_code, opt->getType());
+        // The total length is 40 bytes for data and 4 bytes for header.
+        ASSERT_EQ(44, out_buf_.getLength());
+
+        // Check if pack worked properly:
+        InputBuffer out(out_buf_.getData(), out_buf_.getLength());
+        // if option type is correct
+        EXPECT_EQ(opt_code, out.readUint16());
+        // if option length is correct
+        EXPECT_EQ(40, out.readUint16());
+        // if data is correct
+        std::vector<uint8_t> out_data;
+        ASSERT_NO_THROW(out.readVector(out_data, opt_len));
+        ASSERT_EQ(opt_len, out_data.size());
+        EXPECT_TRUE(std::equal(buf_.begin(), buf_.begin() + opt_len, out_data.begin()));;
+    }
+
+
     OptionBuffer buf_;     ///< Option buffer
     OutputBuffer out_buf_; ///< Output buffer
 };
@@ -68,156 +285,28 @@ TEST_F(Option6IntArrayTest, useInvalidType) {
 }
 
 TEST_F(Option6IntArrayTest, bufferToUint8) {
-    // Create option that conveys array of multiple uint8_t values.
-    // In fact there is no need to use this template class for array
-    // of uint8_t values because Option class is sufficient - it
-    // returns the buffer which is actually the array of uint8_t.
-    // However, since we allow using uint8_t types with this template
-    // class we have to test it here.
-    boost::shared_ptr<Option6IntArray<uint8_t> > opt;
-    const int opt_len = 10;
-    const uint16_t opt_code = 80;
-    // Constructor may throw in case provided buffer is too short.
-    ASSERT_NO_THROW(
-        opt = boost::shared_ptr<
-            Option6IntArray<uint8_t> >(new Option6IntArray<uint8_t>(opt_code, buf_.begin(),
-                                                                    buf_.begin() + opt_len))
-    );
-
-    EXPECT_EQ(Option::V6, opt->getUniverse());
-    EXPECT_EQ(opt_code, opt->getType());
-    // Option should return the same value that we initialized the first
-    // byte of the buffer with.
-    std::vector<uint8_t> values = opt->getValues();
-    ASSERT_EQ(opt_len, values.size());
-    EXPECT_TRUE(std::equal(buf_.begin(), buf_.begin() + opt_len, values.begin()));
-
-    // test for pack()
-    opt->pack(out_buf_);
+    bufferToIntTest8<uint8_t>();
+}
 
-    // Data length is 10 bytes.
-    EXPECT_EQ(10, opt->len() - opt->getHeaderLen());
-    EXPECT_EQ(opt_code, opt->getType());
-    // The total length is 10 bytes for data and 4 bytes for header.
-    ASSERT_EQ(14, out_buf_.getLength());
-
-    // Check if pack worked properly:
-    InputBuffer out(out_buf_.getData(), out_buf_.getLength());
-    // if option type is correct
-    EXPECT_EQ(opt_code, out.readUint16());
-    // if option length is correct
-    EXPECT_EQ(10, out.readUint16());
-    // if data is correct
-    std::vector<uint8_t> out_data;
-    ASSERT_NO_THROW(out.readVector(out_data, opt_len));
-    ASSERT_EQ(opt_len, out_data.size());
-    EXPECT_TRUE(std::equal(buf_.begin(), buf_.begin() + opt_len, out_data.begin()));;
+TEST_F(Option6IntArrayTest, bufferToInt8) {
+    bufferToIntTest8<int8_t>();
 }
 
 TEST_F(Option6IntArrayTest, bufferToUint16) {
-    // Create option that conveys array of multiple uint16_t values.
-    boost::shared_ptr<Option6IntArray<uint16_t> > opt;
-    const int opt_len = 20;
-    const uint16_t opt_code = 81;
-    // Constructor may throw in case provided buffer is too short.
-    ASSERT_NO_THROW(
-        opt = boost::shared_ptr<
-            Option6IntArray<uint16_t> >(new Option6IntArray<uint16_t>(opt_code, buf_.begin(),
-                                                                      buf_.begin() + opt_len))
-    );
-
-    EXPECT_EQ(Option::V6, opt->getUniverse());
-    EXPECT_EQ(opt_code, opt->getType());
-    // Option should return vector of uint16_t values which should be
-    // constructed from the buffer we provided.
-    std::vector<uint16_t> values = opt->getValues();
-    // Let's pack the values back into some uint8_t buffer to compare
-    // it later with the reference data.
-    std::vector<uint8_t> values_buf;
-    for (int i = 0; i < values.size(); ++i) {
-        // Values have been read from the buffer in network
-        // byte order. We put them back in the same order here.
-        values_buf.push_back(values[i] >> 8);
-        values_buf.push_back(values[i] & 0xFF);
-    }
-    ASSERT_EQ(opt_len, values_buf.size());
-    EXPECT_TRUE(std::equal(buf_.begin(), buf_.begin() + opt_len, values_buf.begin()));
-
-    // Test for pack()
-    opt->pack(out_buf_);
+    bufferToIntTest16<uint16_t>();
+}
 
-    // Data length is 20 bytes.
-    EXPECT_EQ(20, opt->len() - opt->getHeaderLen());
-    EXPECT_EQ(opt_code, opt->getType());
-    // The total length is 20 bytes for data and 4 bytes for header.
-    ASSERT_EQ(24, out_buf_.getLength());
-
-    // Check if pack worked properly:
-    InputBuffer out(out_buf_.getData(), out_buf_.getLength());
-    // if option type is correct
-    EXPECT_EQ(opt_code, out.readUint16());
-    // if option length is correct
-    EXPECT_EQ(20, out.readUint16());
-    // if data is correct
-    std::vector<uint8_t> out_data;
-    ASSERT_NO_THROW(out.readVector(out_data, opt_len));
-    ASSERT_EQ(opt_len, out_data.size());
-    EXPECT_TRUE(std::equal(buf_.begin(), buf_.begin() + opt_len, out_data.begin()));;
+TEST_F(Option6IntArrayTest, bufferToInt16) {
+    bufferToIntTest16<int16_t>();
 }
 
 TEST_F(Option6IntArrayTest, bufferToUint32) {
-    // Create option that conveys array of multiple uint16_t values.
-    boost::shared_ptr<Option6IntArray<uint32_t> > opt;
-    const int opt_len = 40;
-    const uint16_t opt_code = 82;
-    // Constructor may throw in case provided buffer is too short.
-    ASSERT_NO_THROW(
-        opt = boost::shared_ptr<
-            Option6IntArray<uint32_t> >(new Option6IntArray<uint32_t>(opt_code, buf_.begin(),
-                                                                      buf_.begin() + opt_len))
-    );
-
-    EXPECT_EQ(Option::V6, opt->getUniverse());
-    EXPECT_EQ(opt_code, opt->getType());
-    // Option should return vector of uint32_t values which should be
-    // constructed from the buffer we provided.
-    std::vector<uint32_t> values = opt->getValues();
-    // Let's pack the values back into some uint8_t buffer to compare
-    // it later with the reference data.
-    std::vector<uint8_t> values_buf;
-    for (int i = 0; i < values.size(); ++i) {
-        // Values have been read from the buffer in network
-        // byte order. We put them back in the same order here.
-        values_buf.push_back(values[i] >> 24);
-        values_buf.push_back(values[i] >> 16 & 0xFF);
-        values_buf.push_back(values[i] >> 8 & 0xFF);
-        values_buf.push_back(values[i] & 0xFF);
-    }
-    ASSERT_EQ(opt_len, values_buf.size());
-    EXPECT_TRUE(std::equal(buf_.begin(), buf_.begin() + opt_len, values_buf.begin()));
-
-    // Test for pack()
-    opt->pack(out_buf_);
-
-    // Data length is 40 bytes.
-    EXPECT_EQ(40, opt->len() - opt->getHeaderLen());
-    EXPECT_EQ(opt_code, opt->getType());
-    // The total length is 40 bytes for data and 4 bytes for header.
-    ASSERT_EQ(44, out_buf_.getLength());
-
-    // Check if pack worked properly:
-    InputBuffer out(out_buf_.getData(), out_buf_.getLength());
-    // if option type is correct
-    EXPECT_EQ(opt_code, out.readUint16());
-    // if option length is correct
-    EXPECT_EQ(40, out.readUint16());
-    // if data is correct
-    std::vector<uint8_t> out_data;
-    ASSERT_NO_THROW(out.readVector(out_data, opt_len));
-    ASSERT_EQ(opt_len, out_data.size());
-    EXPECT_TRUE(std::equal(buf_.begin(), buf_.begin() + opt_len, out_data.begin()));;
+    bufferToIntTest32<uint32_t>();
 }
 
+TEST_F(Option6IntArrayTest, bufferToInt32) {
+    bufferToIntTest32<int32_t>();
+}
 
 TEST_F(Option6IntArrayTest, setValuesUint8) {
     const uint16_t opt_code = 100;
@@ -226,7 +315,7 @@ TEST_F(Option6IntArrayTest, setValuesUint8) {
     // Initialize vector with some data and pass to the option.
     std::vector<uint8_t> values;
     for (int i = 0; i < 10; ++i) {
-        values.push_back(255 - i);
+        values.push_back(numeric_limits<uint8_t>::max() - i);
     }
     opt->setValues(values);
 
@@ -237,6 +326,24 @@ TEST_F(Option6IntArrayTest, setValuesUint8) {
     EXPECT_TRUE(std::equal(values.begin(), values.end(), returned_values.begin()));
 }
 
+TEST_F(Option6IntArrayTest, setValuesInt8) {
+    const uint16_t opt_code = 100;
+    // Create option with empty vector of values.
+    boost::shared_ptr<Option6IntArray<int8_t> > opt(new Option6IntArray<int8_t>(opt_code));
+    // Initialize vector with some data and pass to the option.
+    std::vector<int8_t> values;
+    for (int i = 0; i < 10; ++i) {
+        values.push_back(numeric_limits<int8_t>::min() + i);
+    }
+    opt->setValues(values);
+
+    // Check if universe, option type and data was set correctly.
+    EXPECT_EQ(Option::V6, opt->getUniverse());
+    EXPECT_EQ(opt_code, opt->getType());
+    std::vector<int8_t> returned_values = opt->getValues();
+    EXPECT_TRUE(std::equal(values.begin(), values.end(), returned_values.begin()));
+}
+
 TEST_F(Option6IntArrayTest, setValuesUint16) {
     const uint16_t opt_code = 101;
     // Create option with empty vector of values.
@@ -244,7 +351,7 @@ TEST_F(Option6IntArrayTest, setValuesUint16) {
     // Initialize vector with some data and pass to the option.
     std::vector<uint16_t> values;
     for (int i = 0; i < 10; ++i) {
-        values.push_back(0xFFFF - i);
+        values.push_back(numeric_limits<uint16_t>::max() - i);
     }
     opt->setValues(values);
 
@@ -255,6 +362,24 @@ TEST_F(Option6IntArrayTest, setValuesUint16) {
     EXPECT_TRUE(std::equal(values.begin(), values.end(), returned_values.begin()));
 }
 
+TEST_F(Option6IntArrayTest, setValuesInt16) {
+    const uint16_t opt_code = 101;
+    // Create option with empty vector of values.
+    boost::shared_ptr<Option6IntArray<int16_t> > opt(new Option6IntArray<int16_t>(opt_code));
+    // Initialize vector with some data and pass to the option.
+    std::vector<int16_t> values;
+    for (int i = 0; i < 10; ++i) {
+        values.push_back(numeric_limits<int16_t>::min() + i);
+    }
+    opt->setValues(values);
+
+    // Check if universe, option type and data was set correctly.
+    EXPECT_EQ(Option::V6, opt->getUniverse());
+    EXPECT_EQ(opt_code, opt->getType());
+    std::vector<int16_t> returned_values = opt->getValues();
+    EXPECT_TRUE(std::equal(values.begin(), values.end(), returned_values.begin()));
+}
+
 TEST_F(Option6IntArrayTest, setValuesUint32) {
     const uint32_t opt_code = 101;
     // Create option with empty vector of values.
@@ -262,7 +387,7 @@ TEST_F(Option6IntArrayTest, setValuesUint32) {
     // Initialize vector with some data and pass to the option.
     std::vector<uint32_t> values;
     for (int i = 0; i < 10; ++i) {
-        values.push_back(0xFFFFFFFF - i);
+        values.push_back(numeric_limits<uint32_t>::max() - i);
     }
     opt->setValues(values);
 
@@ -273,4 +398,23 @@ TEST_F(Option6IntArrayTest, setValuesUint32) {
     EXPECT_TRUE(std::equal(values.begin(), values.end(), returned_values.begin()));
 }
 
+TEST_F(Option6IntArrayTest, setValuesInt32) {
+    const uint32_t opt_code = 101;
+    // Create option with empty vector of values.
+    boost::shared_ptr<Option6IntArray<int32_t> > opt(new Option6IntArray<int32_t>(opt_code));
+    // Initialize vector with some data and pass to the option.
+    std::vector<int32_t> values;
+    for (int i = 0; i < 10; ++i) {
+        values.push_back(numeric_limits<int32_t>::min() + i);
+    }
+    opt->setValues(values);
+
+    // Check if universe, option type and data was set correctly.
+    EXPECT_EQ(Option::V6, opt->getUniverse());
+    EXPECT_EQ(opt_code, opt->getType());
+    std::vector<int32_t> returned_values = opt->getValues();
+    EXPECT_TRUE(std::equal(values.begin(), values.end(), returned_values.begin()));
+}
+
+
 } // anonymous namespace

src/lib/dhcp/tests/option6_int_unittest.cc

@@ -42,6 +42,142 @@ public:
         }
     }
 
+    /// @brief Basic test for int8 and uint8 types.
+    ///
+    /// @note this function does not perform type check. Make
+    /// sure that only int8_t or uint8_t type is used.
+    ///
+    /// @tparam T int8_t or uint8_t.
+    template<typename T>
+    void basicTest8() {
+        // Create option that conveys single 8 bit integer value.
+        boost::shared_ptr<Option6Int<T> > opt;
+        // Initialize buffer with this value.
+        buf_[0] = 0xa1;
+        // Constructor may throw in case provided buffer is too short.
+        ASSERT_NO_THROW(
+            opt = boost::shared_ptr<Option6Int<T> >(new Option6Int<T>(D6O_PREFERENCE,
+                                                                      buf_.begin(),
+                                                                      buf_.end()))
+        );
+
+        EXPECT_EQ(Option::V6, opt->getUniverse());
+        EXPECT_EQ(D6O_PREFERENCE, opt->getType());
+        // Option should return the same value that we initialized the first
+        // byte of the buffer with.
+        EXPECT_EQ(static_cast<T>(0xa1), opt->getValue());
+
+        // test for pack()
+        opt->pack(out_buf_);
+
+        // Data length is 1 byte.
+        EXPECT_EQ(1, opt->len() - opt->getHeaderLen());
+        EXPECT_EQ(D6O_PREFERENCE, opt->getType());
+        // The total length is 1 byte for data and 4 bytes for header.
+        EXPECT_EQ(5, out_buf_.getLength());
+
+        // Check if pack worked properly:
+        InputBuffer out(out_buf_.getData(), out_buf_.getLength());
+        // if option type is correct
+        EXPECT_EQ(D6O_PREFERENCE, out.readUint16());
+        // if option length is correct
+        EXPECT_EQ(1, out.readUint16());
+        // if data is correct
+        EXPECT_EQ(0xa1, out.readUint8() );
+    }
+
+    /// @brief Basic test for int16 and uint16 types.
+    ///
+    /// @note this function does not perform type check. Make
+    /// sure that only int16_t or uint16_t type is used.
+    ///
+    /// @tparam T int16_t or uint16_t.
+    template<typename T>
+    void basicTest16() {
+        // Create option that conveys single 16-bit integer value.
+        boost::shared_ptr<Option6Int<T> > opt;
+        // Initialize buffer with uint16_t value.
+        buf_[0] = 0xa1;
+        buf_[1] = 0xa2;
+        // Constructor may throw in case provided buffer is too short.
+        ASSERT_NO_THROW(
+            opt = boost::shared_ptr<Option6Int<T> >(new Option6Int<T>(D6O_ELAPSED_TIME,
+                                                                      buf_.begin(),
+                                                                      buf_.end()))
+        );
+
+        EXPECT_EQ(Option::V6, opt->getUniverse());
+        EXPECT_EQ(D6O_ELAPSED_TIME, opt->getType());
+        // Option should return the value equal to the contents of first
+        // and second byte of the buffer.
+        EXPECT_EQ(static_cast<T>(0xa1a2), opt->getValue());
+
+        // Test for pack()
+        opt->pack(out_buf_);
+
+        // Data length is 2 bytes.
+        EXPECT_EQ(2, opt->len() - opt->getHeaderLen());
+        EXPECT_EQ(D6O_ELAPSED_TIME, opt->getType());
+        // The total length is 2 byte for data and 4 bytes for header.
+        EXPECT_EQ(6, out_buf_.getLength());
+
+        // Check if pack worked properly:
+        InputBuffer out(out_buf_.getData(), out_buf_.getLength());
+        // if option type is correct
+        EXPECT_EQ(D6O_ELAPSED_TIME, out.readUint16());
+        // if option length is correct
+        EXPECT_EQ(2, out.readUint16());
+        // if data is correct
+        EXPECT_EQ(0xa1a2, out.readUint16() );
+    }
+
+    /// @brief Basic test for int32 and uint32 types.
+    ///
+    /// @note this function does not perform type check. Make
+    /// sure that only int32_t or uint32_t type is used.
+    ///
+    /// @tparam T int32_t or uint32_t.
+    template<typename T>
+    void basicTest32() {
+        // Create option that conveys single 32-bit integer value.
+        boost::shared_ptr<Option6Int<T> > opt;
+        // Initialize buffer with 32-bit integer value.
+        buf_[0] = 0xa1;
+        buf_[1] = 0xa2;
+        buf_[2] = 0xa3;
+        buf_[3] = 0xa4;
+        // Constructor may throw in case provided buffer is too short.
+        ASSERT_NO_THROW(
+                        opt = boost::shared_ptr<Option6Int<T> >(new Option6Int<T>(D6O_CLT_TIME,
+                                                                                  buf_.begin(),
+                                                                                  buf_.end()))
+                        );
+
+        EXPECT_EQ(Option::V6, opt->getUniverse());
+        EXPECT_EQ(D6O_CLT_TIME, opt->getType());
+        // Option should return the value equal to the value made of
+        // first 4 bytes of the buffer.
+        EXPECT_EQ(static_cast<T>(0xa1a2a3a4), opt->getValue());
+
+        // Test for pack()
+        opt->pack(out_buf_);
+
+        // Data length is 4 bytes.
+        EXPECT_EQ(4, opt->len() - opt->getHeaderLen());
+        EXPECT_EQ(D6O_CLT_TIME, opt->getType());
+        // The total length is 4 bytes for data and 4 bytes for header.
+        EXPECT_EQ(8, out_buf_.getLength());
+
+        // Check if pack worked properly:
+        InputBuffer out(out_buf_.getData(), out_buf_.getLength());
+        // if option type is correct
+        EXPECT_EQ(D6O_CLT_TIME, out.readUint16());
+        // if option length is correct
+        EXPECT_EQ(4, out.readUint16());
+        // if data is correct
+        EXPECT_EQ(0xa1a2a3a4, out.readUint32());
+    }
+
     OptionBuffer buf_;     ///< Option buffer
     OutputBuffer out_buf_; ///< Output buffer
 };
@@ -64,120 +200,28 @@ TEST_F(Option6IntTest, useInvalidType) {
 }
 
 TEST_F(Option6IntTest, basicUint8) {
-    // Create option that conveys single uint8_t value.
-    boost::shared_ptr<Option6Int<uint8_t> > opt;
-    // Initialize buffer with this value.
-    buf_[0] = 0xa1;
-    // Constructor may throw in case provided buffer is too short.
-    ASSERT_NO_THROW(
-        opt = boost::shared_ptr<Option6Int<uint8_t> >(new Option6Int<uint8_t>(D6O_PREFERENCE,
-                                                                              buf_.begin(),
-                                                                              buf_.end()))
-    );
-
-    EXPECT_EQ(Option::V6, opt->getUniverse());
-    EXPECT_EQ(D6O_PREFERENCE, opt->getType());
-    // Option should return the same value that we initialized the first
-    // byte of the buffer with.
-    EXPECT_EQ(buf_[0], opt->getValue());
-
-    // test for pack()
-    opt->pack(out_buf_);
-
-    // Data length is 1 byte.
-    EXPECT_EQ(1, opt->len() - opt->getHeaderLen());
-    EXPECT_EQ(D6O_PREFERENCE, opt->getType());
-    // The total length is 1 byte for data and 4 bytes for header.
-    EXPECT_EQ(5, out_buf_.getLength());
-
-    // Check if pack worked properly:
-    InputBuffer out(out_buf_.getData(), out_buf_.getLength());
-    // if option type is correct
-    EXPECT_EQ(D6O_PREFERENCE, out.readUint16());
-    // if option length is correct
-    EXPECT_EQ(1, out.readUint16());
-    // if data is correct
-    EXPECT_EQ(0xa1, out.readUint8() );
+    basicTest8<uint8_t>();
 }
 
 TEST_F(Option6IntTest, basicUint16) {
-    // Create option that conveys single uint16_t value.
-    boost::shared_ptr<Option6Int<uint16_t> > opt;
-    // Initialize buffer with uint16_t value.
-    buf_[0] = 0xa1;
-    buf_[1] = 0xa2;
-    // Constructor may throw in case provided buffer is too short.
-    ASSERT_NO_THROW(
-        opt = boost::shared_ptr<Option6Int<uint16_t> >(new Option6Int<uint16_t>(D6O_ELAPSED_TIME,
-                                                                                buf_.begin(),
-                                                                                buf_.end()))
-    );
-
-    EXPECT_EQ(Option::V6, opt->getUniverse());
-    EXPECT_EQ(D6O_ELAPSED_TIME, opt->getType());
-    // Option should return the value equal to the contents of first
-    // and second byte of the buffer.
-    EXPECT_EQ(0xa1a2, opt->getValue());
-
-    // Test for pack()
-    opt->pack(out_buf_);
-
-    // Data length is 2 bytes.
-    EXPECT_EQ(2, opt->len() - opt->getHeaderLen());
-    EXPECT_EQ(D6O_ELAPSED_TIME, opt->getType());
-    // The total length is 2 byte for data and 4 bytes for header.
-    EXPECT_EQ(6, out_buf_.getLength());
-
-    // Check if pack worked properly:
-    InputBuffer out(out_buf_.getData(), out_buf_.getLength());
-    // if option type is correct
-    EXPECT_EQ(D6O_ELAPSED_TIME, out.readUint16());
-    // if option length is correct
-    EXPECT_EQ(2, out.readUint16());
-    // if data is correct
-    EXPECT_EQ(0xa1a2, out.readUint16() );
+    basicTest16<uint16_t>();
 }
 
 TEST_F(Option6IntTest, basicUint32) {
-    // Create option that conveys single uint32_t value.
-    boost::shared_ptr<Option6Int<uint32_t> > opt;
-    // Initialize buffer with uint32_t value.
-    buf_[0] = 0xa1;
-    buf_[1] = 0xa2;
-    buf_[2] = 0xa3;
-    buf_[3] = 0xa4;
-    // Constructor may throw in case provided buffer is too short.
-    ASSERT_NO_THROW(
-        opt = boost::shared_ptr<Option6Int<uint32_t> >(new Option6Int<uint32_t>(D6O_CLT_TIME,
-                                                                                buf_.begin(),
-                                                                                buf_.end()))
-    );
-
-    EXPECT_EQ(Option::V6, opt->getUniverse());
-    EXPECT_EQ(D6O_CLT_TIME, opt->getType());
-    // Option should return the value equal to the value made of
-    // first 4 bytes of the buffer.
-    EXPECT_EQ(0xa1a2a3a4, opt->getValue());
+    basicTest32<uint32_t>();
+}
 
-    // Test for pack()
-    opt->pack(out_buf_);
+TEST_F(Option6IntTest, basicInt8) {
+    basicTest8<int8_t>();
+}
 
-    // Data length is 4 bytes.
-    EXPECT_EQ(4, opt->len() - opt->getHeaderLen());
-    EXPECT_EQ(D6O_CLT_TIME, opt->getType());
-    // The total length is 4 bytes for data and 4 bytes for header.
-    EXPECT_EQ(8, out_buf_.getLength());
-
-    // Check if pack worked properly:
-    InputBuffer out(out_buf_.getData(), out_buf_.getLength());
-    // if option type is correct
-    EXPECT_EQ(D6O_CLT_TIME, out.readUint16());
-    // if option length is correct
-    EXPECT_EQ(4, out.readUint16());
-    // if data is correct
-    EXPECT_EQ(0xa1a2a3a4, out.readUint32());
+TEST_F(Option6IntTest, basicInt16) {
+    basicTest16<int16_t>();
 }
 
+TEST_F(Option6IntTest, basicInt32) {
+    basicTest32<int32_t>();
+}
 
 TEST_F(Option6IntTest, setValueUint8) {
     boost::shared_ptr<Option6Int<uint8_t> > opt(new Option6Int<uint8_t>(D6O_PREFERENCE, 123));
@@ -192,6 +236,20 @@ TEST_F(Option6IntTest, setValueUint8) {
     EXPECT_EQ(111, opt->getValue());
 }
 
+TEST_F(Option6IntTest, setValueInt8) {
+    boost::shared_ptr<Option6Int<int8_t> > opt(new Option6Int<int8_t>(D6O_PREFERENCE, -123));
+    // Check if constructor intitialized the option value correctly.
+    EXPECT_EQ(-123, opt->getValue());
+    // Override the value.
+    opt->setValue(-111);
+
+    EXPECT_EQ(Option::V6, opt->getUniverse());
+    EXPECT_EQ(D6O_PREFERENCE, opt->getType());
+    // Check if the value has been overriden.
+    EXPECT_EQ(-111, opt->getValue());
+}
+
+
 TEST_F(Option6IntTest, setValueUint16) {
     boost::shared_ptr<Option6Int<uint16_t> > opt(new Option6Int<uint16_t>(D6O_ELAPSED_TIME, 123));
     // Check if constructor intitialized the option value correctly.
@@ -205,6 +263,19 @@ TEST_F(Option6IntTest, setValueUint16) {
     EXPECT_EQ(0x0102, opt->getValue());
 }
 
+TEST_F(Option6IntTest, setValueInt16) {
+    boost::shared_ptr<Option6Int<int16_t> > opt(new Option6Int<int16_t>(D6O_ELAPSED_TIME, -16500));
+    // Check if constructor intitialized the option value correctly.
+    EXPECT_EQ(-16500, opt->getValue());
+    // Override the value.
+    opt->setValue(-20100);
+
+    EXPECT_EQ(Option::V6, opt->getUniverse());
+    EXPECT_EQ(D6O_ELAPSED_TIME, opt->getType());
+    // Check if the value has been overriden.
+    EXPECT_EQ(-20100, opt->getValue());
+}
+
 TEST_F(Option6IntTest, setValueUint32) {
     boost::shared_ptr<Option6Int<uint32_t> > opt(new Option6Int<uint32_t>(D6O_CLT_TIME, 123));
     // Check if constructor intitialized the option value correctly.
@@ -218,6 +289,19 @@ TEST_F(Option6IntTest, setValueUint32) {
     EXPECT_EQ(0x01020304, opt->getValue());
 }
 
+TEST_F(Option6IntTest, setValueint32) {
+    boost::shared_ptr<Option6Int<int32_t> > opt(new Option6Int<int32_t>(D6O_CLT_TIME, -120100));
+    // Check if constructor intitialized the option value correctly.
+    EXPECT_EQ(-120100, opt->getValue());
+    // Override the value.
+    opt->setValue(-125000);
+
+    EXPECT_EQ(Option::V6, opt->getUniverse());
+    EXPECT_EQ(D6O_CLT_TIME, opt->getType());
+    // Check if the value has been overriden.
+    EXPECT_EQ(-125000, opt->getValue());
+}
+
 TEST_F(Option6IntTest, packSuboptions) {
     uint16_t opt_code = 80;
 
@@ -321,6 +405,6 @@ TEST_F(Option6IntTest, unpackSuboptions) {
     subopt = opt->getOption(1);
     // Expecting NULL which means that option does not exist.
     ASSERT_FALSE(subopt);
-} 
+}
 
 } // anonymous namespace

src/lib/dhcp/tests/option_definition_unittest.cc

@@ -581,5 +581,28 @@ TEST_F(OptionDefinitionTest, factoryUint32Array) {
     );
 }
 
+TEST_F(OptionDefinitionTest, recognizeFormat) {
+    // IA_NA option format.
+    OptionDefinition opt_def1("OPTION_IA_NA", D6O_IA_NA, "record");
+    for (int i = 0; i < 3; ++i) {
+        opt_def1.addRecordField("uint32");
+    }
+    EXPECT_TRUE(opt_def1.haveIA6Format());
+    // Create non-matching format to check that this function does not
+    // return 'true' all the time.
+    OptionDefinition opt_def2("OPTION_IA_NA", D6O_IA_NA, "uint16");
+    EXPECT_FALSE(opt_def2.haveIA6Format());
+
+    // IAADDR option format.
+    OptionDefinition opt_def3("OPTION_IAADDR", D6O_IAADDR, "record");
+    opt_def3.addRecordField("ipv6-address");
+    opt_def3.addRecordField("uint32");
+    opt_def3.addRecordField("uint32");
+    EXPECT_TRUE(opt_def3.haveIAAddr6Format());
+    // Create non-matching format to check that this function does not
+    // return 'true' all the time.
+    OptionDefinition opt_def4("OPTION_IAADDR", D6O_IAADDR, "uint32", true);
+    EXPECT_FALSE(opt_def4.haveIAAddr6Format());
+}
 
 } // anonymous namespace