Merge branch 'trac1226'

src/lib/asiolink/io_address.cc

@@ -15,6 +15,7 @@
 #include <config.h>
 
 #include <unistd.h>             // for some IPC/network system calls
+#include <stdint.h>
 #include <sys/socket.h>
 #include <netinet/in.h>
 
@@ -49,6 +50,11 @@ IOAddress::IOAddress(const ip::address& asio_address) :
     asio_address_(asio_address)
 {}
 
+IOAddress::IOAddress(uint32_t v4address):
+    asio_address_(asio::ip::address_v4(v4address)) {
+
+}
+
 string
 IOAddress::toText() const {
     return (asio_address_.to_string());
@@ -84,5 +90,14 @@ IOAddress::getAddress() const {
     return asio_address_;
 }
 
+IOAddress::operator uint32_t() const {
+    if (getAddress().is_v4()) {
+        return (getAddress().to_v4().to_ulong());
+    } else {
+        isc_throw(BadValue, "Can't convert " << toText()
+                  << " address to IPv4.");
+    }
+}
+
 } // namespace asiolink
 } // namespace isc

src/lib/asiolink/io_address.h

@@ -19,6 +19,7 @@
 // this file.  In particular, asio.hpp should never be included here.
 // See the description of the namespace below.
 #include <unistd.h>             // for some network system calls
+#include <stdint.h>             // for uint32_t
 #include <asio/ip/address.hpp>
 
 #include <functional>
@@ -71,6 +72,15 @@ public:
     IOAddress(const asio::ip::address& asio_address);
     //@}
 
+    /// @brief Constructor for ip::address_v4 object.
+    ///
+    /// This constructor is intented to be used when constructing
+    /// IPv4 address out of uint32_t type. Passed value must be in
+    /// network byte order
+    ///
+    /// @param v4address IPv4 address represnted by uint32_t
+    IOAddress(uint32_t v4address);
+
     /// \brief Convert the address to a string.
     ///
     /// This method is basically expected to be exception free, but
@@ -139,6 +149,14 @@ public:
         return (nequals(other));
     }
 
+    /// \brief Converts IPv4 address to uint32_t
+    ///
+    /// Will throw BadValue exception if that is not IPv4
+    /// address.
+    ///
+    /// \return uint32_t that represents IPv4 address in
+    ///         network byte order
+    operator uint32_t () const;
 
 private:
     asio::ip::address asio_address_;

src/lib/asiolink/tests/io_address_unittest.cc

@@ -83,3 +83,19 @@ TEST(IOAddressTest, from_bytes) {
     });
     EXPECT_EQ(addr.toText(), IOAddress("192.0.2.3").toText());
 }
+
+TEST(IOAddressTest, uint32) {
+    IOAddress addr1("192.0.2.5");
+
+    // operator uint_32() is used here
+    uint32_t tmp = addr1;
+
+    uint32_t expected = (192U << 24) +  (0U << 16) + (2U << 8) + 5U;
+
+    EXPECT_EQ(expected, tmp);
+
+    // now let's try opposite conversion
+    IOAddress addr3 = IOAddress(expected);
+
+    EXPECT_EQ(addr3.toText(), "192.0.2.5");
+}

src/lib/dhcp/pkt4.cc

@@ -47,7 +47,7 @@ Pkt4::Pkt4(uint8_t msg_type, uint32_t transid)
       yiaddr_(DEFAULT_ADDRESS),
       siaddr_(DEFAULT_ADDRESS),
       giaddr_(DEFAULT_ADDRESS),
-      bufferIn_(0), // not used, this is TX packet
+      bufferIn_(NULL, 0), // not used, this is TX packet
       bufferOut_(DHCPV4_PKT_HDR_LEN),
       msg_type_(msg_type)
 {
@@ -73,15 +73,15 @@ Pkt4::Pkt4(const uint8_t* data, size_t len)
       yiaddr_(DEFAULT_ADDRESS),
       siaddr_(DEFAULT_ADDRESS),
       giaddr_(DEFAULT_ADDRESS),
-      bufferIn_(0), // not used, this is TX packet
-      bufferOut_(DHCPV4_PKT_HDR_LEN),
+      bufferIn_(data, len),
+      bufferOut_(0), // not used, this is RX packet
       msg_type_(DHCPDISCOVER)
 {
     if (len < DHCPV4_PKT_HDR_LEN) {
         isc_throw(OutOfRange, "Truncated DHCPv4 packet (len=" << len
-                  << " received, at least 236 bytes expected.");
+                  << " received, at least " << DHCPV4_PKT_HDR_LEN
+                  << "is expected");
     }
-    bufferIn_.writeData(data, len);
 }
 
 size_t
