[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

src/lib/dhcp/libdhcp++.cc

@@ -45,7 +45,7 @@ size_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,7 +73,7 @@ size_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;
     }
 
@@ -103,7 +103,7 @@ size_t LibDHCP::unpackOptions4(const OptionBuffer& buf,
         }
 
         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.");
@@ -117,7 +117,7 @@ size_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);
@@ -141,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);
         }
@@ -161,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.");
         }

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() {

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
     ///

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

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) );

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)

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.
     ///

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();

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)));
 

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;

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));
 

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 );
 }
 
 }

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[] =

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));

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)) {}
     //@}
 
     ///

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);
+}
+
 }