BIND 10 trac1228, updated. bfab5a33ceabe3f0d31bd465d13308c8b84adf68 [1228] Compilation fix in test.

BIND 10 source code commits bind10-changes at lists.isc.org
Wed Oct 26 17:21:18 UTC 2011


The branch, trac1228 has been updated
       via  bfab5a33ceabe3f0d31bd465d13308c8b84adf68 (commit)
       via  ef51c8418dc44bf2882c898990b30fc76ca9a97b (commit)
       via  ab642e89554bedf0a66c2358db71ec16ddeb2e7f (commit)
       via  91c2cf35e41642a997df020de797324bb4cfedcc (commit)
       via  c6e8dd84e81f5686d45cc41f514d4f61d075a276 (commit)
       via  7d25b201c0bc91987c4d9743d0c21b9486b98fd8 (commit)
       via  b01c18148a840b0d5719cbcd2653bf1b346e45f9 (commit)
      from  41f528a9eacdb430406a0d9047049585cae31db8 (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 bfab5a33ceabe3f0d31bd465d13308c8b84adf68
Author: Tomek Mrugalski <tomasz at isc.org>
Date:   Wed Oct 26 19:20:53 2011 +0200

    [1228] Compilation fix in test.

commit ef51c8418dc44bf2882c898990b30fc76ca9a97b
Author: Tomek Mrugalski <tomasz at isc.org>
Date:   Wed Oct 26 19:16:26 2011 +0200

    [1228] Option6Collection renamed to OptionCollection

commit ab642e89554bedf0a66c2358db71ec16ddeb2e7f
Author: Tomek Mrugalski <tomasz at isc.org>
Date:   Wed Oct 26 19:08:17 2011 +0200

    [1228] Missing tests for Pkt4 implemented.
    
    - Missing tests for Pkt4 implemented.
    - Option::addOption() now supports DHCPv4 options.
    - Pkt4::addOption(), getOption() implemented.
    - Include paths fixed in Makefile.am

commit 91c2cf35e41642a997df020de797324bb4cfedcc
Author: Tomek Mrugalski <tomasz at isc.org>
Date:   Wed Oct 26 19:03:12 2011 +0200

    [1228] InputBuffer::readVector() implemented (+tests)

commit c6e8dd84e81f5686d45cc41f514d4f61d075a276
Author: Tomek Mrugalski <tomasz at isc.org>
Date:   Wed Oct 26 17:23:33 2011 +0200

    [1228] LibDHCP::unpackOptions4 implemented (+tests)

commit 7d25b201c0bc91987c4d9743d0c21b9486b98fd8
Author: Tomek Mrugalski <tomasz at isc.org>
Date:   Wed Oct 26 16:27:29 2011 +0200

    [1228] Work on option support in DHCPv4
    
    - More code migrated to vector<uint8_t>
    - Option4Collection type deemed unnecessary and removed
    - Removed data_len_ field. data_.size() will be used instead
    - unitests for new functionalities in Option class implemented

commit b01c18148a840b0d5719cbcd2653bf1b346e45f9
Author: Tomek Mrugalski <tomasz at isc.org>
Date:   Wed Oct 26 12:53:33 2011 +0200

    [1228] Initial changes for supporting options in DHCPv4
    
    - getData() now returns const vector<uint8_t>&
    - several skeleton tests added
    - implemented Option::getUniverse()
    - Internally Option uses vector<uint8_t> to keep data. However, to avoid
      refactoring of all DHCPv6 code at this stage, API still uses
      share_array<uint8_t>. There is separate ticket for fixing this.

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

Summary of changes:
 src/bin/dhcp6/tests/dhcp6_srv_unittest.cc      |    8 +-
 src/lib/dhcp/libdhcp.cc                        |   63 +++++++++--
 src/lib/dhcp/libdhcp.h                         |   26 ++++-
 src/lib/dhcp/option.cc                         |  138 +++++++++++++++++-------
 src/lib/dhcp/option.h                          |   58 ++++++++---
 src/lib/dhcp/option6_ia.cc                     |    4 +-
 src/lib/dhcp/option6_iaaddr.cc                 |    4 +-
 src/lib/dhcp/pkt4.cc                           |   36 ++++++-
 src/lib/dhcp/pkt4.h                            |   22 ++++-
 src/lib/dhcp/pkt6.cc                           |    8 +-
 src/lib/dhcp/pkt6.h                            |    2 +-
 src/lib/dhcp/tests/Makefile.am                 |    2 -
 src/lib/dhcp/tests/libdhcp_unittest.cc         |  117 +++++++++++++++++---
 src/lib/dhcp/tests/option6_addrlst_unittest.cc |   17 ++--
 src/lib/dhcp/tests/option6_ia_unittest.cc      |    4 +-
 src/lib/dhcp/tests/option6_iaaddr_unittest.cc  |    2 +
 src/lib/dhcp/tests/option_unittest.cc          |   91 +++++++++++++---
 src/lib/dhcp/tests/pkt4_unittest.cc            |  135 +++++++++++++++++++++---
 src/lib/dhcp/tests/pkt6_unittest.cc            |    8 +-
 src/lib/util/buffer.h                          |   26 ++++-
 src/lib/util/tests/buffer_unittest.cc          |   32 ++++++
 21 files changed, 658 insertions(+), 145 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
index f9a1d9d..fcdd2d6 100644
--- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
+++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
@@ -130,13 +130,15 @@ TEST_F(Dhcpv6SrvTest, Solicit_basic) {
     ASSERT_TRUE( tmp );
     EXPECT_EQ(clientid->getType(), tmp->getType() );
     ASSERT_EQ(clientid->len(), tmp->len() );
-    EXPECT_FALSE(memcmp(clientid->getData(), tmp->getData(), tmp->len() ) );
+
+    EXPECT_EQ( clientid->getData(), tmp->getData() );
+
     // check that server included its server-id
     tmp = reply->getOption(D6O_SERVERID);
     EXPECT_EQ(tmp->getType(), srv->getServerID()->getType() );
     ASSERT_EQ(tmp->len(),  srv->getServerID()->len() );
-    EXPECT_FALSE( memcmp(tmp->getData(), srv->getServerID()->getData(),
-                      tmp->len()) );
+
+    EXPECT_EQ(tmp->getData(), srv->getServerID()->getData());
 
     // more checks to be implemented
     delete srv;
diff --git a/src/lib/dhcp/libdhcp.cc b/src/lib/dhcp/libdhcp.cc
index 8e6314e..80b99de 100644
--- a/src/lib/dhcp/libdhcp.cc
+++ b/src/lib/dhcp/libdhcp.cc
@@ -14,16 +14,17 @@
 
 #include <boost/shared_array.hpp>
 #include <boost/shared_ptr.hpp>
-#include "dhcp/libdhcp.h"
+#include <util/buffer.h>
+#include <dhcp/libdhcp.h>
 #include "config.h"
-#include "dhcp6.h"
-
-#include "option.h"
-#include "option6_ia.h"
-#include "option6_iaaddr.h"
+#include <dhcp/dhcp6.h>
+#include <dhcp/option.h>
+#include <dhcp/option6_ia.h>
+#include <dhcp/option6_iaaddr.h>
 
 using namespace std;
 using namespace isc::dhcp;
+using namespace isc::util;
 
 // static array with factories for options
 std::map<unsigned short, Option::Factory*> LibDHCP::v6factories_;
@@ -32,7 +33,7 @@ unsigned int
 LibDHCP::unpackOptions6(const boost::shared_array<uint8_t> buf,
                         unsigned int buf_len,
                         unsigned int offset, unsigned int parse_len,
-                        isc::dhcp::Option::Option6Collection& options) {
+                        isc::dhcp::Option::OptionCollection& options) {
     if (offset + parse_len > buf_len) {
         isc_throw(OutOfRange, "Option parse failed. Tried to parse "
                   << parse_len << " bytes at offset " << offset
@@ -83,13 +84,44 @@ LibDHCP::unpackOptions6(const boost::shared_array<uint8_t> buf,
     return (offset);
 }
 
+void
+LibDHCP::unpackOptions4(const std::vector<uint8_t>& buf,
+                        isc::dhcp::Option::OptionCollection& options) {
+    size_t offset = 0;
+
+    // 2 - header of DHCPv4 option
+    while (offset + 2 <= buf.size()) {
+        uint8_t opt_type = buf[offset++];
+        uint8_t opt_len =  buf[offset++];
+        if (offset + opt_len > buf.size() ) {
+            isc_throw(OutOfRange, "Option parse failed. Tried to parse "
+                      << offset + opt_len << " bytes from " << buf.size()
+                      << "-byte long buffer.");
+        }
+
+        boost::shared_ptr<Option> opt;
+        switch(opt_type) {
+        default:
+            /// TODO: Is this intermediate object really necessary here?
+            vector<uint8_t> tmpVec(&buf[offset], &buf[offset] + opt_len);
+            opt = boost::shared_ptr<Option>(new Option(Option::V4,
+                                                       opt_type,
+                                                       tmpVec));
+        }
+
+        options.insert(pair<int, boost::shared_ptr<Option> >(opt_type, opt));
+        offset += opt_len;
+    }
+}
+
+
 unsigned int
 LibDHCP::packOptions6(boost::shared_array<uint8_t> data,
                       unsigned int data_len,
                       unsigned int offset,
-                      const isc::dhcp::Option::Option6Collection& options) {
+                      const isc::dhcp::Option::OptionCollection& options) {
     try {
-        for (isc::dhcp::Option::Option6Collection::const_iterator it = options.begin();
+        for (Option::OptionCollection::const_iterator it = options.begin();
              it != options.end();
              ++it) {
             unsigned short opt_len = (*it).second->len();
@@ -97,7 +129,7 @@ LibDHCP::packOptions6(boost::shared_array<uint8_t> data,
                 isc_throw(OutOfRange, "Failed to build option " <<
                           (*it).first << ": out of buffer");
             }
-            offset = (*it).second->pack(data, data_len, offset);
+            offset = it->second->pack(data, data_len, offset);
         }
     }
     catch (const Exception& e) {
@@ -107,6 +139,17 @@ LibDHCP::packOptions6(boost::shared_array<uint8_t> data,
     return (offset);
 }
 
+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);
+    }
+}
+
+
 bool
 LibDHCP::OptionFactoryRegister(Option::Universe u,
                                unsigned short opt_type,
diff --git a/src/lib/dhcp/libdhcp.h b/src/lib/dhcp/libdhcp.h
index c2ac949..468e6bb 100644
--- a/src/lib/dhcp/libdhcp.h
+++ b/src/lib/dhcp/libdhcp.h
@@ -16,7 +16,8 @@
 #define LIBDHCP_H_
 
 #include <iostream>
-#include "dhcp/pkt6.h"
+#include <util/buffer.h>
+#include <dhcp/pkt6.h>
 
 namespace isc {
 namespace dhcp {
@@ -39,8 +40,27 @@ public:
     static unsigned int
     packOptions6(boost::shared_array<uint8_t> buf, unsigned int buf_len,
                  unsigned int offset,
-                 const isc::dhcp::Option::Option6Collection& options);
+                 const isc::dhcp::Option::OptionCollection& options);
 
+
+    /// @brief Stores options in a buffer.
+    ///
+    /// Stores all options defined in options containers in a on-wire
+    /// format in output buffer specified by buf.
+    ///
+    /// May throw different exceptions if option assembly fails. There
+    /// may be different reasons (option too large, option malformed,
+    /// too many options etc.)
+    ///
+    /// @param buf
+    /// @param options
+    static void
+    packOptions(isc::util::OutputBuffer& buf,
+                const isc::dhcp::Option::OptionCollection& options);
+
+    static void
+    unpackOptions4(const std::vector<uint8_t>& buf,
+                   isc::dhcp::Option::OptionCollection& options);
     ///
     /// Parses provided buffer and creates Option objects.
     ///
@@ -57,7 +77,7 @@ public:
     static unsigned int
     unpackOptions6(const boost::shared_array<uint8_t> buf, unsigned int buf_len,
                    unsigned int offset, unsigned int parse_len,
-                   isc::dhcp::Option::Option6Collection& options_);
+                   isc::dhcp::Option::OptionCollection& options_);
 
     ///
     /// Registers factory method that produces options of specific option types.
diff --git a/src/lib/dhcp/option.cc b/src/lib/dhcp/option.cc
index dd45c34..9f8635e 100644
--- a/src/lib/dhcp/option.cc
+++ b/src/lib/dhcp/option.cc
@@ -29,50 +29,100 @@ using namespace isc::dhcp;
 using namespace isc::util;
 
 Option::Option(Universe u, unsigned short type)
-    :universe_(u), type_(type), data_len_(0) {
-
+    :universe_(u), type_(type) {
 
+    if (u==V4 && (type>255)) {
+        isc_throw(BadValue, "Can't create V4 option of type "
+                  << type << ", V4 options are in range 0..255");
+    }
 }
 
 Option::Option(Universe u, unsigned short type,
                const boost::shared_array<uint8_t>& buf,
                unsigned int offset, unsigned int len)
-    :universe_(u), type_(type), data_(buf),
-     data_len_(len), offset_(offset)
-      {
+    :universe_(u), type_(type),
+     offset_(offset)
+{
+    if (u==V4 && (type>255)) {
+        isc_throw(BadValue, "Can't create V4 option of type "
+                  << type << ", V4 options are in range 0..255");
+    }
+
+    uint8_t* ptr = &buf[offset];
+    data_ = std::vector<uint8_t>(ptr, ptr+len);
 
     // sanity checks
     // TODO: universe must be in V4 and V6
 }
 
+Option::Option(Universe u, unsigned short type, std::vector<uint8_t>& data)
+    :universe_(u), type_(type), data_(data) {
+    if (u==V4 && data.size() > 255) {
+        isc_throw(OutOfRange, "DHCPv4 Option " << type_ << " is too big."
+                  << "At most 255 bytes are supported.");
+        /// TODO Larger options can be stored as separate instances
+        /// of DHCPv4 options. Clients MUST concatenate them.
+        /// Fortunately, there are no such large options used today.
+    }
+}
+
+
 unsigned int
 Option::pack(boost::shared_array<uint8_t>& buf,
              unsigned int buf_len,
              unsigned int offset) {
+    if (universe_ != V6) {
+        isc_throw(BadValue, "Failed to pack " << type_ << " option. Do not "
+                  << "use this method for options other than DHCPv6.");
+    }
+    return pack6(buf, buf_len, offset);
+}
+
+void
+Option::pack4(isc::util::OutputBuffer& buf) {
     switch (universe_) {
-    case V4:
-        return pack4(buf, buf_len, offset);
+    case V4: {
+        if (data_.size()>255) {
+            isc_throw(OutOfRange, "DHCPv4 Option " << type_ << " is too big."
+                      << "At most 255 bytes are supported.");
+            /// TODO Larger options can be stored as separate instances
+            /// of DHCPv4 options. Clients MUST concatenate them.
+            /// Fortunately, there are no such large options used today.
+        }
+
+        buf.writeUint8(type_);
+        buf.writeUint8(len() - getHeaderLen());
+
+        buf.writeData(&data_[0], data_.size());
+
+        LibDHCP::packOptions(buf, options_);
+        return;
+    }
     case V6:
-        return pack6(buf, buf_len, offset);
+        /// TODO: Do we need a sanity check for option size here?
+        buf.writeUint16(type_);
+        buf.writeUint16(len() - getHeaderLen());
+
+        LibDHCP::packOptions(buf, options_);
+        return;
     default:
-        isc_throw(BadValue, "Unknown universe defined for Option " << type_);
+        isc_throw(OutOfRange, "Invalid universe type" << universe_);
     }
 }
 
-
 unsigned int
 Option::pack4(boost::shared_array<uint8_t>& buf,
              unsigned int buf_len,
              unsigned int offset) {
     if ( offset+len() > buf_len ) {
         isc_throw(OutOfRange, "Failed to pack v4 option=" <<
-                  type_ << ",len=" << data_len_ << ": too small buffer.");
+                  type_ << ",len=" << len() << ": too small buffer.");
     }
     uint8_t *ptr = &buf[offset];
     ptr[0] = type_;
-    ptr[1] = data_len_;
+    ptr[1] = len() - getHeaderLen();
     ptr += 2;
-    memcpy(ptr, &data_[0], data_len_);
+    memcpy(ptr, &data_[0], data_.size());
 
     return offset + len();
 }
@@ -92,11 +142,11 @@ Option::pack6(boost::shared_array<uint8_t>& buf,
 
     ptr = writeUint16(len() - getHeaderLen(), ptr);
 
-    if (data_len_)
-        memcpy(ptr, &data_[offset_], data_len_);
+    if (data_.size())
+        memcpy(ptr, &data_[0], data_.size());
 
     // end of fixed part of this option
-    offset += OPTION6_HDR_LEN + data_len_;
+    offset += OPTION6_HDR_LEN + data_.size();
 
     return LibDHCP::packOptions6(buf, buf_len, offset, options_);
 }
@@ -140,22 +190,27 @@ Option::unpack6(const boost::shared_array<uint8_t>& buf,
                   << "): too small buffer.");
     }
 
-    data_ = buf;
+    uint8_t* ptr = &buf[offset];
+    data_ = std::vector<uint8_t>(ptr, ptr+parse_len);
+
     offset_ = offset;
-    data_len_ = buf_len;
 
-    return LibDHCP::unpackOptions6(buf, buf_len, offset, parse_len,
-                                   options_);
+    return (offset+parse_len);
+
+    //return LibDHCP::unpackOptions6(buf, buf_len, offset, parse_len,
+    //                               options_);
 }
 
+/// Returns length of the complete option (data length + DHCPv4/DHCPv6
+/// option header)
 unsigned short
 Option::len() {
 
     // length of the whole option is header and data stored in this option...
-    int length = getHeaderLen() + data_len_;
+    int length = getHeaderLen() + data_.size();
 
     // ... and sum of lengths of all suboptions
-    for (Option::Option6Collection::iterator it = options_.begin();
+    for (Option::OptionCollection::iterator it = options_.begin();
          it != options_.end();
          ++it) {
         length += (*it).second->len();
@@ -177,16 +232,9 @@ Option::valid() {
     return (true);
 }
 
-void
-isc::dhcp::Option::addOption(boost::shared_ptr<isc::dhcp::Option> opt) {
-    options_.insert(pair<int, boost::shared_ptr<Option> >(opt->getType(),
-                                                            opt));
-
-}
-
 boost::shared_ptr<isc::dhcp::Option>
 Option::getOption(unsigned short opt_type) {
-    isc::dhcp::Option::Option6Collection::const_iterator x =
+    isc::dhcp::Option::OptionCollection::const_iterator x =
         options_.find(opt_type);
     if ( x != options_.end() ) {
         return (*x).second;
@@ -196,7 +244,7 @@ Option::getOption(unsigned short opt_type) {
 
 bool
 Option::delOption(unsigned short opt_type) {
-    isc::dhcp::Option::Option6Collection::iterator x = options_.find(opt_type);
+    isc::dhcp::Option::OptionCollection::iterator x = options_.find(opt_type);
     if ( x != options_.end() ) {
         options_.erase(x);
         return true; // delete successful
@@ -211,18 +259,18 @@ std::string Option::toText(int indent /* =0 */ ) {
     for (int i=0; i<indent; i++)
         tmp << " ";
 
-    tmp << "type=" << type_ << ", len=" << data_len_ << ": ";
+    tmp << "type=" << type_ << ", len=" << len()-getHeaderLen() << ": ";
 
-    for (unsigned int i=0; i<data_len_; i++) {
+    for (unsigned int i=0; i<data_.size(); i++) {
         if (i) {
             tmp << ":";
         }
         tmp << setfill('0') << setw(2) << hex
-            << static_cast<unsigned short>(data_[offset_+i]);
+            << static_cast<unsigned short>(data_[i]);
     }
 
     // print suboptions
-    for (Option6Collection::const_iterator opt=options_.begin();
+    for (OptionCollection::const_iterator opt=options_.begin();
          opt!=options_.end();
          ++opt) {
         tmp << (*opt).second->toText(indent+2);
@@ -235,13 +283,9 @@ Option::getType() {
     return type_;
 }
 
-uint8_t*
+const std::vector<uint8_t>&
 Option::getData() {
-    if (data_len_) {
-        return (&data_[offset_]);
-    } else {
-        return (NULL);
-    }
+    return (data_);
 }
 
 unsigned short
@@ -255,6 +299,18 @@ Option::getHeaderLen() {
     return 0; // should not happen
 }
 
+void
+Option::addOption(boost::shared_ptr<Option> opt) {
+    if (universe_ == V4) {
+        // check for uniqueness (DHCPv4 options must be unique)
+        if (getOption(opt->getType())) {
+            isc_throw(BadValue, "Option " << opt->getType()
+                      << " already present in this message.");
+        }
+    }
+    options_.insert(pair<int, boost::shared_ptr<Option> >(opt->getType(), opt));
+}
+
 Option::~Option() {
 
 }
diff --git a/src/lib/dhcp/option.h b/src/lib/dhcp/option.h
index 5be1be3..cd64991 100644
--- a/src/lib/dhcp/option.h
+++ b/src/lib/dhcp/option.h
@@ -17,8 +17,10 @@
 
 #include <string>
 #include <map>
+#include <vector>
 #include <boost/shared_ptr.hpp>
 #include <boost/shared_array.hpp>
+#include <util/buffer.h>
 
 namespace isc {
 namespace dhcp {
@@ -34,13 +36,9 @@ public:
     /// defines option universe DHCPv4 or DHCPv6
     enum Universe { V4, V6 };
 
-    /// a collection of DHCPv4 options
-    typedef std::map<unsigned int, boost::shared_ptr<Option> >
-    Option4Collection;
-
     /// a collection of DHCPv6 options
     typedef std::multimap<unsigned int, boost::shared_ptr<Option> >
-    Option6Collection;
+    OptionCollection;
 
     /// @brief a factory function prototype
     ///
@@ -80,11 +78,31 @@ public:
            const boost::shared_array<uint8_t>& buf, unsigned int offset,
            unsigned int len);
 
-    /// @brief writes option in wire-format to buf
+    /// @brief Constructor, used for received options.
+    ///
+    /// 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.
+    ///
+    /// @param u specifies universe (V4 or V6)
+    /// @param type option type (0-255 for V4 and 0-65535 for V6)
+    /// @param data content of the option
+    Option(Universe u, unsigned short type, std::vector<uint8_t>& data);
+
+    /// @brief returns option universe (V4 or V6)
+    ///
+    /// @return universe type
+    Universe
+    getUniverse() { return universe_; };
+
+    /// @brief Writes option in wire-format to a buffer.
     ///
     /// 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)
+    /// another). Used in DHCPv6 options.
+    ///
+    /// TODO: Migrate DHCPv6 code to pack(OutputBuffer& buf) version
     ///
     /// @param buf pointer to a buffer
     /// @param buf_len length of the buffer
@@ -97,6 +115,18 @@ public:
          unsigned int buf_len,
          unsigned int offset);
 
+    /// @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)
+    virtual void
+    pack4(isc::util::OutputBuffer& buf);
+
+
     /// @brief Parses buffer.
     ///
     /// Parses received buffer, returns offset to the first unused byte after
@@ -150,7 +180,7 @@ public:
     /// Returns pointer to actual data.
     ///
     /// @return pointer to actual data (or NULL if there is no data)
-    virtual uint8_t*
+    virtual const std::vector<uint8_t>&
     getData();
 
     /// Adds a sub-option.
@@ -248,20 +278,18 @@ protected:
     /// option type (0-255 for DHCPv4, 0-65535 for DHCPv6)
     unsigned short type_;
 
-    /// shared pointer to a buffer (usually a part of packet)
-    boost::shared_array<uint8_t> data_;
-
-    /// length of data only. Use len() if you want to
-    /// know proper length with option header overhead
-    unsigned int data_len_;
+    /// contains content of this data
+    std::vector<uint8_t> data_;
 
+    /// TODO: Remove this field. vector<uint8_t> should be used
+    /// instead.
     /// data is a shared_pointer that points out to the
     /// whole packet. offset_ specifies where data for
     /// this option begins.
     unsigned int offset_;
 
     /// collection for storing suboptions
-    Option6Collection options_;
+    OptionCollection options_;
 
     /// TODO: probably 2 different containers have to be used for v4 (unique
     /// options) and v6 (options with the same type can repeat)
diff --git a/src/lib/dhcp/option6_ia.cc b/src/lib/dhcp/option6_ia.cc
index ee314db..46daee1 100644
--- a/src/lib/dhcp/option6_ia.cc
+++ b/src/lib/dhcp/option6_ia.cc
@@ -113,7 +113,7 @@ std::string Option6IA::toText(int indent /* = 0*/) {
     tmp << " iaid=" << iaid_ << ", t1=" << t1_ << ", t2=" << t2_
         << " " << options_.size() << " sub-options:" << endl;
 
-    for (Option6Collection::const_iterator opt=options_.begin();
+    for (OptionCollection::const_iterator opt=options_.begin();
          opt!=options_.end();
          ++opt) {
         tmp << (*opt).second->toText(indent+2);
@@ -127,7 +127,7 @@ unsigned short Option6IA::len() {
         OPTION6_IA_LEN  /* option content (12) */;
 
     // length of all suboptions
-    for (Option::Option6Collection::iterator it = options_.begin();
+    for (Option::OptionCollection::iterator it = options_.begin();
          it != options_.end();
          ++it) {
         length += (*it).second->len();
diff --git a/src/lib/dhcp/option6_iaaddr.cc b/src/lib/dhcp/option6_iaaddr.cc
index d5b57dd..4177714 100644
--- a/src/lib/dhcp/option6_iaaddr.cc
+++ b/src/lib/dhcp/option6_iaaddr.cc
@@ -108,7 +108,7 @@ std::string Option6IAAddr::toText(int indent /* =0 */) {
         << ", preferred-lft=" << preferred_  << ", valid-lft="
         << valid_ << endl;
 
-    for (Option6Collection::const_iterator opt=options_.begin();
+    for (OptionCollection::const_iterator opt=options_.begin();
          opt!=options_.end();
          ++opt) {
         tmp << (*opt).second->toText(indent+2);
@@ -123,7 +123,7 @@ unsigned short Option6IAAddr::len() {
     // length of all suboptions
     // TODO implement:
     // protected: unsigned short Option::lenHelper(int header_size);
-    for (Option::Option6Collection::iterator it = options_.begin();
+    for (Option::OptionCollection::iterator it = options_.begin();
          it != options_.end();
          ++it) {
         length += (*it).second->len();
diff --git a/src/lib/dhcp/pkt4.cc b/src/lib/dhcp/pkt4.cc
index 8a0295e..9e781ef 100644
--- a/src/lib/dhcp/pkt4.cc
+++ b/src/lib/dhcp/pkt4.cc
@@ -88,7 +88,13 @@ size_t
 Pkt4::len() {
     size_t length = DHCPV4_PKT_HDR_LEN; // DHCPv4 header
 
-    /// TODO: Include options here (ticket #1228)
+    // ... and sum of lengths of all options
+    for (Option::OptionCollection::const_iterator it = options_.begin();
+         it != options_.end();
+         ++it) {
+        length += (*it).second->len();
+    }
+
     return (length);
 }
 
@@ -109,7 +115,7 @@ Pkt4::pack() {
     bufferOut_.writeData(sname_, MAX_SNAME_LEN);
     bufferOut_.writeData(file_, MAX_FILE_LEN);
 
-    /// TODO: Options should follow here (ticket #1228)
+    LibDHCP::packOptions(bufferOut_, options_);
 
     return (true);
 }
@@ -136,7 +142,11 @@ Pkt4::unpack() {
     bufferIn_.readData(sname_, MAX_SNAME_LEN);
     bufferIn_.readData(file_, MAX_FILE_LEN);
 
-    /// TODO: Parse options here (ticket #1228)
+    size_t opts_len = bufferIn_.getLength() - bufferIn_.getPosition();
+    vector<uint8_t> optsBuffer;
+    // fist use of readVector
+    bufferIn_.readVector(optsBuffer, opts_len);
+    LibDHCP::unpackOptions4(optsBuffer, options_);
 
     return (true);
 }
@@ -220,6 +230,26 @@ Pkt4::DHCPTypeToBootpType(uint8_t dhcpType) {
     }
 }
 
+void
+Pkt4::addOption(boost::shared_ptr<Option> opt) {
+    // check for uniqueness (DHCPv4 options must be unique)
+    if (getOption(opt->getType())) {
+        isc_throw(BadValue, "Option " << opt->getType()
+                  << " already present in this message.");
+    }
+    options_.insert(pair<int, boost::shared_ptr<Option> >(opt->getType(), opt));
+}
+
+boost::shared_ptr<isc::dhcp::Option>
+Pkt4::getOption(uint8_t type) {
+    Option::OptionCollection::const_iterator x = options_.find(type);
+    if (x!=options_.end()) {
+        return (*x).second;
+    }
+    return boost::shared_ptr<isc::dhcp::Option>(); // NULL
+}
+
+
 } // end of namespace isc::dhcp
 
 } // end of namespace isc
diff --git a/src/lib/dhcp/pkt4.h b/src/lib/dhcp/pkt4.h
index bb9d9c4..23a1a9e 100644
--- a/src/lib/dhcp/pkt4.h
+++ b/src/lib/dhcp/pkt4.h
@@ -264,7 +264,7 @@ public:
     uint8_t
     getHlen() { return (hlen_); };
 
-    /// @brief Returns chaddr field
+    /// @brief Returns chaddr field.
     ///
     /// Note: This is 16 bytes long field. It doesn't have to be
     /// null-terminated. Do no use strlen() or similar on it.
@@ -274,7 +274,7 @@ public:
     getChaddr() { return (chaddr_); };
 
 
-    /// Returns reference to output buffer
+    /// @brief Returns reference to output buffer.
     ///
     /// Returned buffer will contain reasonable data only for
     /// output (TX) packet and after pack() was called.
@@ -286,6 +286,22 @@ public:
     isc::util::OutputBuffer&
     getBuffer() { return (bufferOut_); };
 
+    /// @brief Add an option.
+    ///
+    /// Throws BadValue if option with that type is already present.
+    ///
+    /// @param opt option to be added
+    void
+    addOption(boost::shared_ptr<Option> opt);
+
+    /// @brief Returns an option of specified type.
+    ///
+    /// @return returns option of requested type (or NULL)
+    ///         if no such option is present
+
+    boost::shared_ptr<Option>
+    getOption(uint8_t opt_type);
+
 protected:
 
     /// converts DHCP message type to BOOTP op type
@@ -382,7 +398,7 @@ protected:
     uint8_t msg_type_;
 
     /// collection of options present in this message
-    isc::dhcp::Option::Option4Collection options_;
+    isc::dhcp::Option::OptionCollection options_;
 }; // Pkt4 class
 
 } // isc::dhcp namespace
diff --git a/src/lib/dhcp/pkt6.cc b/src/lib/dhcp/pkt6.cc
index 70be2bb..84c5729 100644
--- a/src/lib/dhcp/pkt6.cc
+++ b/src/lib/dhcp/pkt6.cc
@@ -63,7 +63,7 @@ unsigned short
 Pkt6::len() {
     unsigned int length = DHCPV6_PKT_HDR_LEN; // DHCPv6 header
 
-    for (Option::Option6Collection::iterator it = options_.begin();
+    for (Option::OptionCollection::iterator it = options_.begin();
          it != options_.end();
          ++it) {
         length += (*it).second->len();
@@ -197,7 +197,7 @@ Pkt6::toText() {
         << "]:" << remote_port_ << endl;
     tmp << "msgtype=" << msg_type_ << ", transid=0x" << hex << transid_
         << dec << endl;
-    for (isc::dhcp::Option::Option6Collection::iterator opt=options_.begin();
+    for (isc::dhcp::Option::OptionCollection::iterator opt=options_.begin();
          opt != options_.end();
          ++opt) {
         tmp << opt->second->toText() << std::endl;
@@ -207,7 +207,7 @@ Pkt6::toText() {
 
 boost::shared_ptr<isc::dhcp::Option>
 Pkt6::getOption(unsigned short opt_type) {
-    isc::dhcp::Option::Option6Collection::const_iterator x = options_.find(opt_type);
+    isc::dhcp::Option::OptionCollection::const_iterator x = options_.find(opt_type);
     if (x!=options_.end()) {
         return (*x).second;
     }
@@ -221,7 +221,7 @@ Pkt6::addOption(boost::shared_ptr<Option> opt) {
 
 bool
 Pkt6::delOption(unsigned short type) {
-    isc::dhcp::Option::Option6Collection::iterator x = options_.find(type);
+    isc::dhcp::Option::OptionCollection::iterator x = options_.find(type);
     if (x!=options_.end()) {
         options_.erase(x);
         return (true); // delete successful
diff --git a/src/lib/dhcp/pkt6.h b/src/lib/dhcp/pkt6.h
index d089444..019eeb2 100644
--- a/src/lib/dhcp/pkt6.h
+++ b/src/lib/dhcp/pkt6.h
@@ -180,7 +180,7 @@ public:
     /// TODO Need to implement getOptions() as well
 
     /// collection of options present in this message
-    isc::dhcp::Option::Option6Collection options_;
+    isc::dhcp::Option::OptionCollection options_;
 
 protected:
     /// Builds on wire packet for TCP transmission.
diff --git a/src/lib/dhcp/tests/Makefile.am b/src/lib/dhcp/tests/Makefile.am
index 41cabba..01799da 100644
--- a/src/lib/dhcp/tests/Makefile.am
+++ b/src/lib/dhcp/tests/Makefile.am
@@ -1,8 +1,6 @@
 SUBDIRS = .
 
 AM_CPPFLAGS = -I$(top_builddir)/src/lib -I$(top_srcdir)/src/lib
-AM_CPPFLAGS += -I$(top_srcdir)/src/lib/asiolink
-AM_CPPFLAGS += -I$(top_builddir)/src/lib/asiolink
 AM_CPPFLAGS += $(BOOST_INCLUDES)
 AM_CXXFLAGS = $(B10_CXXFLAGS)
 
diff --git a/src/lib/dhcp/tests/libdhcp_unittest.cc b/src/lib/dhcp/tests/libdhcp_unittest.cc
index ccb3a7f..11b618c 100644
--- a/src/lib/dhcp/tests/libdhcp_unittest.cc
+++ b/src/lib/dhcp/tests/libdhcp_unittest.cc
@@ -15,16 +15,16 @@
 #include <config.h>
 #include <iostream>
 #include <sstream>
-
 #include <arpa/inet.h>
 #include <gtest/gtest.h>
-
-#include "dhcp/libdhcp.h"
+#include <util/buffer.h>
+#include <dhcp/libdhcp.h>
 #include "config.h"
 
 using namespace std;
 using namespace isc;
 using namespace isc::dhcp;
+using namespace isc::util;
 
 namespace {
 class LibDhcpTest : public ::testing::Test {
@@ -43,7 +43,7 @@ static const uint8_t packed[] = {
 
 TEST(LibDhcpTest, packOptions6) {
     boost::shared_array<uint8_t> buf(new uint8_t[512]);
-    isc::dhcp::Option::Option6Collection opts; // list of options
+    isc::dhcp::Option::OptionCollection opts; // list of options
 
     // generate content for options
     for (int i = 0; i < 64; i++) {
@@ -76,7 +76,7 @@ TEST(LibDhcpTest, unpackOptions6) {
     // Option is used as a simple option implementation
     // More advanced uses are validated in tests dedicated for
     // specific derived classes.
-    isc::dhcp::Option::Option6Collection options; // list of options
+    isc::dhcp::Option::OptionCollection options; // list of options
 
     // we can't use packed directly, as shared_array would try to
     // free it eventually
@@ -91,35 +91,35 @@ TEST(LibDhcpTest, unpackOptions6) {
     EXPECT_EQ(35, offset); // parsed first 35 bytes (offset 0..34)
     EXPECT_EQ(options.size(), 5); // there should be 5 options
 
-    isc::dhcp::Option::Option6Collection::const_iterator x = options.find(12);
+    isc::dhcp::Option::OptionCollection::const_iterator x = options.find(12);
     ASSERT_FALSE(x == options.end()); // option 1 should exist
     EXPECT_EQ(12, x->second->getType());  // this should be option 12
     ASSERT_EQ(9, x->second->len()); // it should be of length 9
-    EXPECT_EQ(0, memcmp(x->second->getData(), packed+4, 5)); // data len=5
+    EXPECT_EQ(0, memcmp(&x->second->getData()[0], packed+4, 5)); // data len=5
 
     x = options.find(13);
     ASSERT_FALSE(x == options.end()); // option 13 should exist
     EXPECT_EQ(13, x->second->getType());  // this should be option 13
     ASSERT_EQ(7, x->second->len()); // it should be of length 7
-    EXPECT_EQ(0, memcmp(x->second->getData(), packed+13, 3)); // data len=3
+    EXPECT_EQ(0, memcmp(&x->second->getData()[0], packed+13, 3)); // data len=3
 
     x = options.find(14);
     ASSERT_FALSE(x == options.end()); // option 3 should exist
     EXPECT_EQ(14, x->second->getType());  // this should be option 14
     ASSERT_EQ(6, x->second->len()); // it should be of length 6
-    EXPECT_EQ(0, memcmp(x->second->getData(), packed+20, 2)); // data len=2
+    EXPECT_EQ(0, memcmp(&x->second->getData()[0], packed+20, 2)); // data len=2
 
     x = options.find(256);
     ASSERT_FALSE(x == options.end()); // option 256 should exist
     EXPECT_EQ(256, x->second->getType());  // this should be option 256
     ASSERT_EQ(8, x->second->len()); // it should be of length 7
-    EXPECT_EQ(0, memcmp(x->second->getData(), packed+26, 4)); // data len=4
+    EXPECT_EQ(0, memcmp(&x->second->getData()[0], packed+26, 4)); // data len=4
 
     x = options.find(257);
     ASSERT_FALSE(x == options.end()); // option 257 should exist
     EXPECT_EQ(257, x->second->getType());  // this should be option 257
     ASSERT_EQ(5, x->second->len()); // it should be of length 5
-    EXPECT_EQ(0, memcmp(x->second->getData(), packed+34, 1)); // data len=1
+    EXPECT_EQ(0, memcmp(&x->second->getData()[0], packed+34, 1)); // data len=1
 
     x = options.find(0);
     EXPECT_TRUE(x == options.end()); // option 0 not found
@@ -134,14 +134,101 @@ TEST(LibDhcpTest, unpackOptions6) {
     EXPECT_TRUE(x == options.end()); // option 32000 not found
 }
 
+
+static uint8_t v4Opts[] = {
+    12,  3, 0,   1,  2,
+    13,  3, 10, 11, 12,
+    14,  3, 20, 21, 22,
+    254, 3, 30, 31, 32,
+    128, 3, 40, 41, 42
+};
+
 TEST(LibDhcpTest, packOptions4) {
-    // TODO
-    ASSERT_TRUE(false);
+
+    vector<uint8_t> payload[5];
+    for (int i = 0; i < 5; i++) {
+        payload[i].resize(3);
+        payload[i][0] = i*10;
+        payload[i][1] = i*10+1;
+        payload[i][2] = i*10+2;
+    }
+
+    boost::shared_ptr<Option> opt1(new Option(Option::V4, 12, payload[0]));
+    boost::shared_ptr<Option> opt2(new Option(Option::V4, 13, payload[1]));
+    boost::shared_ptr<Option> opt3(new Option(Option::V4, 14, payload[2]));
+    boost::shared_ptr<Option> opt4(new Option(Option::V4,254, payload[3]));
+    boost::shared_ptr<Option> opt5(new Option(Option::V4,128, payload[4]));
+
+    isc::dhcp::Option::OptionCollection opts; // list of options
+    opts.insert(pair<int, boost::shared_ptr<Option> >(opt1->getType(), opt1));
+    opts.insert(pair<int, boost::shared_ptr<Option> >(opt1->getType(), opt2));
+    opts.insert(pair<int, boost::shared_ptr<Option> >(opt1->getType(), opt3));
+    opts.insert(pair<int, boost::shared_ptr<Option> >(opt1->getType(), opt4));
+    opts.insert(pair<int, boost::shared_ptr<Option> >(opt1->getType(), opt5));
+
+    vector<uint8_t> expVect(v4Opts, v4Opts + sizeof(v4Opts));
+
+    OutputBuffer buf(100);
+    EXPECT_NO_THROW (
+        LibDHCP::packOptions(buf, opts);
+    );
+    ASSERT_EQ(buf.getLength(), sizeof(v4Opts));
+    EXPECT_EQ(0, memcmp(v4Opts, buf.getData(), sizeof(v4Opts)));
+
 }
 
 TEST(LibDhcpTest, unpackOptions4) {
-    // TODO
-    ASSERT_TRUE(false);
+
+    vector<uint8_t> packed(v4Opts, v4Opts + sizeof(v4Opts));
+    isc::dhcp::Option::OptionCollection options; // list of options
+
+    ASSERT_NO_THROW(
+        LibDHCP::unpackOptions4(packed, options);
+    );
+
+    isc::dhcp::Option::OptionCollection::const_iterator x = options.find(12);
+    ASSERT_FALSE(x == options.end()); // option 1 should exist
+    EXPECT_EQ(12, x->second->getType());  // this should be option 12
+    ASSERT_EQ(3, x->second->getData().size()); // it should be of length 3
+    EXPECT_EQ(5, x->second->len()); // total option length 5
+    EXPECT_EQ(0, memcmp(&x->second->getData()[0], v4Opts+2, 3)); // data len=3
+
+    x = options.find(13);
+    ASSERT_FALSE(x == options.end()); // option 1 should exist
+    EXPECT_EQ(13, x->second->getType());  // this should be option 13
+    ASSERT_EQ(3, x->second->getData().size()); // it should be of length 3
+    EXPECT_EQ(5, x->second->len()); // total option length 5
+    EXPECT_EQ(0, memcmp(&x->second->getData()[0], v4Opts+7, 3)); // data len=3
+
+    x = options.find(14);
+    ASSERT_FALSE(x == options.end()); // option 3 should exist
+    EXPECT_EQ(14, x->second->getType());  // this should be option 14
+    ASSERT_EQ(3, x->second->getData().size()); // it should be of length 3
+    EXPECT_EQ(5, x->second->len()); // total option length 5
+    EXPECT_EQ(0, memcmp(&x->second->getData()[0], v4Opts+12, 3)); // data len=3
+
+    x = options.find(254);
+    ASSERT_FALSE(x == options.end()); // option 3 should exist
+    EXPECT_EQ(254, x->second->getType());  // this should be option 254
+    ASSERT_EQ(3, x->second->getData().size()); // it should be of length 3
+    EXPECT_EQ(5, x->second->len()); // total option length 5
+    EXPECT_EQ(0, memcmp(&x->second->getData()[0], v4Opts+17, 3)); // data len=3
+
+    x = options.find(128);
+    ASSERT_FALSE(x == options.end()); // option 3 should exist
+    EXPECT_EQ(128, x->second->getType());  // this should be option 254
+    ASSERT_EQ(3, x->second->getData().size()); // it should be of length 3
+    EXPECT_EQ(5, x->second->len()); // total option length 5
+    EXPECT_EQ(0, memcmp(&x->second->getData()[0], v4Opts+22, 3)); // data len=3
+
+    x = options.find(0);
+    EXPECT_TRUE(x == options.end()); // option 0 not found
+
+    x = options.find(1);
+    EXPECT_TRUE(x == options.end()); // option 1 not found
+
+    x = options.find(2);
+    EXPECT_TRUE(x == options.end()); // option 2 not found
 }
 
 }
diff --git a/src/lib/dhcp/tests/option6_addrlst_unittest.cc b/src/lib/dhcp/tests/option6_addrlst_unittest.cc
index 2a2fc1a..60b618b 100644
--- a/src/lib/dhcp/tests/option6_addrlst_unittest.cc
+++ b/src/lib/dhcp/tests/option6_addrlst_unittest.cc
@@ -15,14 +15,12 @@
 #include <config.h>
 #include <iostream>
 #include <sstream>
-
 #include <arpa/inet.h>
 #include <gtest/gtest.h>
-
-#include "io_address.h"
-#include "dhcp/dhcp6.h"
-#include "dhcp/option.h"
-#include "dhcp/option6_addrlst.h"
+#include <asiolink/io_address.h>
+#include <dhcp/dhcp6.h>
+#include <dhcp/option.h>
+#include <dhcp/option6_addrlst.h>
 
 using namespace std;
 using namespace isc;
@@ -38,10 +36,10 @@ public:
 
 TEST_F(Option6AddrLstTest, basic) {
 
-    // limiting tests to just a 2001:db8::/32 as is *wrong*.
+    // Limiting tests to just a 2001:db8::/32 as is *wrong*.
     // Good tests check corner cases as well.
     // ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff checks
-    // for integer overflow
+    // for integer overflow.
     // ff02::face:b00c checks if multicast addresses
     // can be represented properly.
 
@@ -111,6 +109,8 @@ TEST_F(Option6AddrLstTest, basic) {
         opt1 = new Option6AddrLst(D6O_NAME_SERVERS, buf, 128, 0, 16);
     );
 
+    EXPECT_EQ(Option::V6, opt1->getUniverse());
+
     EXPECT_EQ(D6O_NAME_SERVERS, opt1->getType());
     EXPECT_EQ(20, opt1->len());
     Option6AddrLst::AddressContainer addrs = opt1->getAddresses();
@@ -178,6 +178,7 @@ TEST_F(Option6AddrLstTest, constructors) {
     EXPECT_NO_THROW(
         opt1 = new Option6AddrLst(1234, IOAddress("::1"));
     );
+    EXPECT_EQ(Option::V6, opt1->getUniverse());
     EXPECT_EQ(1234, opt1->getType());
 
     Option6AddrLst::AddressContainer addrs = opt1->getAddresses();
diff --git a/src/lib/dhcp/tests/option6_ia_unittest.cc b/src/lib/dhcp/tests/option6_ia_unittest.cc
index 91aaba4..3fd52f5 100644
--- a/src/lib/dhcp/tests/option6_ia_unittest.cc
+++ b/src/lib/dhcp/tests/option6_ia_unittest.cc
@@ -67,6 +67,7 @@ TEST_F(Option6IATest, basic) {
                                    0,
                                    12);
 
+    EXPECT_EQ(Option::V6, opt->getUniverse());
     EXPECT_EQ(D6O_IA_NA, opt->getType());
     EXPECT_EQ(0xa1a2a3a4, opt->getIAID());
     EXPECT_EQ(0x81020304, opt->getT1());
@@ -121,6 +122,7 @@ TEST_F(Option6IATest, simple) {
     ia->setT1(2345);
     ia->setT2(3456);
 
+    EXPECT_EQ(Option::V6, ia->getUniverse());
     EXPECT_EQ(D6O_IA_NA, ia->getType());
     EXPECT_EQ(1234, ia->getIAID());
     EXPECT_EQ(2345, ia->getT1());
@@ -251,7 +253,7 @@ TEST_F(Option6IATest, suboptions_unpack) {
     EXPECT_EQ(0xcafe, subopt->getType());
     EXPECT_EQ(4, subopt->len());
     // there should be no data at all
-    EXPECT_EQ(static_cast<void*>(NULL), subopt->getData());
+    EXPECT_EQ(0, subopt->getData().size());
 
     subopt = ia->getOption(1); // get option 1
     ASSERT_FALSE(subopt); // should be NULL
diff --git a/src/lib/dhcp/tests/option6_iaaddr_unittest.cc b/src/lib/dhcp/tests/option6_iaaddr_unittest.cc
index d1f7628..81c3eb3 100644
--- a/src/lib/dhcp/tests/option6_iaaddr_unittest.cc
+++ b/src/lib/dhcp/tests/option6_iaaddr_unittest.cc
@@ -75,6 +75,8 @@ TEST_F(Option6IAAddrTest, basic) {
 
     EXPECT_EQ(78, offset);
 
+    EXPECT_EQ(Option::V6, opt->getUniverse());
+
     // 4 bytes header + 4 bytes content
     EXPECT_EQ("2001:db8:1::dead:beef", opt->getAddress().toText());
     EXPECT_EQ(1000, opt->getPreferred());
diff --git a/src/lib/dhcp/tests/option_unittest.cc b/src/lib/dhcp/tests/option_unittest.cc
index 66464e2..e9de7de 100644
--- a/src/lib/dhcp/tests/option_unittest.cc
+++ b/src/lib/dhcp/tests/option_unittest.cc
@@ -19,6 +19,8 @@
 #include <arpa/inet.h>
 #include <gtest/gtest.h>
 #include <boost/shared_ptr.hpp>
+#include <exceptions/exceptions.h>
+#include <util/buffer.h>
 
 #include "dhcp/dhcp6.h"
 #include "dhcp/option.h"
@@ -26,6 +28,7 @@
 using namespace std;
 using namespace isc;
 using namespace isc::dhcp;
+using namespace isc::util;
 
 namespace {
 class OptionTest : public ::testing::Test {
@@ -37,30 +40,90 @@ public:
 // v4 is not really implemented yet. A simple test will do for now
 TEST_F(OptionTest, v4_basic) {
 
-    Option* opt = new Option(Option::V4, 17);
+    Option* opt = 0;
+    EXPECT_NO_THROW(
+        opt = new Option(Option::V4, 17);
+    );
 
+    EXPECT_EQ(Option::V4, opt->getUniverse());
     EXPECT_EQ(17, opt->getType());
-    EXPECT_EQ(static_cast<uint8_t*>(NULL), opt->getData());
+    EXPECT_EQ(0, opt->getData().size());
     EXPECT_EQ(2, opt->len()); // just v4 header
 
     EXPECT_NO_THROW(
         delete opt;
     );
+    opt = 0;
+
+    // V4 options have type 0...255
+    EXPECT_THROW(
+        opt = new Option(Option::V4, 256),
+        BadValue
+    );
+    if (opt) {
+        delete opt;
+        opt = 0;
+    }
 }
 
+const uint8_t dummyPayload[] =
+{ 1, 2, 3, 4};
+
 TEST_F(OptionTest, v4_data) {
-    // TODO
-    ASSERT_TRUE(false);
-}
 
-TEST_F(OptionTest, v4_addgetdel) {
-    // TODO
-    ASSERT_TRUE(false);
+    vector<uint8_t> data(dummyPayload, dummyPayload + sizeof(dummyPayload));
+
+    Option* opt = 0;
+
+    // create DHCPv4 option of type 123
+    // that contains 4 bytes of data
+    ASSERT_NO_THROW(
+        opt= new Option(Option::V4,
+                        123, // type
+                        data);
+    );
+
+    // check that content is reported properly
+    EXPECT_EQ(123, opt->getType());
+    vector<uint8_t> optData = opt->getData();
+    ASSERT_EQ(optData.size(), data.size());
+    EXPECT_EQ(optData, data);
+    EXPECT_EQ(2, opt->getHeaderLen());
+    EXPECT_EQ(6, opt->len());
+
+    // now store that option into a buffer
+    OutputBuffer buf(100);
+    EXPECT_NO_THROW(
+        opt->pack4(buf);
+    );
+
+    // check content of that buffer
+
+    // 2 byte header + 4 bytes data
+    ASSERT_EQ(6, buf.getLength());
+
+    // that's how this option is supposed to look like
+    uint8_t exp[] = { 123, 4, 1, 2, 3, 4 };
+
+    /// TODO: use vector<uint8_t> getData() when it will be implemented
+    EXPECT_EQ(0, memcmp(exp, buf.getData(), 6));
+
+    // check that we can destroy that option
+    EXPECT_NO_THROW(
+        delete opt;
+    );
 }
 
 TEST_F(OptionTest, v4_toText) {
-    // TODO
-    ASSERT_TRUE(false);
+
+    vector<uint8_t> buf(3);
+    buf[0] = 0;
+    buf[1] = 0xf;
+    buf[2] = 0xff;
+
+    Option opt(Option::V4, 253, buf);
+
+    EXPECT_EQ("type=253, len=3: 00:0f:ff", opt.toText());
 }
 
 // tests simple constructor
@@ -68,8 +131,9 @@ TEST_F(OptionTest, v6_basic) {
 
     Option* opt = new Option(Option::V6, 1);
 
+    EXPECT_EQ(Option::V6, opt->getUniverse());
     EXPECT_EQ(1, opt->getType());
-    EXPECT_EQ(static_cast<uint8_t*>(NULL), opt->getData());
+    EXPECT_EQ(0, opt->getData().size());
     EXPECT_EQ(4, opt->len()); // just v6 header
 
     EXPECT_NO_THROW(
@@ -88,9 +152,10 @@ TEST_F(OptionTest, v6_data1) {
                              3, // offset
                              7); // 7 bytes of data
     EXPECT_EQ(333, opt->getType());
-    ASSERT_EQ(&buf[3], opt->getData());
+
     ASSERT_EQ(11, opt->len());
-    EXPECT_EQ(0, memcmp(&buf[3], opt->getData(), 7) );
+    ASSERT_EQ(7, opt->getData().size());
+    EXPECT_EQ(0, memcmp(&buf[3], &opt->getData()[0], 7) );
 
     int offset = opt->pack(buf, 32, 20);
     EXPECT_EQ(31, offset);
diff --git a/src/lib/dhcp/tests/pkt4_unittest.cc b/src/lib/dhcp/tests/pkt4_unittest.cc
index 0d9bd41..1fe9947 100644
--- a/src/lib/dhcp/tests/pkt4_unittest.cc
+++ b/src/lib/dhcp/tests/pkt4_unittest.cc
@@ -20,16 +20,17 @@
 #include <boost/static_assert.hpp>
 #include <boost/shared_ptr.hpp>
 #include <boost/shared_array.hpp>
-
-#include "io_address.h"
-#include "dhcp/pkt4.h"
-#include "dhcp/dhcp4.h"
-#include "exceptions/exceptions.h"
+#include <util/buffer.h>
+#include <asiolink/io_address.h>
+#include <dhcp/pkt4.h>
+#include <dhcp/dhcp4.h>
+#include <exceptions/exceptions.h>
 
 using namespace std;
 using namespace isc;
 using namespace isc::asiolink;
 using namespace isc::dhcp;
+using namespace isc::util;
 using namespace boost;
 
 namespace {
@@ -433,19 +434,127 @@ TEST(Pkt4Test, file) {
 
 }
 
+static uint8_t v4Opts[] = {
+    12,  3, 0,   1,  2,
+    13,  3, 10, 11, 12,
+    14,  3, 20, 21, 22,
+    128, 3, 30, 31, 32,
+    254, 3, 40, 41, 42
+};
+
 TEST(Pkt4Test, options) {
-    // TODO
-    ASSERT_TRUE(false);
-}
+    Pkt4* pkt = new Pkt4(DHCPOFFER, 0);
+
+    vector<uint8_t> payload[5];
+    for (int i = 0; i < 5; i++) {
+        payload[i].resize(3);
+        payload[i][0] = i*10;
+        payload[i][1] = i*10+1;
+        payload[i][2] = i*10+2;
+    }
+
+    boost::shared_ptr<Option> opt1(new Option(Option::V4, 12, payload[0]));
+    boost::shared_ptr<Option> opt2(new Option(Option::V4, 13, payload[1]));
+    boost::shared_ptr<Option> opt3(new Option(Option::V4, 14, payload[2]));
+    boost::shared_ptr<Option> opt5(new Option(Option::V4,128, payload[3]));
+    boost::shared_ptr<Option> opt4(new Option(Option::V4,254, payload[4]));
+
+    pkt->addOption(opt1);
+    pkt->addOption(opt2);
+    pkt->addOption(opt3);
+    pkt->addOption(opt4);
+    pkt->addOption(opt5);
+
+    EXPECT_TRUE(pkt->getOption(12));
+    EXPECT_TRUE(pkt->getOption(13));
+    EXPECT_TRUE(pkt->getOption(14));
+    EXPECT_TRUE(pkt->getOption(128));
+    EXPECT_TRUE(pkt->getOption(254));
+    EXPECT_FALSE(pkt->getOption(127)); //  no such option
+
+    // options are unique in DHCPv4. It should not be possible
+    // to add more than one option of the same type.
+    EXPECT_THROW(
+        pkt->addOption(opt1),
+        BadValue
+    );
 
-TEST(Pkt4Test, packOptions) {
-    // TODO
-    ASSERT_TRUE(false);
+    EXPECT_NO_THROW(
+        pkt->pack();
+    );
+
+    OutputBuffer& buf = pkt->getBuffer();
+    // check that all options are stored, they should take sizeof(v4Opts)
+    ASSERT_EQ(static_cast<size_t>(Pkt4::DHCPV4_PKT_HDR_LEN) + sizeof(v4Opts),
+              buf.getLength());
+
+    // that that this extra data actually contain our options
+    const uint8_t* ptr = static_cast<const uint8_t*>(buf.getData());
+    ptr += Pkt4::DHCPV4_PKT_HDR_LEN; // rewind to end of fixed part
+    EXPECT_EQ(0, memcmp(ptr, v4Opts, sizeof(v4Opts)));
+
+    EXPECT_NO_THROW(
+        delete pkt;
+    );
 }
 
 TEST(Pkt4Test, unpackOptions) {
-    // TODO
-    ASSERT_TRUE(false);
+
+    vector<uint8_t> expectedFormat = generateTestPacket2();
+
+    for (int i=0; i < sizeof(v4Opts); i++) {
+        expectedFormat.push_back(v4Opts[i]);
+    }
+
+    // now expectedFormat contains fixed format and 5 options
+
+    shared_ptr<Pkt4> pkt(new Pkt4(&expectedFormat[0],
+                                  expectedFormat.size()));
+
+    EXPECT_NO_THROW(
+        pkt->unpack()
+    );
+
+    EXPECT_TRUE(pkt->getOption(12));
+    EXPECT_TRUE(pkt->getOption(13));
+    EXPECT_TRUE(pkt->getOption(14));
+    EXPECT_TRUE(pkt->getOption(128));
+    EXPECT_TRUE(pkt->getOption(254));
+
+    shared_ptr<Option> x = pkt->getOption(12);
+    ASSERT_TRUE(x); // option 1 should exist
+    EXPECT_EQ(12, x->getType());  // this should be option 12
+    ASSERT_EQ(3, x->getData().size()); // it should be of length 3
+    EXPECT_EQ(5, x->len()); // total option length 5
+    EXPECT_EQ(0, memcmp(&x->getData()[0], v4Opts+2, 3)); // data len=3
+
+    x = pkt->getOption(13);
+    ASSERT_TRUE(x); // option 13 should exist
+    EXPECT_EQ(13, x->getType());  // this should be option 13
+    ASSERT_EQ(3, x->getData().size()); // it should be of length 3
+    EXPECT_EQ(5, x->len()); // total option length 5
+    EXPECT_EQ(0, memcmp(&x->getData()[0], v4Opts+7, 3)); // data len=3
+
+    x = pkt->getOption(14);
+    ASSERT_TRUE(x); // option 14 should exist
+    EXPECT_EQ(14, x->getType());  // this should be option 14
+    ASSERT_EQ(3, x->getData().size()); // it should be of length 3
+    EXPECT_EQ(5, x->len()); // total option length 5
+    EXPECT_EQ(0, memcmp(&x->getData()[0], v4Opts+12, 3)); // data len=3
+
+    x = pkt->getOption(128);
+    ASSERT_TRUE(x); // option 3 should exist
+    EXPECT_EQ(128, x->getType());  // this should be option 254
+    ASSERT_EQ(3, x->getData().size()); // it should be of length 3
+    EXPECT_EQ(5, x->len()); // total option length 5
+    EXPECT_EQ(0, memcmp(&x->getData()[0], v4Opts+17, 3)); // data len=3
+
+    x = pkt->getOption(254);
+    ASSERT_TRUE(x); // option 3 should exist
+    EXPECT_EQ(254, x->getType());  // this should be option 254
+    ASSERT_EQ(3, x->getData().size()); // it should be of length 3
+    EXPECT_EQ(5, x->len()); // total option length 5
+    EXPECT_EQ(0, memcmp(&x->getData()[0], v4Opts+22, 3)); // data len=3
 }
 
 } // end of anonymous namespace
diff --git a/src/lib/dhcp/tests/pkt6_unittest.cc b/src/lib/dhcp/tests/pkt6_unittest.cc
index 0f110ba..968b24c 100644
--- a/src/lib/dhcp/tests/pkt6_unittest.cc
+++ b/src/lib/dhcp/tests/pkt6_unittest.cc
@@ -18,10 +18,10 @@
 #include <arpa/inet.h>
 #include <gtest/gtest.h>
 
-#include "io_address.h"
-#include "dhcp/option.h"
-#include "dhcp/pkt6.h"
-#include "dhcp/dhcp6.h"
+#include <asiolink/io_address.h>
+#include <dhcp/option.h>
+#include <dhcp/pkt6.h>
+#include <dhcp/dhcp6.h>
 
 using namespace std;
 using namespace isc;
diff --git a/src/lib/util/buffer.h b/src/lib/util/buffer.h
index b7a8e28..9e328a5 100644
--- a/src/lib/util/buffer.h
+++ b/src/lib/util/buffer.h
@@ -207,6 +207,28 @@ public:
     }
     //@}
 
+    /// @brief Read specified number of bytes as a vector.
+    ///
+    /// If specified buffer is too short, it will be expanded
+    /// using vector::resize() method.
+    ///
+    /// @param Reference to a buffer (data will be stored there).
+    /// @param Size specified number of bytes to read in a vector.
+    ///
+    void readVector(std::vector<uint8_t>& data, size_t len)
+    {
+        if (position_ + len > len_) {
+            isc_throw(InvalidBufferPosition, "read beyond end of buffer");
+        }
+
+        if (data.size() < len)
+            data.resize(len);
+
+        const uint8_t* ptr = &data_[position_];
+        data = std::vector<uint8_t>(ptr, ptr + len);
+        position_ += len;
+    }
+
 private:
     size_t position_;
 
@@ -519,6 +541,6 @@ typedef boost::shared_ptr<OutputBuffer> OutputBufferPtr;
 } // namespace isc
 #endif  // __BUFFER_H
 
-// Local Variables: 
+// Local Variables:
 // mode: c++
-// End: 
+// End:
diff --git a/src/lib/util/tests/buffer_unittest.cc b/src/lib/util/tests/buffer_unittest.cc
index 0cd1823..666924e 100644
--- a/src/lib/util/tests/buffer_unittest.cc
+++ b/src/lib/util/tests/buffer_unittest.cc
@@ -239,4 +239,36 @@ TEST_F(BufferTest, outputBufferZeroSize) {
     });
 }
 
+TEST_F(BufferTest, readVectorAll) {
+    std::vector<uint8_t> vec;
+
+    // check that vector can read the whole buffer
+    ibuffer.readVector(vec, 5);
+
+    ASSERT_EQ(5, vec.size());
+    EXPECT_EQ(0, memcmp(&vec[0], testdata, 5));
+
+    // ibuffer is 5 bytes long. Can't read past it.
+    EXPECT_THROW(
+        ibuffer.readVector(vec, 1),
+        isc::util::InvalidBufferPosition
+    );
+}
+
+TEST_F(BufferTest, readVectorChunks) {
+    std::vector<uint8_t> vec;
+
+    // check that vector can read the whole buffer
+    ibuffer.readVector(vec, 3);
+    EXPECT_EQ(3, vec.size());
+
+    EXPECT_EQ(0, memcmp(&vec[0], testdata, 3));
+
+    EXPECT_NO_THROW(
+        ibuffer.readVector(vec, 2)
+    );
+
+    EXPECT_EQ(0, memcmp(&vec[0], testdata+3, 2));
+}
+
 }




More information about the bind10-changes mailing list