[2637] Merged Option::pack4() and Option::pack6() into single function.

src/lib/dhcp/libdhcp++.cc

@@ -267,20 +267,12 @@ size_t LibDHCP::unpackOptions4(const OptionBuffer& buf,
     return (offset);
 }
 
-void LibDHCP::packOptions6(isc::util::OutputBuffer &buf,
-                           const isc::dhcp::Option::OptionCollection& options) {
-    for (Option::OptionCollection::const_iterator it = options.begin();
-         it != options.end(); ++it) {
-        it->second->pack(buf);
-    }
-}
-
 void
 LibDHCP::packOptions(isc::util::OutputBuffer& buf,
                      const Option::OptionCollection& options) {
     for (Option::OptionCollection::const_iterator it = options.begin();
          it != options.end(); ++it) {
-        it->second->pack4(buf);
+        it->second->pack(buf);
     }
 }
 

src/lib/dhcp/libdhcp++.h

@@ -88,16 +88,6 @@ public:
                                               uint16_t type,
                                               const OptionBuffer& buf);
 
-    /// Builds collection of options.
-    ///
-    /// Builds raw (on-wire) data for provided collection of options.
-    ///
-    /// @param buf output buffer (assembled options will be stored here)
-    /// @param options collection of options to store to
-    static void packOptions6(isc::util::OutputBuffer& buf,
-                             const isc::dhcp::Option::OptionCollection& options);
-
-
     /// @brief Stores options in a buffer.
     ///
     /// Stores all options defined in options containers in a on-wire

src/lib/dhcp/option.cc

@@ -84,51 +84,14 @@ Option::check() {
 }
 
 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 as the "
-                  << "universe type is unknown.");
+    // Write a header.
+    packHeader(buf);
+    // Write data.
+    if (!data_.empty()) {
+        buf.writeData(&data_[0], data_.size());
     }
-}
-
-void
-Option::pack4(isc::util::OutputBuffer& buf) {
-    if (universe_ == V4) {
-        // Write a header.
-        packHeader(buf);
-        // Write data.
-        if (!data_.empty()) {
-            buf.writeData(&data_[0], data_.size());
-        }
-        // Write sub-options.
-        packOptions(buf);
-    } else {
-        isc_throw(BadValue, "Invalid universe type " << universe_);
-    }
-
-    return;
-}
-
-void Option::pack6(isc::util::OutputBuffer& buf) {
-    if (universe_ == V6) {
-        // Write a header.
-        packHeader(buf);
-        // Write data.
-        if (!data_.empty()) {
-            buf.writeData(&data_[0], data_.size());
-        }
-        // Write sub-options.
-        packOptions(buf);
-    } else {
-        isc_throw(BadValue, "Invalid universe type " << universe_);
-    }
-    return;
+    // Write sub-options.
+    packOptions(buf);
 }
 
 void