@@ -94,15 +94,51 @@ Pkt4::len() {
 
 bool
 Pkt4::pack() {
-    /// TODO: Implement this (ticket #1227)
-
-    return (false);
+    bufferOut_.writeUint8(op_);
+    bufferOut_.writeUint8(htype_);
+    bufferOut_.writeUint8(hlen_);
+    bufferOut_.writeUint8(hops_);
+    bufferOut_.writeUint32(transid_);
+    bufferOut_.writeUint16(secs_);
+    bufferOut_.writeUint16(flags_);
+    bufferOut_.writeUint32(ciaddr_);
+    bufferOut_.writeUint32(yiaddr_);
+    bufferOut_.writeUint32(siaddr_);
+    bufferOut_.writeUint32(giaddr_);
+    bufferOut_.writeData(chaddr_, MAX_CHADDR_LEN);
+    bufferOut_.writeData(sname_, MAX_SNAME_LEN);
+    bufferOut_.writeData(file_, MAX_FILE_LEN);
+
+    /// TODO: Options should follow here (ticket #1228)
+
+    return (true);
 }
 bool
 Pkt4::unpack() {
-    /// TODO: Implement this (ticket #1226)
+    if (bufferIn_.getLength()<DHCPV4_PKT_HDR_LEN) {
+        isc_throw(OutOfRange, "Received truncated DHCPv4 packet (len="
+                  << bufferIn_.getLength() << " received, at least "
+                  << DHCPV4_PKT_HDR_LEN << "is expected");
+    }
 
-    return (false);
+    op_ = bufferIn_.readUint8();
+    htype_ = bufferIn_.readUint8();
+    hlen_ = bufferIn_.readUint8();
+    hops_ = bufferIn_.readUint8();
+    transid_ = bufferIn_.readUint32();
+    secs_ = bufferIn_.readUint16();
+    flags_ = bufferIn_.readUint16();
+    ciaddr_ = IOAddress(bufferIn_.readUint32());
+    yiaddr_ = IOAddress(bufferIn_.readUint32());
+    siaddr_ = IOAddress(bufferIn_.readUint32());
+    giaddr_ = IOAddress(bufferIn_.readUint32());
+    bufferIn_.readData(chaddr_, MAX_CHADDR_LEN);
+    bufferIn_.readData(sname_, MAX_SNAME_LEN);
+    bufferIn_.readData(file_, MAX_FILE_LEN);
+
+    /// TODO: Parse options here (ticket #1228)
+
+    return (true);
 }
 
 std::string

src/lib/dhcp/pkt4.h

@@ -105,7 +105,7 @@ public:
     ///
     /// @return hops field
     uint8_t
-    getHops() { return (hops_); };
+    getHops() const { return (hops_); };
 
     // Note: There's no need to manipulate OP field directly,
     // thus no setOp() method. See op_ comment.
@@ -114,7 +114,7 @@ public:
     ///
     /// @return op field
     uint8_t
-    getOp() { return (op_); };
+    getOp() const { return (op_); };
 
     /// Sets secs field
     ///
@@ -126,7 +126,7 @@ public:
     ///
     /// @return secs field
     uint16_t
-    getSecs() { return (secs_); };
+    getSecs() const { return (secs_); };
 
     /// Sets flags field
     ///
@@ -138,14 +138,14 @@ public:
     ///
     /// @return flags field
     uint16_t
-    getFlags() { return (flags_); };
+    getFlags() const { return (flags_); };
 
 
     /// Returns ciaddr field
     ///
     /// @return ciaddr field
-    isc::asiolink::IOAddress&
-    getCiaddr() { return (ciaddr_); };
+    const isc::asiolink::IOAddress&
+    getCiaddr() const { return (ciaddr_); };
 
     /// Sets ciaddr field
     ///
@@ -157,8 +157,8 @@ public:
     /// Returns siaddr field
     ///
     /// @return siaddr field
-    isc::asiolink::IOAddress&
-    getSiaddr() { return (siaddr_); };
+    const isc::asiolink::IOAddress&
+    getSiaddr() const { return (siaddr_); };
 
     /// Sets siaddr field
     ///
@@ -170,8 +170,8 @@ public:
     /// Returns yiaddr field
     ///
     /// @return yiaddr field
-    isc::asiolink::IOAddress&
-    getYiaddr() { return (yiaddr_); };
+    const isc::asiolink::IOAddress&
+    getYiaddr() const { return (yiaddr_); };
 
     /// Sets yiaddr field
     ///
@@ -183,8 +183,8 @@ public:
     /// Returns giaddr field
     ///
     /// @return giaddr field
-    isc::asiolink::IOAddress&
-    getGiaddr() { return (giaddr_); };
+    const isc::asiolink::IOAddress&
+    getGiaddr() const { return (giaddr_); };
 
     /// Sets giaddr field
     ///
@@ -195,13 +195,13 @@ public:
     /// Returns value of transaction-id field
     ///
     /// @return transaction-id
-    uint32_t getTransid() { return (transid_); };
+    uint32_t getTransid() const { return (transid_); };
 
     /// Returns message type (e.g. 1 = DHCPDISCOVER)
     ///
     /// @return message type
     uint8_t
-    getType() { return (msg_type_); }
+    getType() const { return (msg_type_); }
 
     /// Sets message type (e.g. 1 = DHCPDISCOVER)
     ///
@@ -215,7 +215,7 @@ public:
     ///
     /// @return sname field
     const std::vector<uint8_t>
-    getSname() { return (std::vector<uint8_t>(sname_, &sname_[MAX_SNAME_LEN])); };
+    getSname() const { return (std::vector<uint8_t>(sname_, &sname_[MAX_SNAME_LEN])); };
 
     /// Sets sname field
     ///
@@ -230,7 +230,7 @@ public:
     ///
     /// @return pointer to file field
     const std::vector<uint8_t>
-    getFile() { return (std::vector<uint8_t>(file_, &file_[MAX_FILE_LEN])); };
+    getFile() const { return (std::vector<uint8_t>(file_, &file_[MAX_FILE_LEN])); };
 
     /// Sets file field
     ///
@@ -256,13 +256,13 @@ public:
     ///
     /// @return hardware type
     uint8_t
-    getHtype() { return (htype_); };
+    getHtype() const { return (htype_); };
 
     /// Returns hlen field
     ///
     /// @return hardware address length
     uint8_t
-    getHlen() { return (hlen_); };
+    getHlen() const { return (hlen_); };
 
     /// @brief Returns chaddr field
     ///
@@ -271,9 +271,22 @@ public:
     ///
     /// @return pointer to hardware address
     const uint8_t*
-    getChaddr() { return (chaddr_); };
+    getChaddr() const { return (chaddr_); };
 
 
+    /// Returns reference to output buffer
+    ///
+    /// 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.
+    ///
+    /// RX packet or TX packet before pack() will return buffer with
+    /// zero length
+    ///
+    /// @return reference to output buffer
+    const isc::util::OutputBuffer&
+    getBuffer() const { return (bufferOut_); };
+
 protected:
 
     /// converts DHCP message type to BOOTP op type
@@ -359,7 +372,7 @@ protected:
     /// input buffer (used during message reception)
     /// Note that it must be modifiable as hooks can modify incoming buffer),
     /// thus OutputBuffer, not InputBuffer
-    isc::util::OutputBuffer bufferIn_;
+    isc::util::InputBuffer bufferIn_;
 
     /// output buffer (used during message
     isc::util::OutputBuffer bufferOut_;

src/lib/dhcp/pkt6.cc

@@ -88,6 +88,13 @@ Pkt6::pack() {
 
 bool
 Pkt6::packUDP() {
+
+    // TODO: Once OutputBuffer is used here, some thing like this
+    // will be used. Yikes! That's ugly.
+    // bufferOut_.writeData(ciaddr_.getAddress().to_v6().to_bytes().data(), 16);
+    // It is better to implement a method in IOAddress that extracts
+    // vector<uint8_t>
+
     unsigned short length = len();
     if (data_len_ < length) {
         cout << "Previous len=" << data_len_ << ", allocating new buffer: len="

src/lib/dhcp/tests/pkt4_unittest.cc

@@ -32,43 +32,38 @@ using namespace isc::asiolink;
 using namespace isc::dhcp;
 using namespace boost;
 
-// can't compare const to value directly, as it gives strange
-// linker errors in gtest.h
-
-static size_t DHCPV4_PKT_HDR_LEN = Pkt4::DHCPV4_PKT_HDR_LEN;
-
 namespace {
 
 TEST(Pkt4Test, constructor) {
 
-    ASSERT_EQ(236U, DHCPV4_PKT_HDR_LEN);
+    ASSERT_EQ(236U, static_cast<size_t>(Pkt4::DHCPV4_PKT_HDR_LEN) );
     Pkt4* pkt = 0;
 
-    // minimal
+    // Just some dummy payload.
     uint8_t testData[250];
     for (int i = 0; i < 250; i++) {
         testData[i]=i;
     }
 
-    // positive case1. Normal received packet
+    // Positive case1. Normal received packet.
     EXPECT_NO_THROW(
-        pkt = new Pkt4(testData, 236);
+        pkt = new Pkt4(testData, Pkt4::DHCPV4_PKT_HDR_LEN);
     );
 
-    EXPECT_EQ(236, pkt->len());
+    EXPECT_EQ(static_cast<size_t>(Pkt4::DHCPV4_PKT_HDR_LEN), pkt->len());
 
     EXPECT_NO_THROW(
         delete pkt;
         pkt = 0;
     );
 
-    // positive case2. Normal outgoing packet
+    // Positive case2. Normal outgoing packet.
     EXPECT_NO_THROW(
         pkt = new Pkt4(DHCPDISCOVER, 0xffffffff);
     );
 
     // DHCPv4 packet must be at least 236 bytes long
-    EXPECT_EQ(DHCPV4_PKT_HDR_LEN, pkt->len());
+    EXPECT_EQ(static_cast<size_t>(Pkt4::DHCPV4_PKT_HDR_LEN), pkt->len());
     EXPECT_EQ(DHCPDISCOVER, pkt->getType());
     EXPECT_EQ(0xffffffff, pkt->getTransid());
     EXPECT_NO_THROW(
@@ -76,20 +71,32 @@ TEST(Pkt4Test, constructor) {
         pkt = 0;
     );
 
-    // negative case. Should drop truncated messages
+    // Negative case. Should drop truncated messages.
     EXPECT_THROW(
-        pkt = new Pkt4(testData, 235),
+        pkt = new Pkt4(testData, Pkt4::DHCPV4_PKT_HDR_LEN-1),
         OutOfRange
     );
     if (pkt) {
-        // test failed. Exception should have been thrown, but
-        // object was created instead. Let's clean this up
+        // Test failed. Exception should have been thrown, but
+        // object was created instead. Let's clean this up.
         delete pkt;
+        pkt = 0;
     }
 }
 
-// a sample transaction-id
-const static uint32_t dummyTransid = 0x12345678;
+// a sample data
+const uint8_t dummyOp = BOOTREQUEST;
+const uint8_t dummyHtype = 6;
+const uint8_t dummyHlen = 6;
+const uint8_t dummyHops = 13;
+const uint32_t dummyTransid = 0x12345678;
+const uint16_t dummySecs = 42;
+const uint16_t dummyFlags = BOOTP_BROADCAST;
+
+const IOAddress dummyCiaddr("192.0.2.1");
+const IOAddress dummyYiaddr("1.2.3.4");
+const IOAddress dummySiaddr("192.0.2.255");
+const IOAddress dummyGiaddr("255.255.255.255");
 
 // a dummy MAC address
 const uint8_t dummyMacAddr[] = {0, 1, 2, 3, 4, 5};
@@ -110,7 +117,7 @@ const uint8_t dummySname[] = "Lorem ipsum dolor sit amet, consectetur "
 BOOST_STATIC_ASSERT(sizeof(dummyFile)  == Pkt4::MAX_FILE_LEN + 1);
 BOOST_STATIC_ASSERT(sizeof(dummySname) == Pkt4::MAX_SNAME_LEN + 1);
 
-/// Generates test packet
+/// @brief Generates test packet.
 ///
 /// Allocates and generates test packet, with all fixed
 /// fields set to non-zero values. Content is not always
@@ -129,23 +136,23 @@ generateTestPacket1() {
                                   +sizeof(dummyMacAddr));
 
     // hwType = 6(ETHERNET), hlen = 6(MAC address len)
-    pkt->setHWAddr(6, 6, vectorMacAddr);
-    pkt->setHops(13); // 13 relays. Wow!
-    // transaction-id is already set
-    pkt->setSecs(42);
-    pkt->setFlags(0xffffU); // all flags set
-    pkt->setCiaddr(IOAddress("192.0.2.1"));
-    pkt->setYiaddr(IOAddress("1.2.3.4"));
-    pkt->setSiaddr(IOAddress("192.0.2.255"));
-    pkt->setGiaddr(IOAddress("255.255.255.255"));
-    // chaddr already set with setHWAddr()
+    pkt->setHWAddr(dummyHtype, dummyHlen, vectorMacAddr);
+    pkt->setHops(dummyHops); // 13 relays. Wow!
+    // Transaction-id is already set.
+    pkt->setSecs(dummySecs);
+    pkt->setFlags(dummyFlags); // all flags set
+    pkt->setCiaddr(dummyCiaddr);
+    pkt->setYiaddr(dummyYiaddr);
+    pkt->setSiaddr(dummySiaddr);
+    pkt->setGiaddr(dummyGiaddr);
+    // Chaddr already set with setHWAddr().
     pkt->setSname(dummySname, 64);
     pkt->setFile(dummyFile, 128);
 
     return (pkt);
 }
 
-/// Generates test packet
+/// @brief Generates test packet.
 ///
 /// Allocates and generates on-wire buffer that represents
 /// test packet, with all fixed fields set to non-zero values.
@@ -156,7 +163,6 @@ generateTestPacket1() {
 ///
 /// @return pointer to allocated Pkt4 object
 // Returns a vector containing a DHCPv4 packet header.
-#if 0
 vector<uint8_t>
 generateTestPacket2() {
 
@@ -165,7 +171,7 @@ generateTestPacket2() {
     uint8_t hdr[] = {
         1, 6, 6, 13,            // op, htype, hlen, hops,
         0x12, 0x34, 0x56, 0x78, // transaction-id
-        0, 42, 0xff, 0xff,      // 42 secs, 0xffff flags
+        0, 42, 0x80, 0x00,      // 42 secs, BROADCAST flags
         192, 0, 2, 1,           // ciaddr
         1, 2, 3, 4,             // yiaddr
         192, 0, 2, 255,         // siaddr
@@ -187,25 +193,24 @@ generateTestPacket2() {
 
     return (buf);
 }
-#endif
 
 TEST(Pkt4Test, fixedFields) {
 
     shared_ptr<Pkt4> pkt = generateTestPacket1();
 
     // ok, let's check packet values
-    EXPECT_EQ(1, pkt->getOp());
-    EXPECT_EQ(6, pkt->getHtype());
-    EXPECT_EQ(6, pkt->getHlen());
-    EXPECT_EQ(13, pkt->getHops());
+    EXPECT_EQ(dummyOp, pkt->getOp());
+    EXPECT_EQ(dummyHtype, pkt->getHtype());
+    EXPECT_EQ(dummyHlen, pkt->getHlen());
+    EXPECT_EQ(dummyHops, pkt->getHops());
     EXPECT_EQ(dummyTransid, pkt->getTransid());
-    EXPECT_EQ(42, pkt->getSecs());
-    EXPECT_EQ(0xffff, pkt->getFlags());
+    EXPECT_EQ(dummySecs, pkt->getSecs());
+    EXPECT_EQ(dummyFlags, pkt->getFlags());
 
-    EXPECT_EQ(string("192.0.2.1"), pkt->getCiaddr().toText());
-    EXPECT_EQ(string("1.2.3.4"), pkt->getYiaddr().toText());
-    EXPECT_EQ(string("192.0.2.255"), pkt->getSiaddr().toText());
-    EXPECT_EQ(string("255.255.255.255"), pkt->getGiaddr().toText());
+    EXPECT_EQ(dummyCiaddr.toText(), pkt->getCiaddr().toText());
+    EXPECT_EQ(dummyYiaddr.toText(), pkt->getYiaddr().toText());
+    EXPECT_EQ(dummySiaddr.toText(), pkt->getSiaddr().toText());
+    EXPECT_EQ(dummyGiaddr.toText(), pkt->getGiaddr().toText());
 
     // chaddr is always 16 bytes long and contains link-layer addr (MAC)
     EXPECT_EQ(0, memcmp(dummyChaddr, pkt->getChaddr(), 16));
@@ -217,52 +222,59 @@ TEST(Pkt4Test, fixedFields) {
     EXPECT_EQ(DHCPDISCOVER, pkt->getType());
 }
 
-#if 0
-/// TODO Uncomment when ticket #1227 is implemented
 TEST(Pkt4Test, fixedFieldsPack) {
     shared_ptr<Pkt4> pkt = generateTestPacket1();
-    shared_array<uint8_t> expectedFormat = generateTestPacket2();
+    vector<uint8_t> expectedFormat = generateTestPacket2();
 
     EXPECT_NO_THROW(
         pkt->pack();
     );
 
-    ASSERT_EQ(Pkt4::DHCPV4_PKT_HDR_LEN, pkt->len());
+    ASSERT_EQ(static_cast<size_t>(Pkt4::DHCPV4_PKT_HDR_LEN), pkt->len());
+
+    // redundant but MUCH easier for debug in gdb
+    const uint8_t* exp = &expectedFormat[0];
+    const uint8_t* got = static_cast<const uint8_t*>(pkt->getBuffer().getData());
 
-    EXPECT_EQ(0, memcmp(&expectedFormat[0], pkt->getData(), pkt->len()));
+    EXPECT_EQ(0, memcmp(exp, got, Pkt4::DHCPV4_PKT_HDR_LEN));
 }
 
 /// TODO Uncomment when ticket #1226 is implemented
 TEST(Pkt4Test, fixedFieldsUnpack) {
-    shared_array<uint8_t> expectedFormat = generateTestPkt2();
+    vector<uint8_t> expectedFormat = generateTestPacket2();
 
     shared_ptr<Pkt4> pkt(new Pkt4(&expectedFormat[0],
                                   Pkt4::DHCPV4_PKT_HDR_LEN));
 
+    EXPECT_NO_THROW(
+        pkt->unpack()
+    );
+
     // ok, let's check packet values
-    EXPECT_EQ(1, pkt->getOp());
-    EXPECT_EQ(6, pkt->getHtype());
-    EXPECT_EQ(6, pkt->getHlen());
-    EXPECT_EQ(13, pkt->getHops());
-    EXPECT_EQ(transid, pkt->getTransid());
-    EXPECT_EQ(42, pkt->getSecs());
-    EXPECT_EQ(0xffff, pkt->getFlags());
-
-    EXPECT_EQ(string("192.0.2.1"), pkt->getCiaddr.toText());
-    EXPECT_EQ(string("1.2.3.4"), pkt->getYiaddr.toText());
-    EXPECT_EQ(string("192.0.2.255"), pkt->getSiaddr.toText());
-    EXPECT_EQ(string("255.255.255.255"), pkt->getGiaddr.toText());
+    EXPECT_EQ(dummyOp, pkt->getOp());
+    EXPECT_EQ(dummyHtype, pkt->getHtype());
+    EXPECT_EQ(dummyHlen, pkt->getHlen());
+    EXPECT_EQ(dummyHops, pkt->getHops());
+    EXPECT_EQ(dummyTransid, pkt->getTransid());
+    EXPECT_EQ(dummySecs, pkt->getSecs());
+    EXPECT_EQ(dummyFlags, pkt->getFlags());
+
+    EXPECT_EQ(dummyCiaddr.toText(), pkt->getCiaddr().toText());
+    EXPECT_EQ(string("1.2.3.4"), pkt->getYiaddr().toText());
+    EXPECT_EQ(string("192.0.2.255"), pkt->getSiaddr().toText());
+    EXPECT_EQ(string("255.255.255.255"), pkt->getGiaddr().toText());
 
     // chaddr is always 16 bytes long and contains link-layer addr (MAC)
-    EXPECT_EQ(0, memcmp(expectedChaddr, pkt->getChaddr(), 16));
+    EXPECT_EQ(0, memcmp(dummyChaddr, pkt->getChaddr(), Pkt4::MAX_CHADDR_LEN));
 
-    EXPECT_EQ(0, memcmp(expectedSname, pkt->getSname(), 64));
+    ASSERT_EQ(static_cast<size_t>(Pkt4::MAX_SNAME_LEN), pkt->getSname().size());
+    EXPECT_EQ(0, memcmp(dummySname, &pkt->getSname()[0], Pkt4::MAX_SNAME_LEN));
 
-    EXPECT_EQ(0, memcmp(expectedFile, pkt->getFile(), 128));
+    ASSERT_EQ(static_cast<size_t>(Pkt4::MAX_FILE_LEN), pkt->getFile().size());
+    EXPECT_EQ(0, memcmp(dummyFile, &pkt->getFile()[0], Pkt4::MAX_FILE_LEN));
 
-    EXPECT_EQ(DHCPSOLICIT, pkt->getType());
+    EXPECT_EQ(DHCPDISCOVER, pkt->getType());
 }
-#endif
 
 // this test is for hardware addresses (htype, hlen and chaddr fields)
 TEST(Pkt4Test, hwAddr) {
@@ -274,14 +286,14 @@ TEST(Pkt4Test, hwAddr) {
 
     Pkt4* pkt = 0;
     // let's test each hlen, from 0 till 16
-    for (int macLen=0; macLen < Pkt4::MAX_CHADDR_LEN; macLen++) {
-        for (int i=0; i < Pkt4::MAX_CHADDR_LEN; i++) {
+    for (int macLen = 0; macLen < Pkt4::MAX_CHADDR_LEN; macLen++) {
+        for (int i = 0; i < Pkt4::MAX_CHADDR_LEN; i++) {
             mac[i] = 0;
             expectedChaddr[i] = 0;
         }
-        for (int i=0; i < macLen; i++) {
-            mac[i] = 128+i;
-            expectedChaddr[i] = 128+i;
+        for (int i = 0; i < macLen; i++) {
+            mac[i] = 128 + i;
+            expectedChaddr[i] = 128 + i;
         }
 
         // type and transaction doesn't matter in this test
@@ -292,16 +304,15 @@ TEST(Pkt4Test, hwAddr) {
         EXPECT_EQ(0, memcmp(expectedChaddr, pkt->getChaddr(),
                             Pkt4::MAX_CHADDR_LEN));
 
-#if 0
-        /// TODO Uncomment when ticket #1227 is implemented)
         EXPECT_NO_THROW(
             pkt->pack();
         );
 
         // CHADDR starts at offset 28 in DHCP packet
-        EXPECT_EQ(0, memcmp(pkt->getData()+28, expectedChaddr,
-                            Pkt4::MAX_CHADDR_LEN));
-#endif
+        const uint8_t* ptr =
+            static_cast<const uint8_t*>(pkt->getBuffer().getData())+28;
+
+        EXPECT_EQ(0, memcmp(ptr, expectedChaddr, Pkt4::MAX_CHADDR_LEN));
 
         delete pkt;
     }
@@ -333,7 +344,7 @@ TEST(Pkt4Test, msgTypes) {
     };
 
     Pkt4* pkt = 0;
-    for (int i=0; i < sizeof(types)/sizeof(msgType); i++) {
+    for (int i = 0; i < sizeof(types) / sizeof(msgType); i++) {
 
         pkt = new Pkt4(types[i].dhcp, 0);
         EXPECT_EQ(types[i].dhcp, pkt->getType());
@@ -357,35 +368,31 @@ TEST(Pkt4Test, msgTypes) {
 TEST(Pkt4Test, sname) {
 
     uint8_t sname[Pkt4::MAX_SNAME_LEN];
-    uint8_t expectedSname[Pkt4::MAX_SNAME_LEN];
 
     Pkt4* pkt = 0;
     // let's test each sname length, from 0 till 64
     for (int snameLen=0; snameLen < Pkt4::MAX_SNAME_LEN; snameLen++) {
-        for (int i=0; i < Pkt4::MAX_SNAME_LEN; i++) {
+        for (int i = 0; i < Pkt4::MAX_SNAME_LEN; i++) {
             sname[i] = 0;
-            expectedSname[i] = 0;
         }
-        for (int i=0; i < snameLen; i++) {
+        for (int i = 0; i < snameLen; i++) {
             sname[i] = i;
-            expectedSname[i] = i;
         }
 
         // type and transaction doesn't matter in this test
         pkt = new Pkt4(DHCPOFFER, 1234);
         pkt->setSname(sname, snameLen);
 
-        EXPECT_EQ(0, memcmp(expectedSname, &pkt->getSname()[0], Pkt4::MAX_SNAME_LEN));
+        EXPECT_EQ(0, memcmp(sname, &pkt->getSname()[0], Pkt4::MAX_SNAME_LEN));
 
-#if 0
-        /// TODO Uncomment when ticket #1227 is implemented)
         EXPECT_NO_THROW(
             pkt->pack();
         );
 
         // SNAME starts at offset 44 in DHCP packet
-        EXPECT_EQ(0, memcmp(pkt->getData()+44, expectedChaddr, Pkt4::MAX_SNAME_LEN));
-#endif
+        const uint8_t* ptr =
+            static_cast<const uint8_t*>(pkt->getBuffer().getData())+44;
+        EXPECT_EQ(0, memcmp(ptr, sname, Pkt4::MAX_SNAME_LEN));
 
         delete pkt;
     }
@@ -394,35 +401,32 @@ TEST(Pkt4Test, sname) {
 TEST(Pkt4Test, file) {
 
     uint8_t file[Pkt4::MAX_FILE_LEN];
-    uint8_t expectedFile[Pkt4::MAX_FILE_LEN];
 
     Pkt4* pkt = 0;
-    // let's test each file length, from 0 till 64
-    for (int fileLen=0; fileLen < Pkt4::MAX_FILE_LEN; fileLen++) {
-        for (int i=0; i < Pkt4::MAX_FILE_LEN; i++) {
+    // Let's test each file length, from 0 till 128.
+    for (int fileLen = 0; fileLen < Pkt4::MAX_FILE_LEN; fileLen++) {
+        for (int i = 0; i < Pkt4::MAX_FILE_LEN; i++) {
             file[i] = 0;
-            expectedFile[i] = 0;
         }
-        for (int i=0; i < fileLen; i++) {
+        for (int i = 0; i < fileLen; i++) {
             file[i] = i;
-            expectedFile[i] = i;
         }
 
-        // type and transaction doesn't matter in this test
+        // Type and transaction doesn't matter in this test.
         pkt = new Pkt4(DHCPOFFER, 1234);
         pkt->setFile(file, fileLen);
 
-        EXPECT_EQ(0, memcmp(expectedFile, &pkt->getFile()[0], Pkt4::MAX_FILE_LEN));
+        EXPECT_EQ(0, memcmp(file, &pkt->getFile()[0], Pkt4::MAX_FILE_LEN));
 
-#if 0
-        /// TODO Uncomment when ticket #1227 is implemented)
+        //
         EXPECT_NO_THROW(
             pkt->pack();
         );
 
-        // FILE starts at offset 44 in DHCP packet
-        EXPECT_EQ(0, memcmp(pkt->getData()+44, expectedChaddr, Pkt4::MAX_FILE_LEN));
-#endif
+        // FILE starts at offset 108 in DHCP packet.
+        const uint8_t* ptr =
+            static_cast<const uint8_t*>(pkt->getBuffer().getData())+108;
+        EXPECT_EQ(0, memcmp(ptr, file, Pkt4::MAX_FILE_LEN));
 
         delete pkt;
     }