BIND 10 master, updated. 409e800ffc208240ec70eb63bc2e56aadfbb21e1 Merge branch 'trac1226'

BIND 10 source code commits bind10-changes at lists.isc.org
Wed Nov 2 10:59:00 UTC 2011


The branch, master has been updated
       via  409e800ffc208240ec70eb63bc2e56aadfbb21e1 (commit)
       via  b5040b229739c8c69463fe462aa8f7b4a8e47f7f (commit)
       via  0fed56c3692e358184958cc1263cff67db0f62cb (commit)
       via  1173960107363c04608726b57218a54d2b3b3d56 (commit)
       via  e76affc220a5f62b24e34152afdda62328a327ec (commit)
       via  d15cad92c958a6380c90ba76a2ea968e1d8304dc (commit)
       via  e098bcfbef9b8a66c3330bd37c6bbd8d72a1399e (commit)
       via  a7d0518a8c66ebc0eb471eccd67054d27caa07a3 (commit)
       via  b93bdb9b324b7dc56bd12b5c781e20275bfc3310 (commit)
       via  351ce9ee1612362800453a280dabc012565493c6 (commit)
      from  6e4e3ac19c322c65679c6c5653cc41b80305d9b9 (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 409e800ffc208240ec70eb63bc2e56aadfbb21e1
Merge: 6e4e3ac19c322c65679c6c5653cc41b80305d9b9 b5040b229739c8c69463fe462aa8f7b4a8e47f7f
Author: Tomek Mrugalski <tomasz at isc.org>
Date:   Wed Nov 2 11:23:24 2011 +0100

    Merge branch 'trac1226'

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

Summary of changes:
 src/lib/asiolink/io_address.cc                |   15 ++
 src/lib/asiolink/io_address.h                 |   18 ++
 src/lib/asiolink/tests/io_address_unittest.cc |   16 ++
 src/lib/dhcp/pkt4.cc                          |   54 ++++++-
 src/lib/dhcp/pkt4.h                           |   53 ++++---
 src/lib/dhcp/pkt6.cc                          |    7 +
 src/lib/dhcp/tests/pkt4_unittest.cc           |  206 +++++++++++++------------
 7 files changed, 239 insertions(+), 130 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/asiolink/io_address.cc b/src/lib/asiolink/io_address.cc
index 51c0332..0fe1db4 100644
--- a/src/lib/asiolink/io_address.cc
+++ b/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
diff --git a/src/lib/asiolink/io_address.h b/src/lib/asiolink/io_address.h
index 9fac580..c40e5b9 100644
--- a/src/lib/asiolink/io_address.h
+++ b/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_;
diff --git a/src/lib/asiolink/tests/io_address_unittest.cc b/src/lib/asiolink/tests/io_address_unittest.cc
index eddb0e8..4322283 100644
--- a/src/lib/asiolink/tests/io_address_unittest.cc
+++ b/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");
+}
diff --git a/src/lib/dhcp/pkt4.cc b/src/lib/dhcp/pkt4.cc
index d8e05d9..0758539 100644
--- a/src/lib/dhcp/pkt4.cc
+++ b/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
diff --git a/src/lib/dhcp/pkt4.h b/src/lib/dhcp/pkt4.h
index 7ac0ca9..75440de 100644
--- a/src/lib/dhcp/pkt4.h
+++ b/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_;
diff --git a/src/lib/dhcp/pkt6.cc b/src/lib/dhcp/pkt6.cc
index b00652a..70be2bb 100644
--- a/src/lib/dhcp/pkt6.cc
+++ b/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="
diff --git a/src/lib/dhcp/tests/pkt4_unittest.cc b/src/lib/dhcp/tests/pkt4_unittest.cc
index 3988fb0..70a7b76 100644
--- a/src/lib/dhcp/tests/pkt4_unittest.cc
+++ b/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;
     }




More information about the bind10-changes mailing list