@@ -153,16 +116,7 @@ Option::packHeader(isc::util::OutputBuffer& buf) {
 
 void
 Option::packOptions(isc::util::OutputBuffer& buf) {
-    switch (universe_) {
-    case V4:
-        LibDHCP::packOptions(buf, options_);
-        return;
-    case V6:
-        LibDHCP::packOptions6(buf, options_);
-        return;
-    default:
-        isc_throw(isc::BadValue, "Invalid universe type " << universe_);
-    }
+    LibDHCP::packOptions(buf, options_);
 }
 
 void Option::unpack(OptionBufferConstIter begin,

src/lib/dhcp/option.h

@@ -158,28 +158,13 @@ public:
     ///
     /// Writes option in wire-format to buffer, returns pointer to first unused
     /// 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
+    /// another).
     ///
     /// @param buf pointer to a buffer
     ///
     /// @throw BadValue 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,
-    /// 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 BadValue Universe is not V4.
-    virtual void pack4(isc::util::OutputBuffer& buf);
-
     /// @brief Parses received buffer.
     ///
     /// @param begin iterator to first byte of option data
@@ -317,13 +302,6 @@ public:
     virtual bool equal(const OptionPtr& other) const;
 
 protected:
-    /// Builds raw (over-wire) buffer of this option, including all
-    /// defined suboptions. Version for building DHCPv4 options.
-    ///
-    /// @param buf output buffer (built options will be stored here)
-    ///
-    /// @throw BadValue Universe is not V6.
-    virtual void pack6(isc::util::OutputBuffer& buf);
 
     /// @brief Store option's header in a buffer.
     ///

src/lib/dhcp/option4_addrlst.cc

@@ -64,7 +64,7 @@ Option4AddrLst::Option4AddrLst(uint8_t type, const IOAddress& addr)
 }
 
 void
-Option4AddrLst::pack4(isc::util::OutputBuffer& buf) {
+Option4AddrLst::pack(isc::util::OutputBuffer& buf) {
 
     if (addrs_.size() * V4ADDRESS_LEN > 255) {
         isc_throw(OutOfRange, "DHCPv4 Option4AddrLst " << type_ << " is too big."

src/lib/dhcp/option4_addrlst.h

@@ -87,11 +87,8 @@ public:
     ///
     /// Method will throw if option storing fails for some reason.
     ///
-    /// 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)
-    virtual void pack4(isc::util::OutputBuffer& buf);
+    virtual void pack(isc::util::OutputBuffer& buf);
 
     /// Returns string representation of the option.
     ///

src/lib/dhcp/option_custom.cc

@@ -387,14 +387,10 @@ OptionCustom::dataFieldToText(const OptionDataType data_type,
 }
 
 void
-OptionCustom::pack4(isc::util::OutputBuffer& buf) {
-    if (len() > 255) {
-        isc_throw(OutOfRange, "DHCPv4 Option " << type_
-                  << " value is too high. At most 255 is supported.");
-    }
+OptionCustom::pack(isc::util::OutputBuffer& buf) {
 
-    buf.writeUint8(type_);
-    buf.writeUint8(len() - getHeaderLen());
+    // Pack DHCP header (V4 or V6).
+    packHeader(buf);
 
     // Write data from buffers.
     for (std::vector<OptionBuffer>::const_iterator it = buffers_.begin();
@@ -411,21 +407,6 @@ OptionCustom::pack4(isc::util::OutputBuffer& buf) {
     packOptions(buf);
 }
 
-void
-OptionCustom::pack6(isc::util::OutputBuffer& buf) {
-    buf.writeUint16(type_);
-    buf.writeUint16(len() - getHeaderLen());
-
-    // Write data from buffers.
-    for (std::vector<OptionBuffer>::const_iterator it = buffers_.begin();
-         it != buffers_.end(); ++it) {
-        if (!it->empty()) {
-            buf.writeData(&(*it)[0], it->size());
-        }
-    }
-
-    packOptions(buf);
-}
 
 asiolink::IOAddress
 OptionCustom::readAddress(const uint32_t index) const {

src/lib/dhcp/option_custom.h

@@ -249,6 +249,11 @@ public:
     void writeString(const std::string& text,
                      const uint32_t index = 0);
 
+    /// @brief Writes DHCP option in a wire format to a buffer.
+    ///
+    /// @param buf output buffer (option will be stored there).
+    virtual void pack(isc::util::OutputBuffer& buf);
+
     /// @brief Parses received buffer.
     ///
     /// @param begin iterator to first byte of option data
@@ -278,18 +283,6 @@ public:
     void setData(const OptionBufferConstIter first,
                  const OptionBufferConstIter last);
 
-protected:
-
-    /// @brief Writes DHCPv4 option in a wire format to a buffer.
-    ///
-    /// @param buf output buffer (option will be stored there).
-    virtual void pack4(isc::util::OutputBuffer& buf);
-
-    /// @brief Writes DHCPv6 option in a wire format to a buffer.
-    ///
-    /// @param buf output buffer (built options will be stored here)
-    virtual void pack6(isc::util::OutputBuffer& buf);
-
 private:
 
     /// @brief Verify that the option comprises an array of values.

src/lib/dhcp/pkt6.cc

@@ -90,7 +90,7 @@ Pkt6::packUDP() {
         bufferOut_.writeUint8( (transid_) & 0xff );
 
         // the rest are options
-        LibDHCP::packOptions6(bufferOut_, options_);
+        LibDHCP::packOptions(bufferOut_, options_);
     }
     catch (const Exception& e) {
         /// @todo: throw exception here once we turn this function to void.

src/lib/dhcp/tests/libdhcp++_unittest.cc

@@ -259,7 +259,7 @@ TEST_F(LibDhcpTest, packOptions6) {
 
     OutputBuffer assembled(512);
 
-    EXPECT_NO_THROW(LibDHCP::packOptions6(assembled, opts));
+    EXPECT_NO_THROW(LibDHCP::packOptions(assembled, opts));
     EXPECT_EQ(sizeof(v6packed), assembled.getLength());
     EXPECT_EQ(0, memcmp(assembled.getData(), v6packed, sizeof(v6packed)));
 }

src/lib/dhcp/tests/option4_addrlst_unittest.cc

@@ -155,7 +155,7 @@ TEST_F(Option4AddrLstTest, assembly1) {
 
     OutputBuffer buf(100);
     EXPECT_NO_THROW(
-        opt->pack4(buf);
+        opt->pack(buf);
     );
 
     ASSERT_EQ(6, opt->len());
@@ -198,7 +198,7 @@ TEST_F(Option4AddrLstTest, assembly4) {
 
     OutputBuffer buf(100);
     EXPECT_NO_THROW(
-        opt->pack4(buf);
+        opt->pack(buf);
     );
 
     ASSERT_EQ(18, opt->len()); // 2(header) + 4xsizeof(IPv4addr)

src/lib/dhcp/tests/option_unittest.cc

@@ -116,7 +116,7 @@ TEST_F(OptionTest, v4_data1) {
     // now store that option into a buffer
     OutputBuffer buf(100);
     EXPECT_NO_THROW(
-        opt->pack4(buf);
+        opt->pack(buf);
     );
 
     // check content of that buffer
@@ -173,7 +173,7 @@ TEST_F(OptionTest, v4_data2) {
     // now store that option into a buffer
     OutputBuffer buf(100);
     EXPECT_NO_THROW(
-        opt->pack4(buf);
+        opt->pack(buf);
     );
 
     // check content of that buffer
@@ -471,7 +471,7 @@ TEST_F(OptionTest, setUintX) {
     // verify setUint8
     opt1->setUint8(255);
     EXPECT_EQ(255, opt1->getUint8());
-    opt1->pack4(outBuf_);
+    opt1->pack(outBuf_);
     EXPECT_EQ(3, opt1->len());
     EXPECT_EQ(3, outBuf_.getLength());
     uint8_t exp1[] = {125, 1, 255};
@@ -480,7 +480,7 @@ TEST_F(OptionTest, setUintX) {
     // verify getUint16
     outBuf_.clear();
     opt2->setUint16(12345);
-    opt2->pack4(outBuf_);
+    opt2->pack(outBuf_);
     EXPECT_EQ(12345, opt2->getUint16());
     EXPECT_EQ(4, opt2->len());
     EXPECT_EQ(4, outBuf_.getLength());
@@ -490,7 +490,7 @@ TEST_F(OptionTest, setUintX) {
     // verify getUint32
     outBuf_.clear();
     opt4->setUint32(0x12345678);
-    opt4->pack4(outBuf_);
+    opt4->pack(outBuf_);
     EXPECT_EQ(0x12345678, opt4->getUint32());
     EXPECT_EQ(6, opt4->len());
     EXPECT_EQ(6, outBuf_.getLength());
@@ -505,7 +505,7 @@ TEST_F(OptionTest, setData) {
                               buf_.begin(), buf_.begin() + 10));
     buf_.resize(20, 1);
     opt1->setData(buf_.begin(), buf_.end());
-    opt1->pack4(outBuf_);
+    opt1->pack(outBuf_);
     ASSERT_EQ(outBuf_.getLength() - opt1->getHeaderLen(), buf_.size());
     const uint8_t* test_data = static_cast<const uint8_t*>(outBuf_.getData());
     EXPECT_TRUE(0 == memcmp(&buf_[0], test_data + opt1->getHeaderLen(),
@@ -518,7 +518,7 @@ TEST_F(OptionTest, setData) {
     outBuf_.clear();
     buf_.resize(5, 1);
     opt2->setData(buf_.begin(), buf_.end());
-    opt2->pack4(outBuf_);
+    opt2->pack(outBuf_);
     ASSERT_EQ(outBuf_.getLength() - opt1->getHeaderLen(), buf_.size());
     test_data = static_cast<const uint8_t*>(outBuf_.getData());
     EXPECT_TRUE(0 == memcmp(&buf_[0], test_data + opt1->getHeaderLen(),