BIND 10 trac1540, updated. 4006d91c9555c8dffab444aaa6e83f523ad5b338 [1540] Last set of review changes.

BIND 10 source code commits bind10-changes at lists.isc.org
Fri Feb 17 20:16:47 UTC 2012


The branch, trac1540 has been updated
       via  4006d91c9555c8dffab444aaa6e83f523ad5b338 (commit)
       via  526f8713fb6e13456f7008b5cd075f6fbdb89161 (commit)
      from  ec38722482e323b9e2674dd0331c074a4f1b9a61 (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 4006d91c9555c8dffab444aaa6e83f523ad5b338
Author: Tomek Mrugalski <tomasz at isc.org>
Date:   Fri Feb 17 21:16:25 2012 +0100

    [1540] Last set of review changes.
    
    - it is now not possible to register 0 and 255 options
    - all methods now use uint16_t for type
    - many smaller coding style clean-ups
    - added checks for option 0 and 255
    - added test for InputBuffer constructor

commit 526f8713fb6e13456f7008b5cd075f6fbdb89161
Author: Tomek Mrugalski <tomasz at isc.org>
Date:   Fri Feb 17 19:27:42 2012 +0100

    [1540] Changes after review:
    
    - port is now specified using uint16_t in IfaceMgr
    - support for proper DUID generation implemented
    - implemented new tests
    - new HWTYPEs defines
    - unpack methods now return size_t type
    - comments updated in option*.h headers

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

Summary of changes:
 src/bin/dhcp4/dhcp4_srv.cc                     |    6 +-
 src/bin/dhcp6/dhcp6_srv.cc                     |  124 ++++++++++++++++++-----
 src/bin/dhcp6/tests/dhcp6_srv_unittest.cc      |   82 +++++++++++++++-
 src/lib/dhcp/dhcp6.h                           |    9 ++
 src/lib/dhcp/iface_mgr.cc                      |  111 ++++++++-------------
 src/lib/dhcp/iface_mgr.h                       |   15 ++-
 src/lib/dhcp/libdhcp++.cc                      |   41 +++++---
 src/lib/dhcp/libdhcp++.h                       |    8 +-
 src/lib/dhcp/option.cc                         |   17 ++--
 src/lib/dhcp/option.h                          |   35 +++-----
 src/lib/dhcp/option6_addrlst.cc                |   10 +-
 src/lib/dhcp/option6_ia.cc                     |    4 +-
 src/lib/dhcp/option6_iaaddr.h                  |    5 +-
 src/lib/dhcp/pkt4.h                            |    8 +-
 src/lib/dhcp/pkt6.h                            |   30 +++---
 src/lib/dhcp/tests/iface_mgr_unittest.cc       |   26 +++---
 src/lib/dhcp/tests/libdhcp++_unittest.cc       |   18 ++--
 src/lib/dhcp/tests/option6_addrlst_unittest.cc |    6 +-
 src/lib/dhcp/tests/option6_ia_unittest.cc      |   13 +--
 src/lib/dhcp/tests/option6_iaaddr_unittest.cc  |    8 +-
 src/lib/dhcp/tests/option_unittest.cc          |   22 ++++
 src/lib/dhcp/tests/pkt6_unittest.cc            |   79 ++++++++--------
 src/lib/util/buffer.h                          |    2 +-
 src/lib/util/tests/buffer_unittest.cc          |   19 +++-
 24 files changed, 425 insertions(+), 273 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 735c39f..638ae05 100644
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -59,10 +59,10 @@ Dhcpv4Srv::~Dhcpv4Srv() {
 bool
 Dhcpv4Srv::run() {
     while (!shutdown_) {
-        Pkt4Ptr query; // client's message
-        Pkt4Ptr rsp;   // server's response
 
-        query = IfaceMgr::instance().receive4();
+        // client's message
+        Pkt4Ptr query = IfaceMgr::instance().receive4();
+        Pkt4Ptr rsp;   // server's response
 
         if (query) {
             try {
diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index b2bd5f5..a0e2b08 100644
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -12,6 +12,7 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <time.h>
 #include <dhcp/dhcp6.h>
 #include <dhcp/pkt6.h>
 #include <dhcp/iface_mgr.h>
@@ -21,11 +22,13 @@
 #include <dhcp/option6_addrlst.h>
 #include <asiolink/io_address.h>
 #include <exceptions/exceptions.h>
+#include <util/io_utilities.h>
 
 using namespace std;
 using namespace isc;
 using namespace isc::dhcp;
 using namespace isc::asiolink;
+using namespace isc::util;
 
 const std::string HARDCODED_LEASE = "2001:db8:1::1234:abcd";
 const uint32_t HARDCODED_T1 = 1500; // in seconds
@@ -67,13 +70,11 @@ Dhcpv6Srv::~Dhcpv6Srv() {
     IfaceMgr::instance().closeSockets();
 }
 
-bool
-Dhcpv6Srv::run() {
+bool Dhcpv6Srv::run() {
     while (!shutdown) {
-        Pkt6Ptr query; // client's message
-        Pkt6Ptr rsp;   // server's response
 
-        query = IfaceMgr::instance().receive6();
+        Pkt6Ptr query = IfaceMgr::instance().receive6(); // client's message
+        Pkt6Ptr rsp;   // server's response
 
         if (query) {
             if (!query->unpack()) {
@@ -138,24 +139,91 @@ Dhcpv6Srv::run() {
     return (true);
 }
 
-void
-Dhcpv6Srv::setServerID() {
-    /// TODO implement this for real once interface detection is done.
-    /// Use hardcoded server-id for now
-
-    OptionBuffer srvid(14);
-    srvid[0] = 0;
-    srvid[1] = 1; // DUID type 1 = DUID-LLT (see section 9.2 of RFC3315)
-    srvid[2] = 0;
-    srvid[3] = 6; // HW type = ethernet (I think. I'm typing this from my head
-                  // in hotel, without Internet connection)
-    for (int i=4; i<14; i++) {
-        srvid[i]=i-4;
+void Dhcpv6Srv::setServerID() {
+
+    /// @todo: DUID should be generated once and then stored, rather
+    /// than generated each time
+
+    /// @todo: This code implements support for DUID-LLT (the recommended one).
+    /// We should eventually add support for other DUID types: DUID-LL, DUID-EN
+    /// and DUID-UUID
+
+    const IfaceMgr::IfaceCollection& ifaces = IfaceMgr::instance().getIfaces();
+
+    // let's find suitable interface
+    for (IfaceMgr::IfaceCollection::const_iterator iface = ifaces.begin();
+         iface != ifaces.end(); ++iface) {
+        // all those conditions could be merged into one multi-condition
+        // statement, but let's keep them separated as perhaps one day
+        // we will grow knobs to selectively turn them on or off. Also,
+        // this code is used only *once* during first start on a new machine
+        // and then server-id is stored. (or at least it will be once
+        // DUID storage is implemente
+
+        // I wish there was a this_is_a_real_physical_interface flag...
+
+        // mac at least 6 bytes. All decent physical interfaces (Ethernet,
+        // WiFi, Infiniband, etc.) have 6 bytes long MAC address
+        if (iface->mac_len_ < 6) {
+            continue;
+        }
+
+        // let's don't use loopback
+        if (iface->flag_loopback_) {
+            continue;
+        }
+
+        // let's skip downed interfaces. It is better to use working ones.
+        if (!iface->flag_up_) {
+            continue;
+        }
+
+        uint8_t zeros[IfaceMgr::MAX_MAC_LEN];
+        memset(zeros, 0, IfaceMgr::MAX_MAC_LEN);
+
+        // some interfaces (like lo on Linux) report 6-bytes long
+        // MAC adress 00:00:00:00:00:00. Let's not use such weird interfaces
+        // to generate DUID.
+        if (!memcmp(iface->mac_, zeros, iface->mac_len_)) {
+            continue;
+        }
+
+        // Ok, we have useful MAC. Let's generate DUID-LLT based on
+        // it. See RFC3315, Section 9.2 for details.
+
+        // DUID uses seconds since midnight of 01-01-2000, time() returns
+        // seconds since 01-01-1970. DUID_TIME_EPOCH substution corrects that.
+        time_t seconds = time(NULL);
+        seconds -= DUID_TIME_EPOCH;
+
+        OptionBuffer srvid(8 + iface->mac_len_);
+        writeUint16(DUID_LLT, &srvid[0]);
+        writeUint16(HWTYPE_ETHERNET, &srvid[2]);
+        writeUint32(static_cast<uint32_t>(seconds), &srvid[4]);
+        memcpy(&srvid[0]+8, iface->mac_, iface->mac_len_);
+
+        serverid_ = OptionPtr(new Option(Option::V6, D6O_SERVERID,
+                                         srvid.begin(), srvid.end()));
+        return;
     }
-    serverid_ = boost::shared_ptr<Option>(new Option(Option::V6,
-                                                     D6O_SERVERID,
-                                                     srvid.begin(),
-                                                     srvid.end()));
+
+    // if we reached here, there are no suitable interfaces found.
+    // Either interface detection is not supported on this platform or
+    // this is really weird box. Let's use DUID-EN instead.
+    // See Section 9.3 of RFC3315 for details.
+
+    OptionBuffer srvid(12);
+    srand(time(NULL));
+    writeUint16(DUID_EN, &srvid[0]);
+    writeUint32(ENTERPRISE_ID_ISC, &srvid[2]);
+
+    // length of the identifier is company specific. I hereby declare
+    // ISC standard of 6 bytes long random numbers.
+    for (int i = 6; i < 12; i++) {
+        srvid[i] = static_cast<uint8_t>(rand());
+    }
+    serverid_ = OptionPtr(new Option(Option::V6, D6O_SERVERID,
+                                     srvid.begin(), srvid.end()));
 }
 
 void Dhcpv6Srv::copyDefaultOptions(const Pkt6Ptr& question, Pkt6Ptr& answer) {
@@ -214,7 +282,6 @@ void Dhcpv6Srv::assignLeases(const Pkt6Ptr& question, Pkt6Ptr& answer) {
 }
 
 Pkt6Ptr Dhcpv6Srv::processSolicit(const Pkt6Ptr& solicit) {
-
     Pkt6Ptr advertise(new Pkt6(DHCPV6_ADVERTISE, solicit->getTransid()));
 
     copyDefaultOptions(solicit, advertise);
@@ -227,7 +294,6 @@ Pkt6Ptr Dhcpv6Srv::processSolicit(const Pkt6Ptr& solicit) {
 }
 
 Pkt6Ptr Dhcpv6Srv::processRequest(const Pkt6Ptr& request) {
-
     Pkt6Ptr reply(new Pkt6(DHCPV6_REPLY, request->getTransid()));
 
     copyDefaultOptions(request, reply);
@@ -240,33 +306,37 @@ Pkt6Ptr Dhcpv6Srv::processRequest(const Pkt6Ptr& request) {
 }
 
 Pkt6Ptr Dhcpv6Srv::processRenew(const Pkt6Ptr& renew) {
+    /// @todo: Implement this
     Pkt6Ptr reply(new Pkt6(DHCPV6_REPLY, renew->getTransid()));
     return reply;
 }
 
 Pkt6Ptr Dhcpv6Srv::processRebind(const Pkt6Ptr& rebind) {
-    Pkt6Ptr reply(new Pkt6(DHCPV6_REPLY,
-                                           rebind->getTransid(),
-                                           Pkt6::UDP));
+    /// @todo: Implement this
+    Pkt6Ptr reply(new Pkt6(DHCPV6_REPLY, rebind->getTransid()));
     return reply;
 }
 
 Pkt6Ptr Dhcpv6Srv::processConfirm(const Pkt6Ptr& confirm) {
+    /// @todo: Implement this
     Pkt6Ptr reply(new Pkt6(DHCPV6_REPLY, confirm->getTransid()));
     return reply;
 }
 
 Pkt6Ptr Dhcpv6Srv::processRelease(const Pkt6Ptr& release) {
+    /// @todo: Implement this
     Pkt6Ptr reply(new Pkt6(DHCPV6_REPLY, release->getTransid()));
     return reply;
 }
 
 Pkt6Ptr Dhcpv6Srv::processDecline(const Pkt6Ptr& decline) {
+    /// @todo: Implement this
     Pkt6Ptr reply(new Pkt6(DHCPV6_REPLY, decline->getTransid()));
     return reply;
 }
 
 Pkt6Ptr Dhcpv6Srv::processInfRequest(const Pkt6Ptr& infRequest) {
+    /// @todo: Implement this
     Pkt6Ptr reply(new Pkt6(DHCPV6_REPLY, infRequest->getTransid()));
     return reply;
 }
diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
index 33267a1..eb351ac 100644
--- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
+++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
@@ -20,13 +20,15 @@
 #include <arpa/inet.h>
 #include <gtest/gtest.h>
 
-#include "dhcp/dhcp6.h"
-#include "dhcp6/dhcp6_srv.h"
-#include "dhcp/option6_ia.h"
+#include <dhcp/dhcp6.h>
+#include <dhcp/option6_ia.h>
+#include <dhcp6/dhcp6_srv.h>
+#include <util/buffer.h>
 
 using namespace std;
 using namespace isc;
 using namespace isc::dhcp;
+using namespace isc::util;
 
 // namespace has to be named, because friends are defined in Dhcpv6Srv class
 // Maybe it should be isc::test?
@@ -79,16 +81,84 @@ TEST_F(Dhcpv6SrvTest, basic) {
     delete srv;
 }
 
+TEST_F(Dhcpv6SrvTest, DUID) {
+    // tests that DUID is generated properly
+
+    Dhcpv6Srv* srv = NULL;
+    ASSERT_NO_THROW( {
+        srv = new Dhcpv6Srv(DHCP6_SERVER_PORT + 10000);
+    });
+
+    OptionPtr srvid = srv->getServerID();
+    ASSERT_TRUE(srvid);
+
+    EXPECT_EQ(D6O_SERVERID, srvid->getType());
+
+    OutputBuffer buf(32);
+    srvid->pack(buf);
+
+    // length of the actual DUID
+    size_t len = buf.getLength() - srvid->getHeaderLen();
+
+    InputBuffer data(buf.getData(), buf.getLength());
+
+    // ignore first four bytes (standard DHCPv6 header)
+    data.readUint32();
+
+    uint16_t duid_type = data.readUint16();
+    cout << "Duid-type=" << duid_type << endl;
+    switch(duid_type) {
+    case DUID_LLT: {
+        // DUID must contain at least 6 bytes long MAC
+        // + 8 bytes of fixed header
+        EXPECT_GE(14, len);
+
+        uint16_t hw_type = data.readUint16();
+        // there's no real way to find out "correct"
+        // hardware type
+        EXPECT_GT(hw_type, 0);
+
+        // check that timer is counted since 1.1.2000,
+        // not from 1.1.1970.
+        uint32_t seconds = data.readUint32();
+        EXPECT_LE(seconds, DUID_TIME_EPOCH);
+        // this test will start failing after 2030.
+        // Hopefully we will be at BIND12 by then.
+
+        // MAC must not be zeros
+        vector<uint8_t> mac(len-8);
+        vector<uint8_t> zeros(len-8, 0);
+        data.readVector(mac, len-8);
+        EXPECT_NE(mac, zeros);
+        break;
+    }
+    case DUID_EN: {
+        // there's not much we can check. Just simple
+        // check if it is not all zeros
+        vector<uint8_t> content(len-2);
+        vector<uint8_t> zeros(0, len-2);
+        data.readVector(content, len-2);
+        EXPECT_NE(content, zeros);
+    }
+    case DUID_LL: // not supported yet
+    case DUID_UUID: // not supported yet
+    default:
+        cout << "Not supported duid type=" << duid_type << endl;
+        FAIL();
+    }
+}
+
 TEST_F(Dhcpv6SrvTest, Solicit_basic) {
     NakedDhcpv6Srv* srv = NULL;
     ASSERT_NO_THROW( srv = new NakedDhcpv6Srv(); );
 
     // a dummy content for client-id
     OptionBuffer clntDuid(32);
-    for (int i = 0; i < 32; i++)
+    for (int i = 0; i < 32; i++) {
         clntDuid[i] = 100 + i;
+    }
 
-    Pkt6Ptr sol = Pkt6Ptr(new Pkt6(DHCPV6_SOLICIT, 1234, Pkt6::UDP));
+    Pkt6Ptr sol = Pkt6Ptr(new Pkt6(DHCPV6_SOLICIT, 1234));
 
     boost::shared_ptr<Option6IA> ia =
         boost::shared_ptr<Option6IA>(new Option6IA(D6O_IA_NA, 234));
@@ -113,7 +183,7 @@ TEST_F(Dhcpv6SrvTest, Solicit_basic) {
 
     OptionPtr clientid = OptionPtr(new Option(Option::V6, D6O_CLIENTID,
                                               clntDuid.begin(),
-                                              clntDuid.begin()+16));
+                                              clntDuid.begin() + 16));
     sol->addOption(clientid);
 
     boost::shared_ptr<Pkt6> reply = srv->processSolicit(sol);
diff --git a/src/lib/dhcp/dhcp6.h b/src/lib/dhcp/dhcp6.h
index 6012003..15c306d 100644
--- a/src/lib/dhcp/dhcp6.h
+++ b/src/lib/dhcp/dhcp6.h
@@ -108,6 +108,15 @@ extern const int dhcpv6_type_name_max;
 #define DUID_LLT        1
 #define DUID_EN         2
 #define DUID_LL         3
+#define DUID_UUID       4
+
+// Define hardware types
+// Taken from http://www.iana.org/assignments/arp-parameters/
+#define HWTYPE_ETHERNET    0x0001
+#define HWTYPE_INIFINIBAND 0x0020
+
+// Taken from http://www.iana.org/assignments/enterprise-numbers
+#define ENTERPRISE_ID_ISC 2495
 
 /* Offsets into IA_*'s where Option spaces commence.  */
 #define IA_NA_OFFSET 12 /* IAID, T1, T2, all 4 octets each */
diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc
index f0df462..e328cda 100644
--- a/src/lib/dhcp/iface_mgr.cc
+++ b/src/lib/dhcp/iface_mgr.cc
@@ -178,6 +178,12 @@ IfaceMgr::stubDetectIfaces() {
         cout << "Detected interface " << ifaceName << "/" << linkLocal << endl;
 
         Iface iface(ifaceName, if_nametoindex( ifaceName.c_str() ) );
+        iface.flag_up_ = true;
+        iface.flag_running_ = true;
+        iface.flag_loopback_ = false;
+        iface.flag_multicast_ = true;
+        iface.flag_broadcast_ = true;
+        iface.hardware_type_ = HWTYPE_ETHERNET;
         IOAddress addr(linkLocal);
         iface.addAddress(addr);
         addInterface(iface);
@@ -347,9 +353,7 @@ IfaceMgr::getIface(const std::string& ifname) {
     return (NULL); // not found
 }
 
-int IfaceMgr::openSocket(const std::string& ifname,
-                     const IOAddress& addr,
-                     int port) {
+int IfaceMgr::openSocket(const std::string& ifname, const IOAddress& addr, uint16_t port) {
     Iface* iface = getIface(ifname);
     if (!iface) {
         isc_throw(BadValue, "There is no " << ifname << " interface present.");
@@ -365,7 +369,7 @@ int IfaceMgr::openSocket(const std::string& ifname,
     }
 }
 
-int IfaceMgr::openSocket4(Iface& iface, const IOAddress& addr, int port) {
+int IfaceMgr::openSocket4(Iface& iface, const IOAddress& addr, uint16_t port) {
 
     cout << "Creating UDP4 socket on " << iface.getFullName()
          << " " << addr.toText() << "/port=" << port << endl;
@@ -409,7 +413,7 @@ int IfaceMgr::openSocket4(Iface& iface, const IOAddress& addr, int port) {
     return (sock);
 }
 
-int IfaceMgr::openSocket6(Iface& iface, const IOAddress& addr, int port) {
+int IfaceMgr::openSocket6(Iface& iface, const IOAddress& addr, uint16_t port) {
 
     cout << "Creating UDP6 socket on " << iface.getFullName()
          << " " << addr.toText() << "/port=" << port << endl;
@@ -515,7 +519,7 @@ const std::string & mcast) {
 }
 
 bool
-IfaceMgr::send(boost::shared_ptr<Pkt6>& pkt) {
+IfaceMgr::send(const Pkt6Ptr& pkt) {
     struct msghdr m;
     struct iovec v;
     int result;
@@ -549,7 +553,7 @@ IfaceMgr::send(boost::shared_ptr<Pkt6>& pkt) {
     // Set the data buffer we're sending. (Using this wacky
     // "scatter-gather" stuff... we only have a single chunk
     // of data to send, so we declare a single vector entry.)
-    v.iov_base = (char *) pkt->getBuffer().getData();
+    v.iov_base = const_cast<void *>(pkt->getBuffer().getData());
     v.iov_len = pkt->getBuffer().getLength();
     m.msg_iov = &v;
     m.msg_iovlen = 1;
@@ -585,11 +589,10 @@ IfaceMgr::send(boost::shared_ptr<Pkt6>& pkt) {
 }
 
 bool
-IfaceMgr::send(boost::shared_ptr<Pkt4>& pkt)
+IfaceMgr::send(const Pkt4Ptr& pkt)
 {
     struct msghdr m;
     struct iovec v;
-    int result;
 
     Iface* iface = getIface(pkt->getIface());
     if (!iface) {
@@ -620,7 +623,6 @@ IfaceMgr::send(boost::shared_ptr<Pkt4>& pkt)
     m.msg_iov = &v;
     m.msg_iovlen = 1;
 
-// OS_LINUX defines are part of ticket #1237
 #if defined(OS_LINUX)
     // Setting the interface is a bit more involved.
     //
@@ -628,16 +630,14 @@ IfaceMgr::send(boost::shared_ptr<Pkt4>& pkt)
     // define the IPv4 packet information. We could set the
     // source address if we wanted, but we can safely let the
     // kernel decide what that should be.
-    struct in_pktinfo *pktinfo;
-    struct cmsghdr *cmsg;
     m.msg_control = &control_buf_[0];
     m.msg_controllen = control_buf_len_;
-    cmsg = CMSG_FIRSTHDR(&m);
+    struct cmsghdr* cmsg = CMSG_FIRSTHDR(&m);
     cmsg->cmsg_level = IPPROTO_IP;
     cmsg->cmsg_type = IP_PKTINFO;
-    cmsg->cmsg_len = CMSG_LEN(sizeof(*pktinfo));
-    pktinfo = (struct in_pktinfo *)CMSG_DATA(cmsg);
-    memset(pktinfo, 0, sizeof(*pktinfo));
+    cmsg->cmsg_len = CMSG_LEN(sizeof(struct in_pktinfo));
+    struct in_pktinfo* pktinfo =(struct in_pktinfo *)CMSG_DATA(cmsg);
+    memset(pktinfo, 0, sizeof(struct in_pktinfo));
     pktinfo->ipi_ifindex = pkt->getIndex();
     m.msg_controllen = cmsg->cmsg_len;
 #endif
@@ -647,7 +647,7 @@ IfaceMgr::send(boost::shared_ptr<Pkt4>& pkt)
          << " over socket " << getSocket(*pkt) << " on interface "
          << getIface(pkt->getIface())->getFullName() << endl;
 
-    result = sendmsg(getSocket(*pkt), &m, 0);
+        int result = sendmsg(getSocket(*pkt), &m, 0);
     if (result < 0) {
         isc_throw(Unexpected, "Pkt4 send failed.");
     }
@@ -669,22 +669,12 @@ IfaceMgr::receive4() {
     IfaceCollection::const_iterator iface;
     for (iface = ifaces_.begin(); iface != ifaces_.end(); ++iface) {
 
-#if 0
-        // uncomment this once #1237 is merged
-        // Let's skip loopback and downed interfaces.
-        if (iface->flag_loopback_ ||
-            !iface->flag_up_ ||
-            !iface->flag_running_) {
-            continue;
-        }
-#endif
-
-        SocketCollection::const_iterator s = iface->sockets_.begin();
-        while (s != iface->sockets_.end()) {
+        /// @todo: rewrite this as part of #1555
+        for (SocketCollection::const_iterator s = iface->sockets_.begin();
+             s != iface->sockets_.end(); ++s) {
 
             // We don't want IPv6 addresses here.
             if (s->addr_.getFamily() != AF_INET) {
-                ++s;
                 continue;
             }
 
@@ -693,8 +683,6 @@ IfaceMgr::receive4() {
                 candidate = &(*s);
                 break;
             }
-
-            ++s;
         }
 
         if (candidate) {
@@ -711,13 +699,8 @@ IfaceMgr::receive4() {
          << iface->getFullName() << endl;
 
     // Now we have a socket, let's get some data from it!
-
-    struct msghdr m;
-    struct iovec v;
-    int result;
     struct sockaddr_in from_addr;
     struct in_addr to_addr;
-    boost::shared_ptr<Pkt4> pkt;
     const uint32_t RCVBUFSIZE = 1500;
     static uint8_t buf[RCVBUFSIZE];
 
@@ -726,13 +709,15 @@ IfaceMgr::receive4() {
     memset(&to_addr, 0, sizeof(to_addr));
 
     // Initialize our message header structure.
+    struct msghdr m;
     memset(&m, 0, sizeof(m));
 
     // Point so we can get the from address.
     m.msg_name = &from_addr;
     m.msg_namelen = sizeof(from_addr);
 
-    v.iov_base = (void*)buf;
+    struct iovec v;
+    v.iov_base = static_cast<void*>(buf);
     v.iov_len = RCVBUFSIZE;
     m.msg_iov = &v;
     m.msg_iovlen = 1;
@@ -746,16 +731,14 @@ IfaceMgr::receive4() {
     m.msg_control = &control_buf_[0];
     m.msg_controllen = control_buf_len_;
 
-    result = recvmsg(candidate->sockfd_, &m, 0);
-
+    int result = recvmsg(candidate->sockfd_, &m, 0);
     if (result < 0) {
         cout << "Failed to receive UDP4 data." << endl;
-        return (boost::shared_ptr<Pkt4>()); // NULL
+        return (Pkt4Ptr()); // NULL
     }
 
     unsigned int ifindex = iface->getIndex();
 
-// OS_LINUX defines are part of ticket #1237
 #if defined(OS_LINUX)
     struct cmsghdr* cmsg;
     struct in_pktinfo* pktinfo;
@@ -800,7 +783,7 @@ IfaceMgr::receive4() {
          << iface->getFullName() << endl;
 
     // we have all data let's create Pkt4 object
-    pkt = boost::shared_ptr<Pkt4>(new Pkt4(buf, result));
+    Pkt4Ptr pkt = Pkt4Ptr(new Pkt4(buf, result));
 
     pkt->setIface(iface->getName());
     pkt->setIndex(ifindex);
@@ -820,7 +803,7 @@ Pkt6Ptr IfaceMgr::receive6() {
     struct in6_pktinfo* pktinfo;
     struct sockaddr_in6 from;
     struct in6_addr to_addr;
-    int ifindex;
+    int ifindex = -1;
     Pkt6Ptr pkt;
 
     // RFC3315 states that server responses may be
@@ -866,10 +849,9 @@ Pkt6Ptr IfaceMgr::receive6() {
     IfaceCollection::const_iterator iface = ifaces_.begin();
     const SocketInfo* candidate = 0;
     while (iface != ifaces_.end()) {
-        SocketCollection::const_iterator s = iface->sockets_.begin();
-        while (s != iface->sockets_.end()) {
+        for (SocketCollection::const_iterator s = iface->sockets_.begin();
+             s != iface->sockets_.end(); ++s) {
             if (s->addr_.getFamily() != AF_INET6) {
-                ++s;
                 continue;
             }
             if (s->addr_.getAddress().to_v6().is_multicast()) {
@@ -879,7 +861,6 @@ Pkt6Ptr IfaceMgr::receive6() {
             if (!candidate) {
                 candidate = &(*s); // it's not multicast, but it's better than nothing
             }
-            ++s;
         }
         if (candidate) {
             break;
@@ -887,7 +868,7 @@ Pkt6Ptr IfaceMgr::receive6() {
         ++iface;
     }
     if (iface == ifaces_.end()) {
-        isc_throw(Unexpected, "No interfaces detected. Can't receive anything.");
+        isc_throw(Unexpected, "No suitable IPv6 interfaces detected. Can't receive anything.");
     }
 
     if (!candidate) {
@@ -921,11 +902,11 @@ Pkt6Ptr IfaceMgr::receive6() {
         }
         if (!found_pktinfo) {
             cout << "Unable to find pktinfo" << endl;
-            return (boost::shared_ptr<Pkt6>()); // NULL
+            return (Pkt6Ptr()); // NULL
         }
     } else {
         cout << "Failed to receive data." << endl;
-        return (boost::shared_ptr<Pkt6>()); // NULL
+        return (Pkt6Ptr()); // NULL
     }
 
 
@@ -933,11 +914,11 @@ Pkt6Ptr IfaceMgr::receive6() {
         pkt = Pkt6Ptr(new Pkt6(buf, result));
     } catch (const std::exception& ex) {
         cout << "Failed to create new packet." << endl;
-        return (boost::shared_ptr<Pkt6>()); // NULL
+        return (Pkt6Ptr()); // NULL
     }
 
-    pkt->setLocalAddr(IOAddress::from_bytes(AF_INET6, (const uint8_t*)&to_addr));
-    pkt->setRemoteAddr(IOAddress::from_bytes(AF_INET6, (const uint8_t*)&from.sin6_addr));
+    pkt->setLocalAddr(IOAddress::from_bytes(AF_INET6, reinterpret_cast<const uint8_t*>(&to_addr)));
+    pkt->setRemoteAddr(IOAddress::from_bytes(AF_INET6, reinterpret_cast<const uint8_t*>(&from.sin6_addr)));
 
     pkt->setRemotePort(ntohs(from.sin6_port));
 
@@ -962,7 +943,7 @@ Pkt6Ptr IfaceMgr::receive6() {
     return (pkt);
 }
 
-uint16_t IfaceMgr::getSocket(isc::dhcp::Pkt6 const& pkt) {
+uint16_t IfaceMgr::getSocket(const isc::dhcp::Pkt6& pkt) {
     Iface* iface = getIface(pkt.getIface());
     if (!iface) {
         isc_throw(BadValue, "Tried to find socket for non-existent interface "
@@ -971,19 +952,13 @@ uint16_t IfaceMgr::getSocket(isc::dhcp::Pkt6 const& pkt) {
 
     SocketCollection::const_iterator s;
     for (s = iface->sockets_.begin(); s != iface->sockets_.end(); ++s) {
-        if (s->family_ != AF_INET6) {
-            // don't use IPv4 sockets
-            continue;
-        }
-        if (s->addr_.getAddress().to_v6().is_multicast()) {
-            // don't use IPv6 sockets bound to multicast address
-            continue;
+        if ( (s->family_ == AF_INET6) &&
+             (!s->addr_.getAddress().to_v6().is_multicast()) ) {
+            return (s->sockfd_);
         }
         /// TODO: Add more checks here later. If remote address is
         /// not link-local, we can't use link local bound socket
         /// to send data.
-
-        return (s->sockfd_);
     }
 
     isc_throw(Unexpected, "Interface " << iface->getFullName()
@@ -999,16 +974,12 @@ uint16_t IfaceMgr::getSocket(isc::dhcp::Pkt4 const& pkt) {
 
     SocketCollection::const_iterator s;
     for (s = iface->sockets_.begin(); s != iface->sockets_.end(); ++s) {
-        if (s->family_ != AF_INET) {
-            // don't use IPv6 sockets
-            continue;
+        if (s->family_ == AF_INET) {
+            return (s->sockfd_);
         }
-
         /// TODO: Add more checks here later. If remote address is
         /// not link-local, we can't use link local bound socket
         /// to send data.
-
-        return (s->sockfd_);
     }
 
     isc_throw(Unexpected, "Interface " << iface->getFullName()
diff --git a/src/lib/dhcp/iface_mgr.h b/src/lib/dhcp/iface_mgr.h
index 93e6b98..bb3a73a 100644
--- a/src/lib/dhcp/iface_mgr.h
+++ b/src/lib/dhcp/iface_mgr.h
@@ -230,6 +230,11 @@ public:
     Iface*
     getIface(const std::string& ifname);
 
+    /// @brief Returns container with all interfaces.
+    ///
+    /// @return container with all interfaces.
+    const IfaceCollection& getIfaces() { return ifaces_; }
+
     /// @brief Return most suitable socket for transmitting specified IPv6 packet.
     ///
     /// This method takes Pkt6 (see overloaded implementation that takes
@@ -271,7 +276,7 @@ public:
     /// @param pkt packet to be sent
     ///
     /// @return true if sending was successful
-    bool send(boost::shared_ptr<Pkt6>& pkt);
+    bool send(const Pkt6Ptr& pkt);
 
     /// @brief Sends an IPv4 packet.
     ///
@@ -282,7 +287,7 @@ public:
     /// @param pkt a packet to be sent
     ///
     /// @return true if sending was successful
-    bool send(boost::shared_ptr<Pkt4>& pkt);
+    bool send(const Pkt4Ptr& pkt);
 
     /// @brief Tries to receive IPv6 packet over open IPv6 sockets.
     ///
@@ -325,7 +330,7 @@ public:
     /// @return socket descriptor, if socket creation, binding and multicast
     /// group join were all successful.
     int openSocket(const std::string& ifname,
-                   const isc::asiolink::IOAddress& addr, int port);
+                   const isc::asiolink::IOAddress& addr, uint16_t port);
 
     /// Opens IPv6 sockets on detected interfaces.
     ///
@@ -375,7 +380,7 @@ protected:
     /// @param port a port that created socket should be bound to
     ///
     /// @return socket descriptor
-    int openSocket4(Iface& iface, const isc::asiolink::IOAddress& addr, int port);
+    int openSocket4(Iface& iface, const isc::asiolink::IOAddress& addr, uint16_t port);
 
     /// @brief Opens IPv6 socket.
     ///
@@ -388,7 +393,7 @@ protected:
     /// @param port a port that created socket should be bound to
     ///
     /// @return socket descriptor
-    int openSocket6(Iface& iface, const isc::asiolink::IOAddress& addr, int port);
+    int openSocket6(Iface& iface, const isc::asiolink::IOAddress& addr, uint16_t port);
 
     /// @brief Adds an interface to list of known interfaces.
     ///
diff --git a/src/lib/dhcp/libdhcp++.cc b/src/lib/dhcp/libdhcp++.cc
index 9a4f4de..23d1072 100644
--- a/src/lib/dhcp/libdhcp++.cc
+++ b/src/lib/dhcp/libdhcp++.cc
@@ -34,10 +34,10 @@ std::map<unsigned short, Option::Factory*> LibDHCP::v4factories_;
 std::map<unsigned short, Option::Factory*> LibDHCP::v6factories_;
 
 
-uint32_t LibDHCP::unpackOptions6(const OptionBuffer& buf,
-                                 isc::dhcp::Option::OptionCollection& options) {
-    uint32_t offset = 0;
-    unsigned int end = buf.size();
+size_t LibDHCP::unpackOptions6(const OptionBuffer& buf,
+                               isc::dhcp::Option::OptionCollection& options) {
+    size_t offset = 0;
+    size_t end = buf.size();
 
     while (offset +4 <= end) {
         uint16_t opt_type = buf[offset]*256 + buf[offset+1];
@@ -45,7 +45,7 @@ uint32_t LibDHCP::unpackOptions6(const OptionBuffer& buf,
         uint16_t opt_len = buf[offset]*256 + buf[offset+1];
         offset += 2;
 
-        if (offset + opt_len > end ) {
+        if (offset + opt_len > end) {
             cout << "Option " << opt_type << " truncated." << endl;
             return (offset);
         }
@@ -73,31 +73,37 @@ uint32_t LibDHCP::unpackOptions6(const OptionBuffer& buf,
             break;
         }
         // add option to options
-        options.insert(pair<int, boost::shared_ptr<Option> >(opt_type, opt));
+        options.insert(std::make_pair(opt_type, opt));
         offset += opt_len;
     }
 
     return (offset);
 }
 
-uint32_t LibDHCP::unpackOptions4(const OptionBuffer& buf,
+size_t LibDHCP::unpackOptions4(const OptionBuffer& buf,
                                  isc::dhcp::Option::OptionCollection& options) {
     size_t offset = 0;
 
-    // 2 - header of DHCPv4 option
+    // 2 byte - header of DHCPv4 option
     while (offset + 1 <= buf.size()) {
         uint8_t opt_type = buf[offset++];
 
+        // DHO_END is a special, one octet long option
         if (opt_type == DHO_END)
             return (offset); // just return. Don't need to add DHO_END option
 
+        // DHO_PAD is just a padding after DHO_END. Let's continue parsing
+        // in case we receive a message without DHO_END.
+        if (opt_type == DHO_PAD)
+            continue;
+
         if (offset + 1 >= buf.size()) {
             isc_throw(OutOfRange, "Attempt to parse truncated option "
                       << opt_type);
         }
 
         uint8_t opt_len =  buf[offset++];
-        if (offset + opt_len > buf.size() ) {
+        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.");
@@ -111,7 +117,7 @@ uint32_t LibDHCP::unpackOptions4(const OptionBuffer& buf,
                                        buf.begin()+offset+opt_len));
         }
 
-        options.insert(pair<int, boost::shared_ptr<Option> >(opt_type, opt));
+        options.insert(std::make_pair(opt_type, opt));
         offset += opt_len;
     }
     return (offset);
@@ -135,18 +141,17 @@ void
 LibDHCP::packOptions(isc::util::OutputBuffer& buf,
                      const Option::OptionCollection& options) {
     for (Option::OptionCollection::const_iterator it = options.begin();
-         it != options.end();
-         ++it) {
+         it != options.end(); ++it) {
         it->second->pack4(buf);
     }
 }
 
 void LibDHCP::OptionFactoryRegister(Option::Universe u,
                                     uint16_t opt_type,
-                                    Option::Factory * factory) {
+                                    Option::Factory* factory) {
     switch (u) {
     case Option::V6: {
-        if (v6factories_.find(opt_type)!=v6factories_.end()) {
+        if (v6factories_.find(opt_type) != v6factories_.end()) {
             isc_throw(BadValue, "There is already DHCPv6 factory registered "
                      << "for option type "  << opt_type);
         }
@@ -155,6 +160,14 @@ void LibDHCP::OptionFactoryRegister(Option::Universe u,
     }
     case Option::V4:
     {
+        // option 0 is special (a one octet-long, equal 0) PAD option. It is never
+        // instantiated as Option object, but rather consumer during packet parsing.
+        if (opt_type == 0) {
+            isc_throw(BadValue, "Cannot redefine PAD option (code=0)");
+        }
+        // option 255 is never instantiated as an option object. It is special
+        // (a one-octet equal 255) option that is added at the end of all options
+        // during packet assembly. It is also silently consumed during packet parsing.
         if (opt_type > 254) {
             isc_throw(BadValue, "Too big option type for DHCPv4, only 0-254 allowed.");
         }
diff --git a/src/lib/dhcp/libdhcp++.h b/src/lib/dhcp/libdhcp++.h
index 658a893..c7935c8 100644
--- a/src/lib/dhcp/libdhcp++.h
+++ b/src/lib/dhcp/libdhcp++.h
@@ -57,8 +57,8 @@ public:
     /// @param buf Buffer to be parsed.
     /// @param options Reference to option container. Options will be
     ///        put here.
-    static uint32_t unpackOptions4(const OptionBuffer& buf,
-                                   isc::dhcp::Option::OptionCollection& options);
+    static size_t unpackOptions4(const OptionBuffer& buf,
+                                 isc::dhcp::Option::OptionCollection& options);
 
     /// @brief Parses provided buffer as DHCPv6 options and creates Option objects.
     ///
@@ -68,8 +68,8 @@ public:
     /// @param buf Buffer to be parsed.
     /// @param options Reference to option container. Options will be
     ///        put here.
-    static uint32_t unpackOptions6(const OptionBuffer& buf,
-                                   isc::dhcp::Option::OptionCollection& options);
+    static size_t unpackOptions6(const OptionBuffer& buf,
+                                 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 6af32a0..78fd372 100644
--- a/src/lib/dhcp/option.cc
+++ b/src/lib/dhcp/option.cc
@@ -32,9 +32,10 @@ namespace dhcp {
 Option::Option(Universe u, uint16_t type)
     :universe_(u), type_(type) {
 
-    if ((u == V4) && (type > 255)) {
+    // END option (type 255 is forbidden as well)
+    if ((u == V4) && ((type == 0) || (type > 254))) {
         isc_throw(BadValue, "Can't create V4 option of type "
-                  << type << ", V4 options are in range 0..255");
+                  << type << ", V4 options are in range 1..254");
     }
 }
 
@@ -76,9 +77,9 @@ Option::check() {
 void Option::pack(isc::util::OutputBuffer& buf) {
     switch (universe_) {
     case V6:
-        return pack6(buf);
+        return (pack6(buf));
     case V4:
-        return pack4(buf);
+        return (pack4(buf));
     default:
         isc_throw(BadValue, "Failed to pack " << type_ << " option. Do not "
                   << "use this method for options other than DHCPv6.");
@@ -89,7 +90,7 @@ void
 Option::pack4(isc::util::OutputBuffer& buf) {
     switch (universe_) {
     case V4: {
-        if (data_.size() > 255) {
+        if (len() > 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
@@ -163,7 +164,7 @@ Option::valid() {
     return (true);
 }
 
-OptionPtr Option::getOption(unsigned short opt_type) {
+OptionPtr Option::getOption(uint16_t opt_type) {
     isc::dhcp::Option::OptionCollection::const_iterator x =
         options_.find(opt_type);
     if ( x != options_.end() ) {
@@ -172,7 +173,7 @@ OptionPtr Option::getOption(unsigned short opt_type) {
     return OptionPtr(); // NULL
 }
 
-bool Option::delOption(unsigned short opt_type) {
+bool Option::delOption(uint16_t opt_type) {
     isc::dhcp::Option::OptionCollection::iterator x = options_.find(opt_type);
     if ( x != options_.end() ) {
         options_.erase(x);
@@ -226,7 +227,7 @@ void Option::addOption(OptionPtr opt) {
                       << " already present in this message.");
         }
     }
-    options_.insert(pair<int, OptionPtr >(opt->getType(), opt));
+    options_.insert(make_pair(opt->getType(), opt));
 }
 
 uint8_t Option::getUint8() {
diff --git a/src/lib/dhcp/option.h b/src/lib/dhcp/option.h
index f0dec3b..c7f5d10 100644
--- a/src/lib/dhcp/option.h
+++ b/src/lib/dhcp/option.h
@@ -24,7 +24,9 @@
 namespace isc {
 namespace dhcp {
 
-/// buffer types used in DHCP code
+/// @brief buffer types used in DHCP code.
+///
+/// Dereferencing OptionBuffer iterator will point out to contiguous memory.
 typedef std::vector<uint8_t> OptionBuffer;
 
 /// iterator for walking over OptionBuffer
@@ -34,7 +36,7 @@ typedef OptionBuffer::iterator OptionBufferIter;
 typedef OptionBuffer::const_iterator OptionBufferConstIter;
 
 /// pointer to a DHCP buffer
-typedef boost::shared_ptr< OptionBuffer > OptionBufferPtr;
+typedef boost::shared_ptr<OptionBuffer> OptionBufferPtr;
 
 /// shared pointer to Option object
 class Option;
@@ -53,8 +55,7 @@ public:
     enum Universe { V4, V6 };
 
     /// a collection of DHCPv6 options
-    typedef std::multimap<unsigned int, OptionPtr >
-    OptionCollection;
+    typedef std::multimap<unsigned int, OptionPtr> OptionCollection;
 
     /// @brief a factory function prototype
     ///
@@ -63,9 +64,7 @@ public:
     /// @param buf pointer to a buffer
     ///
     /// @return a pointer to a created option object
-    typedef OptionPtr Factory(Option::Universe u,
-                              uint16_t type,
-                              const OptionBuffer& buf);
+    typedef OptionPtr Factory(Option::Universe u, uint16_t type, const OptionBuffer& buf);
 
     /// @brief ctor, used for options constructed, usually during transmission
     ///
@@ -122,9 +121,6 @@ public:
     /// TODO: Migrate DHCPv6 code to pack(OutputBuffer& buf) version
     ///
     /// @param buf pointer to a buffer
-    ///
-    /// @return offset to first unused byte after stored option
-    ///
     virtual void pack(isc::util::OutputBuffer& buf);
 
     /// @brief Writes option in a wire-format to a buffer.
@@ -137,10 +133,7 @@ public:
     /// @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
-    /// parsed option.
+    /// @brief Parses received buffer.
     ///
     /// @param begin iterator to first byte of option data
     /// @param end iterator to end of option data (first byte after option end)
@@ -152,32 +145,28 @@ public:
     /// @param indent number of spaces before printing text
     ///
     /// @return string with text representation.
-    virtual std::string
-    toText(int indent = 0);
+    virtual std::string toText(int indent = 0);
 
     /// Returns option type (0-255 for DHCPv4, 0-65535 for DHCPv6)
     ///
     /// @return option type
-    unsigned short getType() { return (type_); }
+    uint16_t getType() { return (type_); }
 
     /// Returns length of the complete option (data length + DHCPv4/DHCPv6
     /// option header)
     ///
     /// @return length of the option
-    virtual uint16_t
-    len();
+    virtual uint16_t len();
 
     /// @brief Returns length of header (2 for v4, 4 for v6)
     ///
     /// @return length of option header
-    virtual uint16_t
-    getHeaderLen();
+    virtual uint16_t getHeaderLen();
 
     /// returns if option is valid (e.g. option may be truncated)
     ///
     /// @return true, if option is valid
-    virtual bool
-    valid();
+    virtual bool valid();
 
     /// Returns pointer to actual data.
     ///
diff --git a/src/lib/dhcp/option6_addrlst.cc b/src/lib/dhcp/option6_addrlst.cc
index b076d7b..d23b700 100644
--- a/src/lib/dhcp/option6_addrlst.cc
+++ b/src/lib/dhcp/option6_addrlst.cc
@@ -69,7 +69,6 @@ void Option6AddrLst::pack(isc::util::OutputBuffer& buf) {
     // len field contains length without 4-byte option header
     buf.writeUint16(len() - getHeaderLen());
 
-    // this wrapping is *ugly*. I wish there was a a
     for (AddressContainer::const_iterator addr=addrs_.begin();
          addr!=addrs_.end(); ++addr) {
         buf.writeData(addr->getAddress().to_v6().to_bytes().data(), V6ADDRESS_LEN);
@@ -78,7 +77,7 @@ void Option6AddrLst::pack(isc::util::OutputBuffer& buf) {
 
 void Option6AddrLst::unpack(OptionBufferConstIter begin,
                         OptionBufferConstIter end) {
-    if (distance(begin, end) % 16) {
+    if ((distance(begin, end) % V6ADDRESS_LEN) != 0) {
         isc_throw(OutOfRange, "Option " << type_
                   << " malformed: len=" << distance(begin, end)
                   << " is not divisible by 16.");
@@ -91,14 +90,13 @@ void Option6AddrLst::unpack(OptionBufferConstIter begin,
 
 std::string Option6AddrLst::toText(int indent /* =0 */) {
     stringstream tmp;
-    for (int i=0; i<indent; i++)
+    for (int i = 0; i < indent; i++)
         tmp << " ";
 
     tmp << "type=" << type_ << " " << addrs_.size() << "addr(s): ";
 
     for (AddressContainer::const_iterator addr=addrs_.begin();
-         addr!=addrs_.end();
-         ++addr) {
+         addr!=addrs_.end(); ++addr) {
         tmp << addr->toText() << " ";
     }
     return tmp.str();
@@ -106,7 +104,7 @@ std::string Option6AddrLst::toText(int indent /* =0 */) {
 
 uint16_t Option6AddrLst::len() {
 
-    return (OPTION6_HDR_LEN + addrs_.size()*16);
+    return (OPTION6_HDR_LEN + addrs_.size()*V6ADDRESS_LEN);
 }
 
 } // end of namespace isc::dhcp
diff --git a/src/lib/dhcp/option6_ia.cc b/src/lib/dhcp/option6_ia.cc
index 6178549..7fd6d06 100644
--- a/src/lib/dhcp/option6_ia.cc
+++ b/src/lib/dhcp/option6_ia.cc
@@ -49,7 +49,9 @@ void Option6IA::pack(isc::util::OutputBuffer& buf) {
 
 void Option6IA::unpack(OptionBufferConstIter begin,
                        OptionBufferConstIter end) {
-    if (distance(begin, end) < 12) {
+    // IA_NA and IA_PD have 12 bytes content (iaid, t1, t2 fields)
+    // followed by 0 or more sub-options.
+    if (distance(begin, end) < OPTION6_IA_LEN) {
         isc_throw(OutOfRange, "Option " << type_ << " truncated");
     }
     iaid_ = readUint32( &(*begin) );
diff --git a/src/lib/dhcp/option6_iaaddr.h b/src/lib/dhcp/option6_iaaddr.h
index b407b04..e6e2c16 100644
--- a/src/lib/dhcp/option6_iaaddr.h
+++ b/src/lib/dhcp/option6_iaaddr.h
@@ -52,10 +52,7 @@ public:
     /// @param buf pointer to a buffer
     void pack(isc::util::OutputBuffer& buf);
 
-    /// @brief Parses buffer.
-    ///
-    /// Parses received buffer, returns offset to the first unused byte after
-    /// parsed option.
+    /// @brief Parses received buffer.
     ///
     /// @param begin iterator to first byte of option data
     /// @param end iterator to end of option data (first byte after option end)
diff --git a/src/lib/dhcp/pkt4.h b/src/lib/dhcp/pkt4.h
index cdbdbd6..a3f683f 100644
--- a/src/lib/dhcp/pkt4.h
+++ b/src/lib/dhcp/pkt4.h
@@ -472,10 +472,10 @@ protected:
     /// output buffer (used during message transmission)
     isc::util::OutputBuffer bufferOut_;
 
-    // that's the data of input buffer used in RX packet. Note that
-    // InputBuffer does not store the data itself, but just expects that
-    // data will be valid for the whole life of InputBuffer. Therefore we
-    // need to keep the data around.
+    /// that's the data of input buffer used in RX packet. Note that
+    /// InputBuffer does not store the data itself, but just expects that
+    /// data will be valid for the whole life of InputBuffer. Therefore we
+    /// need to keep the data around.
     std::vector<uint8_t> data_;
 
     /// message type (e.g. 1=DHCPDISCOVER)
diff --git a/src/lib/dhcp/pkt6.h b/src/lib/dhcp/pkt6.h
index 6a4f44e..e1b4aff 100644
--- a/src/lib/dhcp/pkt6.h
+++ b/src/lib/dhcp/pkt6.h
@@ -76,7 +76,7 @@ public:
     ///
     /// Returned buffer will contain reasonable data only for
     /// output (TX) packet and after pack() was called. This buffer
-    /// is only valid till Pkt4 object is valid.
+    /// is only valid till Pkt6 object is valid.
     ///
     /// RX packet or TX packet before pack() will return buffer with
     /// zero length
@@ -107,12 +107,16 @@ public:
     /// @return string with text representation
     std::string toText();
 
-    /// @brief Returns calculated length of the packet.
+    /// @brief Returns length of the packet.
     ///
-    /// This function returns size of all options including DHCPv6
-    /// header. To use that function, options_ field must be set.
+    /// This function returns size required to hold this packet.
+    /// It includes DHCPv6 header and all options stored in
+    /// options_ field.
     ///
-    /// @return number of bytes required to build this packet
+    /// Note: It does not return proper length of incoming packets
+    /// before they are unpacked.
+    ///
+    /// @return number of bytes required to assemble this packet
     uint16_t len();
 
     /// Returns message type (e.g. 1 = SOLICIT)
@@ -161,30 +165,22 @@ public:
     /// @brief Sets remote address.
     ///
     /// @param remote specifies remote address
-    void setRemoteAddr(const isc::asiolink::IOAddress& remote) {
-        remote_addr_ = remote;
-    }
+    void setRemoteAddr(const isc::asiolink::IOAddress& remote) { remote_addr_ = remote; }
 
     /// @brief Returns remote address
     ///
     /// @return remote address
-    const isc::asiolink::IOAddress& getRemoteAddr() {
-        return (remote_addr_);
-    }
+    const isc::asiolink::IOAddress& getRemoteAddr() { return (remote_addr_); }
 
     /// @brief Sets local address.
     ///
     /// @param local specifies local address
-    void setLocalAddr(const isc::asiolink::IOAddress& local) {
-        local_addr_ = local;
-    }
+    void setLocalAddr(const isc::asiolink::IOAddress& local) { local_addr_ = local; }
 
     /// @brief Returns local address.
     ///
     /// @return local address
-    const isc::asiolink::IOAddress& getLocalAddr() {
-        return (local_addr_);
-    }
+    const isc::asiolink::IOAddress& getLocalAddr() { return (local_addr_); }
 
     /// @brief Sets local port.
     ///
diff --git a/src/lib/dhcp/tests/iface_mgr_unittest.cc b/src/lib/dhcp/tests/iface_mgr_unittest.cc
index bfca5ac..d9382e0 100644
--- a/src/lib/dhcp/tests/iface_mgr_unittest.cc
+++ b/src/lib/dhcp/tests/iface_mgr_unittest.cc
@@ -7,7 +7,7 @@
 // THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
 // REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
 // AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
-// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+ // INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
 // LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
@@ -127,8 +127,8 @@ TEST_F(IfaceMgrTest, dhcp6Sniffer) {
         cout << "    pkt->ifindex_ = " << pkt->ifindex_ << ";" << endl;
         cout << "    pkt->iface_ = \"" << pkt->iface_ << "\";" << endl;
 
-        // TODO it is better to declare an array and then memcpy it to
-        // packet.
+        // TODO it is better to declare statically initialize the array
+        // and then memcpy it to packet.
         for (int i=0; i< pkt->data_len_; i++) {
             cout << "    pkt->data_[" << i << "]="
                  << (int)(unsigned char)pkt->data_[i] << "; ";
@@ -202,16 +202,16 @@ TEST_F(IfaceMgrTest, getIface) {
     // check that interface can be retrieved by ifindex
     IfaceMgr::Iface* tmp = ifacemgr->getIface(5);
     // ASSERT_NE(NULL, tmp); is not supported. hmmmm.
-    ASSERT_TRUE( tmp != NULL );
+    ASSERT_TRUE(tmp != NULL);
 
-    EXPECT_EQ( "en3", tmp->getName() );
+    EXPECT_EQ("en3", tmp->getName());
     EXPECT_EQ(5, tmp->getIndex());
 
     // check that interface can be retrieved by name
     tmp = ifacemgr->getIface("lo1");
-    ASSERT_TRUE( tmp != NULL );
+    ASSERT_TRUE(tmp != NULL);
 
-    EXPECT_EQ( "lo1", tmp->getName() );
+    EXPECT_EQ("lo1", tmp->getName());
     EXPECT_EQ(1, tmp->getIndex());
 
     // check that non-existing interfaces are not returned
@@ -237,7 +237,7 @@ TEST_F(IfaceMgrTest, detectIfaces_stub) {
 
     NakedIfaceMgr* ifacemgr = new NakedIfaceMgr();
 
-    ASSERT_TRUE( ifacemgr->getIface("eth0") != NULL );
+    ASSERT_TRUE(ifacemgr->getIface("eth0") != NULL);
 
     IfaceMgr::Iface* eth0 = ifacemgr->getIface("eth0");
 
@@ -247,7 +247,7 @@ TEST_F(IfaceMgrTest, detectIfaces_stub) {
 
     IOAddress addr = *addrs.begin();
 
-    EXPECT_STREQ( "fe80::1234", addr.toText().c_str() );
+    EXPECT_STREQ("fe80::1234", addr.toText().c_str());
 
     delete ifacemgr;
 }
@@ -338,7 +338,7 @@ TEST_F(IfaceMgrTest, sendReceive6) {
 
     // prepare dummy payload
     uint8_t data[128];
-    for (int i=0;i<128; i++) {
+    for (int i = 0; i < 128; i++) {
         data[i] = i;
     }
     Pkt6Ptr sendPkt = Pkt6Ptr(new Pkt6(data, 128));
@@ -356,7 +356,7 @@ TEST_F(IfaceMgrTest, sendReceive6) {
 
     rcvPkt = ifacemgr->receive6();
 
-    ASSERT_TRUE( rcvPkt ); // received our own packet
+    ASSERT_TRUE(rcvPkt); // received our own packet
 
     // let's check that we received what was sent
     ASSERT_EQ(sendPkt->getData().size(), rcvPkt->getData().size());
@@ -369,7 +369,7 @@ TEST_F(IfaceMgrTest, sendReceive6) {
     // none is preferred over the other for sending data, so we really should not
     // assume the one or the other will always be choosen for sending data. Therefore
     // we should accept both values as source ports.
-    EXPECT_TRUE( (rcvPkt->getRemotePort() == 10546) || (rcvPkt->getRemotePort() == 10547) );
+    EXPECT_TRUE((rcvPkt->getRemotePort() == 10546) || (rcvPkt->getRemotePort() == 10547));
 
     delete ifacemgr;
 }
@@ -430,7 +430,7 @@ TEST_F(IfaceMgrTest, sendReceive4) {
 
     rcvPkt = ifacemgr->receive4();
 
-    ASSERT_TRUE( rcvPkt ); // received our own packet
+    ASSERT_TRUE(rcvPkt); // received our own packet
 
     ASSERT_NO_THROW(
         rcvPkt->unpack();
diff --git a/src/lib/dhcp/tests/libdhcp++_unittest.cc b/src/lib/dhcp/tests/libdhcp++_unittest.cc
index 33712a4..6930338 100644
--- a/src/lib/dhcp/tests/libdhcp++_unittest.cc
+++ b/src/lib/dhcp/tests/libdhcp++_unittest.cc
@@ -64,9 +64,7 @@ TEST(LibDhcpTest, packOptions6) {
 
     OutputBuffer assembled(512);
 
-    EXPECT_NO_THROW ({
-            LibDHCP::packOptions6(assembled, opts);
-    });
+    EXPECT_NO_THROW ( LibDHCP::packOptions6(assembled, opts) );
     EXPECT_EQ(35, assembled.getLength()); // options should take 35 bytes
     EXPECT_EQ(0, memcmp(assembled.getData(), packed, 35) );
 }
@@ -157,18 +155,16 @@ TEST(LibDhcpTest, packOptions4) {
     OptionPtr opt5(new Option(Option::V4,128, payload[4]));
 
     isc::dhcp::Option::OptionCollection opts; // list of options
-    opts.insert(pair<int, OptionPtr >(opt1->getType(), opt1));
-    opts.insert(pair<int, OptionPtr >(opt1->getType(), opt2));
-    opts.insert(pair<int, OptionPtr >(opt1->getType(), opt3));
-    opts.insert(pair<int, OptionPtr >(opt1->getType(), opt4));
-    opts.insert(pair<int, OptionPtr >(opt1->getType(), opt5));
+    opts.insert(make_pair(opt1->getType(), opt1));
+    opts.insert(make_pair(opt1->getType(), opt2));
+    opts.insert(make_pair(opt1->getType(), opt3));
+    opts.insert(make_pair(opt1->getType(), opt4));
+    opts.insert(make_pair(opt1->getType(), opt5));
 
     vector<uint8_t> expVect(v4Opts, v4Opts + sizeof(v4Opts));
 
     OutputBuffer buf(100);
-    EXPECT_NO_THROW (
-        LibDHCP::packOptions(buf, opts);
-    );
+    EXPECT_NO_THROW ( LibDHCP::packOptions(buf, opts) );
     ASSERT_EQ(buf.getLength(), sizeof(v4Opts));
     EXPECT_EQ(0, memcmp(v4Opts, buf.getData(), sizeof(v4Opts)));
 
diff --git a/src/lib/dhcp/tests/option6_addrlst_unittest.cc b/src/lib/dhcp/tests/option6_addrlst_unittest.cc
index 040cc70..89d4f7c 100644
--- a/src/lib/dhcp/tests/option6_addrlst_unittest.cc
+++ b/src/lib/dhcp/tests/option6_addrlst_unittest.cc
@@ -125,7 +125,7 @@ TEST_F(Option6AddrLstTest, basic) {
     opt1->pack(outBuf_);
 
     EXPECT_EQ(20, outBuf_.getLength());
-    EXPECT_EQ( 0, memcmp(expected1, outBuf_.getData(), 20) );
+    EXPECT_EQ(0, memcmp(expected1, outBuf_.getData(), 20));
 
     // two addresses
     Option6AddrLst* opt2 = 0;
@@ -144,7 +144,7 @@ TEST_F(Option6AddrLstTest, basic) {
     opt2->pack(outBuf_);
 
     EXPECT_EQ(36, outBuf_.getLength() );
-    EXPECT_EQ( 0, memcmp(expected2, outBuf_.getData(), 36));
+    EXPECT_EQ(0, memcmp(expected2, outBuf_.getData(), 36));
 
     // three addresses
     Option6AddrLst* opt3 = 0;
@@ -165,7 +165,7 @@ TEST_F(Option6AddrLstTest, basic) {
     opt3->pack(outBuf_);
 
     EXPECT_EQ(52, outBuf_.getLength());
-    EXPECT_EQ( 0, memcmp(expected3, outBuf_.getData(), 52) );
+    EXPECT_EQ(0, memcmp(expected3, outBuf_.getData(), 52));
 
     EXPECT_NO_THROW(
         delete opt1;
diff --git a/src/lib/dhcp/tests/option6_ia_unittest.cc b/src/lib/dhcp/tests/option6_ia_unittest.cc
index 08a3567..67c84a3 100644
--- a/src/lib/dhcp/tests/option6_ia_unittest.cc
+++ b/src/lib/dhcp/tests/option6_ia_unittest.cc
@@ -133,12 +133,10 @@ TEST_F(Option6IATest, suboptions_pack) {
     ia->setT1(0x2345);
     ia->setT2(0x3456);
 
-    OptionPtr sub1(new Option(Option::V6,
-                                              0xcafe));
+    OptionPtr sub1(new Option(Option::V6, 0xcafe));
 
     boost::shared_ptr<Option6IAAddr> addr1(
-        new Option6IAAddr(D6O_IAADDR, IOAddress("2001:db8:1234:5678::abcd"),
-                          0x5000, 0x7000));
+        new Option6IAAddr(D6O_IAADDR, IOAddress("2001:db8:1234:5678::abcd"), 0x5000, 0x7000));
 
     ia->addOption(sub1);
     ia->addOption(addr1);
@@ -180,16 +178,16 @@ TEST_F(Option6IATest, suboptions_pack) {
 
 // test if option can parse suboptions
 TEST_F(Option6IATest, suboptions_unpack) {
-    // 48 bytes
+    // sizeof (expected) = 48 bytes
     uint8_t expected[] = {
-        D6O_IA_NA/256, D6O_IA_NA%256, // type
+        D6O_IA_NA / 256, D6O_IA_NA % 256, // type
         0, 28, // length
         0x13, 0x57, 0x9a, 0xce, // iaid
         0, 0, 0x23, 0x45,  // T1
         0, 0, 0x34, 0x56,  // T2
 
         // iaaddr suboption
-        D6O_IAADDR/256, D6O_IAADDR%256, // type
+        D6O_IAADDR / 256, D6O_IAADDR % 256, // type
         0, 24, // len
         0x20, 0x01, 0xd, 0xb8, 0x12,0x34, 0x56, 0x78,
         0, 0, 0, 0, 0, 0, 0xab, 0xcd, // IP address
@@ -200,6 +198,7 @@ TEST_F(Option6IATest, suboptions_unpack) {
         0xca, 0xfe, // type
         0, 0 // len
     };
+    ASSERT_EQ(sizeof(expected), 48);
 
     memcpy(&buf_[0], expected, sizeof(expected));
 
diff --git a/src/lib/dhcp/tests/option6_iaaddr_unittest.cc b/src/lib/dhcp/tests/option6_iaaddr_unittest.cc
index a6f763f..e351d17 100644
--- a/src/lib/dhcp/tests/option6_iaaddr_unittest.cc
+++ b/src/lib/dhcp/tests/option6_iaaddr_unittest.cc
@@ -42,8 +42,8 @@ public:
 };
 
 TEST_F(Option6IAAddrTest, basic) {
-    for (int i=0; i<255; i++) {
-        buf_[i]=0;
+    for (int i = 0; i < 255; i++) {
+        buf_[i] = 0;
     }
     buf_[0] = 0x20;
     buf_[1] = 0x01;
@@ -100,9 +100,7 @@ TEST_F(Option6IAAddrTest, basic) {
     // if option content is correct
     EXPECT_EQ(0, memcmp(out + 4, &buf_[0], 24));
 
-    EXPECT_NO_THROW(
-        delete opt;
-    );
+    EXPECT_NO_THROW( delete opt );
 }
 
 }
diff --git a/src/lib/dhcp/tests/option_unittest.cc b/src/lib/dhcp/tests/option_unittest.cc
index 265da28..66159f4 100644
--- a/src/lib/dhcp/tests/option_unittest.cc
+++ b/src/lib/dhcp/tests/option_unittest.cc
@@ -69,6 +69,28 @@ TEST_F(OptionTest, v4_basic) {
         delete opt;
         opt = 0;
     }
+
+    // 0 is a special PAD option
+    EXPECT_THROW(
+        opt = new Option(Option::V4, 0),
+        BadValue
+    );
+
+    if (opt) {
+        delete opt;
+        opt = 0;
+    }
+
+    // 255 is a special END option
+    EXPECT_THROW(
+        opt = new Option(Option::V4, 255),
+        BadValue
+    );
+
+    if (opt) {
+        delete opt;
+        opt = 0;
+    }
 }
 
 const uint8_t dummyPayload[] =
diff --git a/src/lib/dhcp/tests/pkt6_unittest.cc b/src/lib/dhcp/tests/pkt6_unittest.cc
index 75376cc..ea7b52c 100644
--- a/src/lib/dhcp/tests/pkt6_unittest.cc
+++ b/src/lib/dhcp/tests/pkt6_unittest.cc
@@ -41,7 +41,7 @@ TEST_F(Pkt6Test, constructor) {
     Pkt6 * pkt1 = new Pkt6(data, sizeof(data) );
 
     EXPECT_EQ(6, pkt1->getData().size());
-    EXPECT_EQ(0, memcmp( &pkt1->getData()[0], data, sizeof(data)) );
+    EXPECT_EQ(0, memcmp( &pkt1->getData()[0], data, sizeof(data)));
 
     delete pkt1;
 }
@@ -49,34 +49,34 @@ TEST_F(Pkt6Test, constructor) {
 // captured actual SOLICIT packet: transid=0x3d79fb
 // options: client-id, in_na, dns-server, elapsed-time, option-request
 // this code is autogenerated (see src/bin/dhcp6/tests/iface_mgr_unittest.c)
-Pkt6 *capture1() {
+Pkt6* capture1() {
     Pkt6* pkt;
     uint8_t data[98];
     data[0]=1;
-    data[1]=01;     data[2]=02;     data[3]=03;     data[4]=0;
-    data[5]=1;     data[6]=0;     data[7]=14;     data[8]=0;
-    data[9]=1;     data[10]=0;     data[11]=1;     data[12]=21;
-    data[13]=158;     data[14]=60;     data[15]=22;     data[16]=0;
-    data[17]=30;     data[18]=140;     data[19]=155;     data[20]=115;
-    data[21]=73;     data[22]=0;     data[23]=3;     data[24]=0;
-    data[25]=40;     data[26]=0;     data[27]=0;     data[28]=0;
-    data[29]=1;     data[30]=255;     data[31]=255;     data[32]=255;
-    data[33]=255;     data[34]=255;     data[35]=255;     data[36]=255;
+    data[1]=01;       data[2]=02;     data[3]=03;     data[4]=0;
+    data[5]=1;        data[6]=0;      data[7]=14;     data[8]=0;
+    data[9]=1;        data[10]=0;     data[11]=1;     data[12]=21;
+    data[13]=158;     data[14]=60;    data[15]=22;    data[16]=0;
+    data[17]=30;      data[18]=140;   data[19]=155;   data[20]=115;
+    data[21]=73;      data[22]=0;     data[23]=3;     data[24]=0;
+    data[25]=40;      data[26]=0;     data[27]=0;     data[28]=0;
+    data[29]=1;       data[30]=255;   data[31]=255;   data[32]=255;
+    data[33]=255;     data[34]=255;   data[35]=255;   data[36]=255;
     data[37]=255;     data[38]=0;     data[39]=5;     data[40]=0;
-    data[41]=24;     data[42]=32;     data[43]=1;     data[44]=13;
+    data[41]=24;      data[42]=32;    data[43]=1;     data[44]=13;
     data[45]=184;     data[46]=0;     data[47]=1;     data[48]=0;
-    data[49]=0;     data[50]=0;     data[51]=0;     data[52]=0;
-    data[53]=0;     data[54]=0;     data[55]=0;     data[56]=18;
-    data[57]=52;     data[58]=255;     data[59]=255;     data[60]=255;
-    data[61]=255;     data[62]=255;     data[63]=255;     data[64]=255;
-    data[65]=255;     data[66]=0;     data[67]=23;     data[68]=0;
-    data[69]=16;     data[70]=32;     data[71]=1;     data[72]=13;
+    data[49]=0;       data[50]=0;     data[51]=0;     data[52]=0;
+    data[53]=0;       data[54]=0;     data[55]=0;     data[56]=18;
+    data[57]=52;      data[58]=255;   data[59]=255;   data[60]=255;
+    data[61]=255;     data[62]=255;   data[63]=255;   data[64]=255;
+    data[65]=255;     data[66]=0;     data[67]=23;    data[68]=0;
+    data[69]=16;      data[70]=32;    data[71]=1;     data[72]=13;
     data[73]=184;     data[74]=0;     data[75]=1;     data[76]=0;
-    data[77]=0;     data[78]=0;     data[79]=0;     data[80]=0;
-    data[81]=0;     data[82]=0;     data[83]=0;     data[84]=221;
+    data[77]=0;       data[78]=0;     data[79]=0;     data[80]=0;
+    data[81]=0;       data[82]=0;     data[83]=0;     data[84]=221;
     data[85]=221;     data[86]=0;     data[87]=8;     data[88]=0;
-    data[89]=2;     data[90]=0;     data[91]=100;     data[92]=0;
-    data[93]=6;     data[94]=0;     data[95]=2;     data[96]=0;
+    data[89]=2;       data[90]=0;     data[91]=100;   data[92]=0;
+    data[93]=6;       data[94]=0;     data[95]=2;     data[96]=0;
     data[97]=23;
 
     pkt = new Pkt6(data, sizeof(data));
@@ -92,7 +92,7 @@ Pkt6 *capture1() {
 
 
 TEST_F(Pkt6Test, unpack_solicit1) {
-    Pkt6 * sol = capture1();
+    Pkt6* sol = capture1();
 
     ASSERT_EQ(true, sol->unpack());
 
@@ -121,9 +121,9 @@ TEST_F(Pkt6Test, packUnpack) {
 
     Pkt6* parent = new Pkt6(DHCPV6_SOLICIT, 0x020304);
 
-    boost::shared_ptr<Option> opt1(new Option(Option::V6, 1));
-    boost::shared_ptr<Option> opt2(new Option(Option::V6, 2));
-    boost::shared_ptr<Option> opt3(new Option(Option::V6, 100));
+    OptionPtr opt1(new Option(Option::V6, 1));
+    OptionPtr opt2(new Option(Option::V6, 2));
+    OptionPtr opt3(new Option(Option::V6, 100));
     // let's not use zero-length option type 3 as it is IA_NA
 
     parent->addOption(opt1);
@@ -133,16 +133,17 @@ TEST_F(Pkt6Test, packUnpack) {
     EXPECT_EQ(DHCPV6_SOLICIT, parent->getType());
 
     // calculated length should be 16
-    EXPECT_EQ( Pkt6::DHCPV6_PKT_HDR_LEN + 3*Option::OPTION6_HDR_LEN,
-               parent->len() );
+    EXPECT_EQ(Pkt6::DHCPV6_PKT_HDR_LEN + 3 * Option::OPTION6_HDR_LEN,
+              parent->len());
 
-    EXPECT_TRUE( parent->pack() );
+    EXPECT_TRUE(parent->pack());
 
-    EXPECT_EQ( Pkt6::DHCPV6_PKT_HDR_LEN + 3*Option::OPTION6_HDR_LEN,
-               parent->len() );
+    EXPECT_EQ(Pkt6::DHCPV6_PKT_HDR_LEN + 3 * Option::OPTION6_HDR_LEN,
+              parent->len());
 
     // create second packet,based on assembled data from the first one
-    Pkt6* clone = new Pkt6((const uint8_t*)parent->getBuffer().getData(), parent->getBuffer().getLength());
+    Pkt6* clone = new Pkt6((const uint8_t*)parent->getBuffer().getData(),
+                           parent->getBuffer().getLength());
 
     // now recreate options list
     EXPECT_TRUE( clone->unpack() );
@@ -161,11 +162,11 @@ TEST_F(Pkt6Test, packUnpack) {
 }
 
 TEST_F(Pkt6Test, addGetDelOptions) {
-    Pkt6 * parent = new Pkt6(DHCPV6_SOLICIT, random() );
+    Pkt6* parent = new Pkt6(DHCPV6_SOLICIT, random() );
 
-    boost::shared_ptr<Option> opt1(new Option(Option::V6, 1));
-    boost::shared_ptr<Option> opt2(new Option(Option::V6, 2));
-    boost::shared_ptr<Option> opt3(new Option(Option::V6, 2));
+    OptionPtr opt1(new Option(Option::V6, 1));
+    OptionPtr opt2(new Option(Option::V6, 2));
+    OptionPtr opt3(new Option(Option::V6, 2));
 
     parent->addOption(opt1);
     parent->addOption(opt2);
@@ -175,7 +176,7 @@ TEST_F(Pkt6Test, addGetDelOptions) {
     EXPECT_EQ(opt2, parent->getOption(2));
 
     // expect NULL
-    EXPECT_EQ(boost::shared_ptr<Option>(), parent->getOption(4));
+    EXPECT_EQ(OptionPtr(), parent->getOption(4));
 
     // now there are 2 options of type 2
     parent->addOption(opt3);
@@ -184,13 +185,13 @@ TEST_F(Pkt6Test, addGetDelOptions) {
     EXPECT_EQ(true, parent->delOption(2));
 
     // there still should be the other option 2
-    EXPECT_NE(boost::shared_ptr<Option>(), parent->getOption(2));
+    EXPECT_NE(OptionPtr(), parent->getOption(2));
 
     // let's delete the other option 2
     EXPECT_EQ(true, parent->delOption(2));
 
     // no more options with type=2
-    EXPECT_EQ(boost::shared_ptr<Option>(), parent->getOption(2));
+    EXPECT_EQ(OptionPtr(), parent->getOption(2));
 
     // let's try to delete - should fail
     EXPECT_TRUE(false ==  parent->delOption(2));
diff --git a/src/lib/util/buffer.h b/src/lib/util/buffer.h
index 24f2707..7982fce 100644
--- a/src/lib/util/buffer.h
+++ b/src/lib/util/buffer.h
@@ -111,7 +111,7 @@ public:
     /// @param end iterator to end of the vector
     InputBuffer(std::vector<uint8_t>::const_iterator begin,
                 std::vector<uint8_t>::const_iterator end) :
-        data_(&(*begin)), len_(distance(begin, end)) {}
+        position_(0), data_(&(*begin)), len_(distance(begin, end)) {}
     //@}
 
     ///
diff --git a/src/lib/util/tests/buffer_unittest.cc b/src/lib/util/tests/buffer_unittest.cc
index 666924e..5edd763 100644
--- a/src/lib/util/tests/buffer_unittest.cc
+++ b/src/lib/util/tests/buffer_unittest.cc
@@ -239,7 +239,7 @@ TEST_F(BufferTest, outputBufferZeroSize) {
     });
 }
 
-TEST_F(BufferTest, readVectorAll) {
+TEST_F(BufferTest, inputBufferReadVectorAll) {
     std::vector<uint8_t> vec;
 
     // check that vector can read the whole buffer
@@ -255,7 +255,7 @@ TEST_F(BufferTest, readVectorAll) {
     );
 }
 
-TEST_F(BufferTest, readVectorChunks) {
+TEST_F(BufferTest, inputBufferReadVectorChunks) {
     std::vector<uint8_t> vec;
 
     // check that vector can read the whole buffer
@@ -271,4 +271,19 @@ TEST_F(BufferTest, readVectorChunks) {
     EXPECT_EQ(0, memcmp(&vec[0], testdata+3, 2));
 }
 
+TEST_F(BufferTest, inputBufferConstructorVector) {
+    std::vector<uint8_t> vec(17);
+    for (int i = 0; i < vec.size(); i++) {
+        vec[i] = i;
+    }
+
+    InputBuffer buf(vec.begin(), vec.end());
+
+    EXPECT_EQ(buf.getLength(), 17);
+
+    std::vector<uint8_t> vec2;
+    EXPECT_NO_THROW(buf.readVector(vec2, 17));
+    EXPECT_EQ(vec, vec2);
+}
+
 }



More information about the bind10-changes mailing list