Browse Source

[2467] Replace "raw" pointers with scoped pointers in libdhcp tests

Also tidy up a few comments.
Stephen Morris 12 years ago
parent
commit
f22ba82438

+ 82 - 97
src/lib/dhcp/tests/iface_mgr_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2012 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2013 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -33,6 +33,7 @@ using namespace std;
 using namespace isc;
 using namespace isc::asiolink;
 using namespace isc::dhcp;
+using boost::scoped_ptr;
 
 namespace {
 
@@ -44,25 +45,24 @@ char LOOPBACK[BUF_SIZE] = "lo";
 const uint16_t PORT1 = 10547;   // V6 socket
 const uint16_t PORT2 = 10548;   // V4 socket
 
-// On some systems measured duration of receive6() and
-// receive4() appears to be shorter than select() timeout.
-// called by these functions. This may be the case
-// if different ime resolutions are used by these functions.
-// For such cases we set the tolerance of 0.01s.
+// On some systems measured duration of receive6() and receive4() appears to be
+// shorter than select() timeout.  This may be the case if different time
+// resolutions are used by these functions.  For such cases we set the
+// tolerance to 0.01s.
 const uint32_t TIMEOUT_TOLERANCE = 10000;
 
 
 class NakedIfaceMgr: public IfaceMgr {
-    // "naked" Interface Manager, exposes internal fields
+    // "Naked" Interface Manager, exposes internal fields
 public:
     NakedIfaceMgr() { }
     IfaceCollection & getIfacesLst() { return ifaces_; }
 };
 
-// dummy class for now, but this will be expanded when needed
+// Dummy class for now, but this will be expanded when needed
 class IfaceMgrTest : public ::testing::Test {
 public:
-    // these are empty for now, but let's keep them around
+    // These are empty for now, but let's keep them around
     IfaceMgrTest() {
     }
 
@@ -71,18 +71,17 @@ public:
 
 };
 
-// We need some known interface to work reliably. Loopback interface
-// is named lo on Linux and lo0 on BSD boxes. We need to find out
-// which is available. This is not a real test, but rather a workaround
-// that will go away when interface detection is implemented.
+// We need some known interface to work reliably. Loopback interface is named
+// lo on Linux and lo0 on BSD boxes. We need to find out which is available.
+// This is not a real test, but rather a workaround that will go away when
+// interface detection is implemented.
 
 // NOTE: At this stage of development, write access to current directory
 // during running tests is required.
 TEST_F(IfaceMgrTest, loDetect) {
 
-    // poor man's interface detection
-    // it will go away as soon as proper interface detection
-    // is implemented
+    // Poor man's interface detection.  It will go away as soon as proper
+    // interface detection is implemented
     if (if_nametoindex("lo") > 0) {
         snprintf(LOOPBACK, BUF_SIZE - 1, "lo");
     } else if (if_nametoindex("lo0") > 0) {
@@ -103,7 +102,7 @@ TEST_F(IfaceMgrTest, loDetect) {
 
 #if 0
 TEST_F(IfaceMgrTest, dhcp6Sniffer) {
-    // testing socket operation in a portable way is tricky
+    // Testing socket operation in a portable way is tricky
     // without interface detection implemented
 
     unlink("interfaces.txt");
@@ -150,27 +149,24 @@ TEST_F(IfaceMgrTest, dhcp6Sniffer) {
     }
     cout << "---8X-----------------------------------------" << endl;
 
-    // never happens. Infinite loop is infinite
+    // Never happens. Infinite loop is infinite
     delete pkt;
     delete ifacemgr;
 }
 #endif
 
 TEST_F(IfaceMgrTest, basic) {
-    // checks that IfaceManager can be instantiated
+    // Checks that IfaceManager can be instantiated
 
     IfaceMgr & ifacemgr = IfaceMgr::instance();
     ASSERT_TRUE(&ifacemgr != 0);
 }
 
 TEST_F(IfaceMgrTest, ifaceClass) {
-    // basic tests for Iface inner class
-
-    IfaceMgr::Iface* iface = new IfaceMgr::Iface("eth5", 7);
+    // Basic tests for Iface inner class
 
-    EXPECT_STREQ("eth5/7", iface->getFullName().c_str());
-
-    delete iface;
+    IfaceMgr::Iface iface("eth5", 7);
+    EXPECT_STREQ("eth5/7", iface.getFullName().c_str());
 }
 
 // TODO: Implement getPlainMac() test as soon as interface detection
@@ -178,9 +174,9 @@ TEST_F(IfaceMgrTest, ifaceClass) {
 TEST_F(IfaceMgrTest, getIface) {
 
     cout << "Interface checks. Please ignore socket binding errors." << endl;
-    NakedIfaceMgr* ifacemgr = new NakedIfaceMgr();
+    scoped_ptr<NakedIfaceMgr> ifacemgr(new NakedIfaceMgr());
 
-    // interface name, ifindex
+    // Interface name, ifindex
     IfaceMgr::Iface iface1("lo1", 100);
     IfaceMgr::Iface iface2("eth9", 101);
     IfaceMgr::Iface iface3("en3", 102);
@@ -189,7 +185,7 @@ TEST_F(IfaceMgrTest, getIface) {
          << " in the tested system and there are no lo1, eth9, en3, e1000g4"
          << " or wifi15 interfaces present." << endl;
 
-    // note: real interfaces may be detected as well
+    // Note: real interfaces may be detected as well
     ifacemgr->getIfacesLst().push_back(iface1);
     ifacemgr->getIfacesLst().push_back(iface2);
     ifacemgr->getIfacesLst().push_back(iface3);
@@ -204,25 +200,22 @@ TEST_F(IfaceMgrTest, getIface) {
     }
 
 
-    // check that interface can be retrieved by ifindex
+    // Check that interface can be retrieved by ifindex
     IfaceMgr::Iface* tmp = ifacemgr->getIface(102);
     ASSERT_TRUE(tmp != NULL);
 
     EXPECT_EQ("en3", tmp->getName());
     EXPECT_EQ(102, tmp->getIndex());
 
-    // check that interface can be retrieved by name
+    // Check that interface can be retrieved by name
     tmp = ifacemgr->getIface("lo1");
     ASSERT_TRUE(tmp != NULL);
 
     EXPECT_EQ("lo1", tmp->getName());
     EXPECT_EQ(100, tmp->getIndex());
 
-    // check that non-existing interfaces are not returned
+    // Check that non-existing interfaces are not returned
     EXPECT_EQ(static_cast<void*>(NULL), ifacemgr->getIface("wifi15") );
-
-    delete ifacemgr;
-
 }
 
 TEST_F(IfaceMgrTest, receiveTimeout6) {
@@ -231,7 +224,7 @@ TEST_F(IfaceMgrTest, receiveTimeout6) {
               << " Test will block for a few seconds when waiting"
               << " for timeout to occur." << std::endl;
 
-    boost::scoped_ptr<NakedIfaceMgr> ifacemgr(new NakedIfaceMgr());
+    scoped_ptr<NakedIfaceMgr> ifacemgr(new NakedIfaceMgr());
     // Open socket on the lo interface.
     IOAddress loAddr("::1");
     int socket1 = 0;
@@ -283,7 +276,7 @@ TEST_F(IfaceMgrTest, receiveTimeout4) {
               << " Test will block for a few seconds when waiting"
               << " for timeout to occur." << std::endl;
 
-    boost::scoped_ptr<NakedIfaceMgr> ifacemgr(new NakedIfaceMgr());
+    scoped_ptr<NakedIfaceMgr> ifacemgr(new NakedIfaceMgr());
     // Open socket on the lo interface.
     IOAddress loAddr("127.0.0.1");
     int socket1 = 0;
@@ -330,12 +323,12 @@ TEST_F(IfaceMgrTest, receiveTimeout4) {
 }
 
 TEST_F(IfaceMgrTest, multipleSockets) {
-    boost::scoped_ptr<NakedIfaceMgr> ifacemgr(new NakedIfaceMgr());
+    scoped_ptr<NakedIfaceMgr> ifacemgr(new NakedIfaceMgr());
 
-    // container for initialized socket descriptors
+    // Container for initialized socket descriptors
     std::list<uint16_t> init_sockets;
 
-    // create socket #1
+    // Create socket #1
     int socket1 = 0;
     ASSERT_NO_THROW(
         socket1 = ifacemgr->openSocketFromIface(LOOPBACK, PORT1, AF_INET);
@@ -343,7 +336,7 @@ TEST_F(IfaceMgrTest, multipleSockets) {
     ASSERT_GT(socket1, 0);
     init_sockets.push_back(socket1);
 
-    // create socket #2
+    // Create socket #2
     IOAddress loAddr("127.0.0.1");
     int socket2 = 0;
     ASSERT_NO_THROW(
@@ -388,7 +381,7 @@ TEST_F(IfaceMgrTest, multipleSockets) {
             }
         }
     }
-    // all created sockets have been matched if this condition works.
+    // All created sockets have been matched if this condition works.
     EXPECT_EQ(sockets.size(), matched_sockets);
 
     // closeSockets() is the other function that we want to test. It
@@ -396,7 +389,7 @@ TEST_F(IfaceMgrTest, multipleSockets) {
     // them anymore communication.
     ifacemgr->closeSockets();
 
-    // closed sockets are supposed to be removed from the list
+    // Closed sockets are supposed to be removed from the list
     sockets = iface_ptr->getSockets();
     ASSERT_EQ(0, sockets.size());
 
@@ -418,27 +411,27 @@ TEST_F(IfaceMgrTest, multipleSockets) {
 }
 
 TEST_F(IfaceMgrTest, sockets6) {
-    // testing socket operation in a portable way is tricky
-    // without interface detection implemented
+    // Testing socket operation in a portable way is tricky
+    // without interface detection implemented.
 
-    boost::scoped_ptr<NakedIfaceMgr> ifacemgr(new NakedIfaceMgr());
+    scoped_ptr<NakedIfaceMgr> ifacemgr(new NakedIfaceMgr());
 
     IOAddress loAddr("::1");
 
     Pkt6 pkt6(DHCPV6_SOLICIT, 123);
     pkt6.setIface(LOOPBACK);
 
-    // bind multicast socket to port 10547
+    // Bind multicast socket to port 10547
     int socket1 = ifacemgr->openSocket(LOOPBACK, loAddr, 10547);
     EXPECT_GT(socket1, 0); // socket > 0
 
     EXPECT_EQ(socket1, ifacemgr->getSocket(pkt6));
 
-    // bind unicast socket to port 10548
+    // Bind unicast socket to port 10548
     int socket2 = ifacemgr->openSocket(LOOPBACK, loAddr, 10548);
     EXPECT_GT(socket2, 0);
 
-    // removed code for binding socket twice to the same address/port
+    // Removed code for binding socket twice to the same address/port
     // as it caused problems on some platforms (e.g. Mac OS X)
 
     // Close sockets here because the following tests will want to
@@ -463,7 +456,7 @@ TEST_F(IfaceMgrTest, sockets6) {
 }
 
 TEST_F(IfaceMgrTest, socketsFromIface) {
-    boost::scoped_ptr<NakedIfaceMgr> ifacemgr(new NakedIfaceMgr());
+    scoped_ptr<NakedIfaceMgr> ifacemgr(new NakedIfaceMgr());
 
     // Open v6 socket on loopback interface and bind to port
     int socket1 = 0;
@@ -499,7 +492,7 @@ TEST_F(IfaceMgrTest, socketsFromIface) {
 
 
 TEST_F(IfaceMgrTest, socketsFromAddress) {
-    boost::scoped_ptr<NakedIfaceMgr> ifacemgr(new NakedIfaceMgr());
+    scoped_ptr<NakedIfaceMgr> ifacemgr(new NakedIfaceMgr());
 
     // Open v6 socket on loopback interface and bind to port
     int socket1 = 0;
@@ -534,7 +527,7 @@ TEST_F(IfaceMgrTest, socketsFromAddress) {
 }
 
 TEST_F(IfaceMgrTest, socketsFromRemoteAddress) {
-    boost::scoped_ptr<NakedIfaceMgr> ifacemgr(new NakedIfaceMgr());
+    scoped_ptr<NakedIfaceMgr> ifacemgr(new NakedIfaceMgr());
 
     // Open v6 socket to connect to remote address.
     // Loopback address is the only one that we know
@@ -582,7 +575,7 @@ TEST_F(IfaceMgrTest, DISABLED_sockets6Mcast) {
     // testing socket operation in a portable way is tricky
     // without interface detection implemented
 
-    NakedIfaceMgr* ifacemgr = new NakedIfaceMgr();
+    scoped_ptr<NakedIfaceMgr> ifacemgr(new NakedIfaceMgr());
 
     IOAddress loAddr("::1");
     IOAddress mcastAddr("ff02::1:2");
@@ -603,8 +596,6 @@ TEST_F(IfaceMgrTest, DISABLED_sockets6Mcast) {
 
     close(socket1);
     close(socket2);
-
-    delete ifacemgr;
 }
 
 TEST_F(IfaceMgrTest, sendReceive6) {
@@ -612,7 +603,7 @@ TEST_F(IfaceMgrTest, sendReceive6) {
     // testing socket operation in a portable way is tricky
     // without interface detection implemented
 
-    boost::scoped_ptr<NakedIfaceMgr> ifacemgr(new NakedIfaceMgr());
+    scoped_ptr<NakedIfaceMgr> ifacemgr(new NakedIfaceMgr());
 
     // let's assume that every supported OS have lo interface
     IOAddress loAddr("::1");
@@ -673,7 +664,7 @@ TEST_F(IfaceMgrTest, sendReceive4) {
     // testing socket operation in a portable way is tricky
     // without interface detection implemented
 
-    boost::scoped_ptr<NakedIfaceMgr> ifacemgr(new NakedIfaceMgr());
+    scoped_ptr<NakedIfaceMgr> ifacemgr(new NakedIfaceMgr());
 
     // let's assume that every supported OS have lo interface
     IOAddress loAddr("127.0.0.1");
@@ -762,7 +753,7 @@ TEST_F(IfaceMgrTest, sendReceive4) {
 
 TEST_F(IfaceMgrTest, socket4) {
 
-    NakedIfaceMgr* ifacemgr = new NakedIfaceMgr();
+    scoped_ptr<NakedIfaceMgr> ifacemgr(new NakedIfaceMgr());
 
     // Let's assume that every supported OS have lo interface.
     IOAddress loAddr("127.0.0.1");
@@ -782,16 +773,12 @@ TEST_F(IfaceMgrTest, socket4) {
     EXPECT_EQ(socket1, ifacemgr->getSocket(pkt));
 
     close(socket1);
-
-    delete ifacemgr;
 }
 
 // Test the Iface structure itself
 TEST_F(IfaceMgrTest, iface) {
-    IfaceMgr::Iface* iface = NULL;
-    EXPECT_NO_THROW(
-        iface = new IfaceMgr::Iface("eth0",1);
-    );
+    scoped_ptr<IfaceMgr::Iface> iface;
+    EXPECT_NO_THROW(iface.reset(new IfaceMgr::Iface("eth0",1)));
 
     EXPECT_EQ("eth0", iface->getName());
     EXPECT_EQ(1, iface->getIndex());
@@ -822,9 +809,7 @@ TEST_F(IfaceMgrTest, iface) {
 
     EXPECT_EQ(0, addrs.size());
 
-    EXPECT_NO_THROW(
-        delete iface;
-    );
+    EXPECT_NO_THROW(iface.reset());
 }
 
 TEST_F(IfaceMgrTest, iface_methods) {
@@ -862,22 +847,22 @@ TEST_F(IfaceMgrTest, iface_methods) {
 
 TEST_F(IfaceMgrTest, socketInfo) {
 
-    // check that socketinfo for IPv4 socket is functional
+    // Check that socketinfo for IPv4 socket is functional
     IfaceMgr::SocketInfo sock1(7, IOAddress("192.0.2.56"), DHCP4_SERVER_PORT + 7);
     EXPECT_EQ(7, sock1.sockfd_);
     EXPECT_EQ("192.0.2.56", sock1.addr_.toText());
     EXPECT_EQ(AF_INET, sock1.family_);
     EXPECT_EQ(DHCP4_SERVER_PORT + 7, sock1.port_);
 
-    // check that socketinfo for IPv6 socket is functional
+    // Check that socketinfo for IPv6 socket is functional
     IfaceMgr::SocketInfo sock2(9, IOAddress("2001:db8:1::56"), DHCP4_SERVER_PORT + 9);
     EXPECT_EQ(9, sock2.sockfd_);
     EXPECT_EQ("2001:db8:1::56", sock2.addr_.toText());
     EXPECT_EQ(AF_INET6, sock2.family_);
     EXPECT_EQ(DHCP4_SERVER_PORT + 9, sock2.port_);
 
-    // now let's test if IfaceMgr handles socket info properly
-    NakedIfaceMgr* ifacemgr = new NakedIfaceMgr();
+    // Now let's test if IfaceMgr handles socket info properly
+    scoped_ptr<NakedIfaceMgr> ifacemgr(new NakedIfaceMgr());
     IfaceMgr::Iface* loopback = ifacemgr->getIface(LOOPBACK);
     ASSERT_TRUE(loopback);
     loopback->addSocket(sock1);
@@ -891,14 +876,14 @@ TEST_F(IfaceMgrTest, socketInfo) {
         BadValue
     );
 
-    // try to send over non-existing interface
+    // Try to send over non-existing interface
     pkt6.setIface("nosuchinterface45");
     EXPECT_THROW(
         ifacemgr->getSocket(pkt6),
         BadValue
     );
 
-    // this will work
+    // This will work
     pkt6.setIface(LOOPBACK);
     EXPECT_EQ(9, ifacemgr->getSocket(pkt6));
 
@@ -908,13 +893,13 @@ TEST_F(IfaceMgrTest, socketInfo) {
     );
     EXPECT_EQ(true, deleted);
 
-    // it should throw again, there's no usable socket anymore
+    // It should throw again, there's no usable socket anymore
     EXPECT_THROW(
         ifacemgr->getSocket(pkt6),
         Unexpected
     );
 
-    // repeat for pkt4
+    // Repeat for pkt4
     Pkt4 pkt4(DHCPDISCOVER, 1);
 
     // pkt4 does not have interface set yet.
@@ -943,8 +928,6 @@ TEST_F(IfaceMgrTest, socketInfo) {
         ifacemgr->getSocket(pkt4),
         Unexpected
     );
-
-    delete ifacemgr;
 }
 
 #if defined(OS_LINUX)
@@ -1070,14 +1053,17 @@ void parse_ifconfig(const std::string& textFile, IfaceMgr::IfaceCollection& ifac
                 // Ubuntu style format: inet6 addr: ::1/128 Scope:Host
                 addr = line.substr(line.find("addr:") + 6, string::npos);
             } else {
-                // Gentoo style format: inet6 fe80::6ef0:49ff:fe96:ba17  prefixlen 64  scopeid 0x20<link>
+                // Gentoo style format: inet6 fe80::6ef0:49ff:fe96:ba17
+                // prefixlen 64  scopeid 0x20<link>
                 addr = line.substr(line.find("inet6") + 6, string::npos);
             }
 
-            // handle Ubuntu format: inet6 addr: fe80::f66d:4ff:fe96:58f2/64 Scope:Link
+            // Handle Ubuntu format: inet6 addr: fe80::f66d:4ff:fe96:58f2/64
+            // Scope:Link
             addr = addr.substr(0, addr.find("/"));
 
-            // handle inet6 fe80::ca3a:35ff:fed4:8f1d  prefixlen 64  scopeid 0x20<link>
+            // Handle inet6 fe80::ca3a:35ff:fed4:8f1d  prefixlen 64
+            // scopeid 0x20<link>
             addr = addr.substr(0, addr.find(" "));
             IOAddress a(addr);
             iface->addAddress(a);
@@ -1096,7 +1082,7 @@ void parse_ifconfig(const std::string& textFile, IfaceMgr::IfaceCollection& ifac
             IOAddress a(addr);
             iface->addAddress(a);
         } else if(line.find("Metric") != string::npos) {
-            // flags
+            // Flags
             if (line.find("UP") != string::npos) {
                 iface->flag_up_ = true;
             }
@@ -1117,15 +1103,16 @@ void parse_ifconfig(const std::string& textFile, IfaceMgr::IfaceCollection& ifac
 }
 
 
-// This test compares implemented detection routines to output of "ifconfig -a" command.
-// It is far from perfect, but it is able to verify that interface names, flags,
-// MAC address, IPv4 and IPv6 addresses are detected properly. Interface list completeness
-// (check that each interface is reported, i.e. no missing or extra interfaces) and
-// address completeness is verified.
+// This test compares implemented detection routines to output of "ifconfig
+// -a" command.  It is far from perfect, but it is able to verify that
+// interface names, flags, MAC address, IPv4 and IPv6 addresses are detected
+// properly. Interface list completeness (check that each interface is reported,
+// i.e. no missing or extra interfaces) and address completeness is verified.
 //
 // Things that are not tested:
 // - ifindex (ifconfig does not print it out)
-// - address scopes and lifetimes (we don't need it, so it is not implemented in IfaceMgr)
+// - address scopes and lifetimes (we don't need it, so it is not implemented
+//   in IfaceMgr)
 // TODO: temporarily disabled, see ticket #1529
 TEST_F(IfaceMgrTest, DISABLED_detectIfaces_linux) {
 
@@ -1139,7 +1126,7 @@ TEST_F(IfaceMgrTest, DISABLED_detectIfaces_linux) {
 
     ASSERT_EQ(0, result);
 
-    // list of interfaces parsed from ifconfig
+    // List of interfaces parsed from ifconfig
     IfaceMgr::IfaceCollection parsedIfaces;
 
     ASSERT_NO_THROW(
@@ -1173,7 +1160,7 @@ TEST_F(IfaceMgrTest, DISABLED_detectIfaces_linux) {
         cout << endl;
     }
 
-    // Ok, now we have 2 lists of interfaces. Need to compare them
+    // OK, now we have 2 lists of interfaces. Need to compare them
     ASSERT_EQ(detectedIfaces.size(), parsedIfaces.size());
 
     // TODO: This could could probably be written simple with find()
@@ -1191,14 +1178,14 @@ TEST_F(IfaceMgrTest, DISABLED_detectIfaces_linux) {
 
             cout << "Checking interface " << detected->getName() << endl;
 
-            // start with checking flags
+            // Start with checking flags
             EXPECT_EQ(detected->flag_loopback_, i->flag_loopback_);
             EXPECT_EQ(detected->flag_up_, i->flag_up_);
             EXPECT_EQ(detected->flag_running_, i->flag_running_);
             EXPECT_EQ(detected->flag_multicast_, i->flag_multicast_);
             EXPECT_EQ(detected->flag_broadcast_, i->flag_broadcast_);
 
-            // skip MAC comparison for loopback as netlink returns MAC
+            // Skip MAC comparison for loopback as netlink returns MAC
             // 00:00:00:00:00:00 for lo
             if (!detected->flag_loopback_) {
                 ASSERT_EQ(detected->getMacLen(), i->getMacLen());
@@ -1207,7 +1194,7 @@ TEST_F(IfaceMgrTest, DISABLED_detectIfaces_linux) {
 
             EXPECT_EQ(detected->getAddresses().size(), i->getAddresses().size());
 
-            // now compare addresses
+            // Now compare addresses
             const IfaceMgr::AddressCollection& addrs = detected->getAddresses();
             for (IfaceMgr::AddressCollection::const_iterator addr = addrs.begin();
                  addr != addrs.end(); ++addr) {
@@ -1230,7 +1217,7 @@ TEST_F(IfaceMgrTest, DISABLED_detectIfaces_linux) {
                      << " matched with 'ifconfig -a' output." << endl;
             }
         }
-        if (!found) { // corresponding interface was not found
+        if (!found) { // Corresponding interface was not found
             FAIL();
         }
     }
@@ -1247,14 +1234,14 @@ void my_callback(void) {
 }
 
 TEST_F(IfaceMgrTest, controlSession) {
-    // tests if extra control socket and its callback can be passed and
+    // Tests if extra control socket and its callback can be passed and
     // it is supported properly by receive4() method.
 
     callback_ok = false;
 
-    NakedIfaceMgr* ifacemgr = new NakedIfaceMgr();
+    scoped_ptr<NakedIfaceMgr> ifacemgr(new NakedIfaceMgr());
 
-    // create pipe and register it as extra socket
+    // Create pipe and register it as extra socket
     int pipefd[2];
     EXPECT_TRUE(pipe(pipefd) == 0);
     EXPECT_NO_THROW(ifacemgr->set_session_socket(pipefd[0], my_callback));
@@ -1280,8 +1267,6 @@ TEST_F(IfaceMgrTest, controlSession) {
     // There was some data, so this time callback should be called
     EXPECT_TRUE(callback_ok);
 
-    delete ifacemgr;
-
     // close both pipe ends
     close(pipefd[1]);
     close(pipefd[0]);

+ 1 - 2
src/lib/dhcp/tests/libdhcp++_unittest.cc

@@ -52,8 +52,7 @@ public:
     /// @param buf option-buffer
     static OptionPtr genericOptionFactory(Option::Universe u, uint16_t type,
                                           const OptionBuffer& buf) {
-        Option* option = new Option(u, type, buf);
-        return OptionPtr(option);
+        return (OptionPtr(new Option(u, type, buf)));
     }
 
     /// @brief Test DHCPv4 option definition.

+ 34 - 56
src/lib/dhcp/tests/option4_addrlst_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011i-2013  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -21,6 +21,7 @@
 #include <util/buffer.h>
 
 #include <gtest/gtest.h>
+#include <boost/scoped_ptr.hpp>
 
 #include <iostream>
 #include <sstream>
@@ -32,6 +33,7 @@ using namespace isc;
 using namespace isc::dhcp;
 using namespace isc::asiolink;
 using namespace isc::util;
+using boost::scoped_ptr;
 
 namespace {
 
@@ -62,7 +64,7 @@ class Option4AddrLstTest : public ::testing::Test {
 protected:
 
     Option4AddrLstTest():
-        vec_(vector<uint8_t>(300,0)) // 300 bytes long filled with 0s
+        vec_(vector<uint8_t>(300, 0)) // 300 bytes long filled with 0s
     {
         sampleAddrs_.push_back(IOAddress("192.0.2.3"));
         sampleAddrs_.push_back(IOAddress("255.255.255.0"));
@@ -79,13 +81,13 @@ TEST_F(Option4AddrLstTest, parse1) {
 
     memcpy(&vec_[0], sampledata, sizeof(sampledata));
 
-    // just one address
-    Option4AddrLst* opt1 = 0;
+    // Just one address
+    scoped_ptr<Option4AddrLst> opt1;
     EXPECT_NO_THROW(
-        opt1 = new Option4AddrLst(DHO_DOMAIN_NAME_SERVERS,
-                                  vec_.begin(),
-                                  vec_.begin()+4);
-        // use just first address (4 bytes), not the whole
+        opt1.reset(new Option4AddrLst(DHO_DOMAIN_NAME_SERVERS,
+                                      vec_.begin(),
+                                      vec_.begin()+4));
+        // Use just first address (4 bytes), not the whole
         // sampledata
     );
 
@@ -99,26 +101,21 @@ TEST_F(Option4AddrLstTest, parse1) {
 
     EXPECT_EQ("192.0.2.3", addrs[0].toText());
 
-    EXPECT_NO_THROW(
-        delete opt1;
-        opt1 = 0;
-    );
-
-    // 1 address
+    EXPECT_NO_THROW(opt1.reset());
 }
 
 TEST_F(Option4AddrLstTest, parse4) {
 
-    vector<uint8_t> buffer(300,0); // 300 bytes long filled with 0s
+    vector<uint8_t> buffer(300, 0); // 300 bytes long filled with 0s
 
     memcpy(&buffer[0], sampledata, sizeof(sampledata));
 
     // 4 addresses
-    Option4AddrLst* opt4 = 0;
+    scoped_ptr<Option4AddrLst> opt4;
     EXPECT_NO_THROW(
-        opt4 = new Option4AddrLst(254,
-                                  buffer.begin(),
-                                  buffer.begin()+sizeof(sampledata));
+        opt4.reset(new Option4AddrLst(254,
+                                      buffer.begin(),
+                                      buffer.begin()+sizeof(sampledata)));
     );
 
     EXPECT_EQ(Option::V4, opt4->getUniverse());
@@ -134,17 +131,15 @@ TEST_F(Option4AddrLstTest, parse4) {
     EXPECT_EQ("0.0.0.0", addrs[2].toText());
     EXPECT_EQ("127.0.0.1", addrs[3].toText());
 
-    EXPECT_NO_THROW(
-        delete opt4;
-        opt4 = 0;
-    );
+    EXPECT_NO_THROW(opt4.reset());
 }
 
 TEST_F(Option4AddrLstTest, assembly1) {
 
-    Option4AddrLst* opt = 0;
+    scoped_ptr<Option4AddrLst> opt;
     EXPECT_NO_THROW(
-        opt = new Option4AddrLst(DHO_DOMAIN_NAME_SERVERS, IOAddress("192.0.2.3"));
+        opt.reset(new Option4AddrLst(DHO_DOMAIN_NAME_SERVERS,
+                                     IOAddress("192.0.2.3")));
     );
     EXPECT_EQ(Option::V4, opt->getUniverse());
     EXPECT_EQ(DHO_DOMAIN_NAME_SERVERS, opt->getType());
@@ -163,28 +158,21 @@ TEST_F(Option4AddrLstTest, assembly1) {
 
     EXPECT_EQ(0, memcmp(expected1, buf.getData(), 6));
 
-    EXPECT_NO_THROW(
-        delete opt;
-        opt = 0;
-    );
+    EXPECT_NO_THROW(opt.reset());
 
     // This is old-fashioned option. We don't serve IPv6 types here!
     EXPECT_THROW(
-        opt = new Option4AddrLst(DHO_DOMAIN_NAME_SERVERS, IOAddress("2001:db8::1")),
+        opt.reset(new Option4AddrLst(DHO_DOMAIN_NAME_SERVERS,
+                                     IOAddress("2001:db8::1"))),
         BadValue
     );
-    if (opt) {
-        // test failed. Exception was not thrown, but option was created instead.
-        delete opt;
-    }
 }
 
 TEST_F(Option4AddrLstTest, assembly4) {
 
-
-    Option4AddrLst* opt = 0;
+    scoped_ptr<Option4AddrLst> opt;
     EXPECT_NO_THROW(
-        opt = new Option4AddrLst(254, sampleAddrs_);
+        opt.reset(new Option4AddrLst(254, sampleAddrs_));
     );
     EXPECT_EQ(Option::V4, opt->getUniverse());
     EXPECT_EQ(254, opt->getType());
@@ -206,27 +194,21 @@ TEST_F(Option4AddrLstTest, assembly4) {
 
     ASSERT_EQ(0, memcmp(expected4, buf.getData(), 18));
 
-    EXPECT_NO_THROW(
-        delete opt;
-        opt = 0;
-    );
+    EXPECT_NO_THROW(opt.reset());
 
     // This is old-fashioned option. We don't serve IPv6 types here!
     sampleAddrs_.push_back(IOAddress("2001:db8::1"));
     EXPECT_THROW(
-        opt = new Option4AddrLst(DHO_DOMAIN_NAME_SERVERS, sampleAddrs_),
+        opt.reset(new Option4AddrLst(DHO_DOMAIN_NAME_SERVERS, sampleAddrs_)),
         BadValue
     );
-    if (opt) {
-        // test failed. Exception was not thrown, but option was created instead.
-        delete opt;
-    }
 }
 
 TEST_F(Option4AddrLstTest, setAddress) {
-    Option4AddrLst* opt = 0;
+
+    scoped_ptr<Option4AddrLst> opt;
     EXPECT_NO_THROW(
-        opt = new Option4AddrLst(123, IOAddress("1.2.3.4"));
+        opt.reset(new Option4AddrLst(123, IOAddress("1.2.3.4")));
     );
     opt->setAddress(IOAddress("192.0.255.255"));
 
@@ -240,17 +222,15 @@ TEST_F(Option4AddrLstTest, setAddress) {
         BadValue
     );
 
-    EXPECT_NO_THROW(
-        delete opt;
-    );
+    EXPECT_NO_THROW(opt.reset());
 }
 
 TEST_F(Option4AddrLstTest, setAddresses) {
 
-    Option4AddrLst* opt = 0;
+    scoped_ptr<Option4AddrLst> opt;
 
     EXPECT_NO_THROW(
-        opt = new Option4AddrLst(123); // empty list
+        opt.reset(new Option4AddrLst(123)); // Empty list
     );
 
     opt->setAddresses(sampleAddrs_);
@@ -269,9 +249,7 @@ TEST_F(Option4AddrLstTest, setAddresses) {
         BadValue
     );
 
-    EXPECT_NO_THROW(
-        delete opt;
-    );
+    EXPECT_NO_THROW(opt.reset());
 }
 
 } // namespace

+ 32 - 33
src/lib/dhcp/tests/option6_addrlst_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2012 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2013 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -21,6 +21,7 @@
 #include <util/buffer.h>
 
 #include <gtest/gtest.h>
+#include <boost/scoped_ptr.hpp>
 
 #include <iostream>
 #include <sstream>
@@ -32,6 +33,7 @@ using namespace isc;
 using namespace isc::dhcp;
 using namespace isc::asiolink;
 using namespace isc::util;
+using boost::scoped_ptr;
 
 namespace {
 class Option6AddrLstTest : public ::testing::Test {
@@ -47,7 +49,7 @@ public:
 
 TEST_F(Option6AddrLstTest, basic) {
 
-    // Limiting tests to just a 2001:db8::/32 as is *wrong*.
+    // Limiting tests to just a 2001:db8::/32 is *wrong*.
     // Good tests check corner cases as well.
     // ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff checks
     // for integer overflow.
@@ -110,10 +112,11 @@ TEST_F(Option6AddrLstTest, basic) {
 
     memcpy(&buf_[0], sampledata, 48);
 
-    // just a single address
-    Option6AddrLst* opt1 = 0;
+    // Just a single address
+    scoped_ptr<Option6AddrLst> opt1;
     EXPECT_NO_THROW(
-        opt1 = new Option6AddrLst(D6O_NAME_SERVERS, buf_.begin(), buf_.begin() + 16 );
+        opt1.reset(new Option6AddrLst(D6O_NAME_SERVERS,
+                                      buf_.begin(), buf_.begin() + 16));
     );
 
     EXPECT_EQ(Option::V6, opt1->getUniverse());
@@ -125,16 +128,17 @@ TEST_F(Option6AddrLstTest, basic) {
     IOAddress addr = addrs[0];
     EXPECT_EQ("2001:db8:1::dead:beef", addr.toText());
 
-    // pack this option
+    // Pack this option
     opt1->pack(outBuf_);
 
     EXPECT_EQ(20, outBuf_.getLength());
     EXPECT_EQ(0, memcmp(expected1, outBuf_.getData(), 20));
 
-    // two addresses
-    Option6AddrLst* opt2 = 0;
+    // Two addresses
+    scoped_ptr<Option6AddrLst> opt2;
     EXPECT_NO_THROW(
-        opt2 = new Option6AddrLst(D6O_SIP_SERVERS_ADDR, buf_.begin(), buf_.begin() + 32);
+        opt2.reset(new Option6AddrLst(D6O_SIP_SERVERS_ADDR,
+                                      buf_.begin(), buf_.begin() + 32));
     );
     EXPECT_EQ(D6O_SIP_SERVERS_ADDR, opt2->getType());
     EXPECT_EQ(36, opt2->len());
@@ -143,17 +147,18 @@ TEST_F(Option6AddrLstTest, basic) {
     EXPECT_EQ("2001:db8:1::dead:beef", addrs[0].toText());
     EXPECT_EQ("ff02::face:b00c", addrs[1].toText());
 
-    // pack this option
+    // Pack this option
     outBuf_.clear();
     opt2->pack(outBuf_);
 
     EXPECT_EQ(36, outBuf_.getLength() );
     EXPECT_EQ(0, memcmp(expected2, outBuf_.getData(), 36));
 
-    // three addresses
-    Option6AddrLst* opt3 = 0;
+    // Three addresses
+    scoped_ptr<Option6AddrLst> opt3;
     EXPECT_NO_THROW(
-        opt3 = new Option6AddrLst(D6O_NIS_SERVERS, buf_.begin(), buf_.begin() + 48);
+        opt3.reset(new Option6AddrLst(D6O_NIS_SERVERS,
+                                      buf_.begin(), buf_.begin() + 48));
     );
 
     EXPECT_EQ(D6O_NIS_SERVERS, opt3->getType());
@@ -164,25 +169,23 @@ TEST_F(Option6AddrLstTest, basic) {
     EXPECT_EQ("ff02::face:b00c", addrs[1].toText());
     EXPECT_EQ("ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", addrs[2].toText());
 
-    // pack this option
+    // Pack this option
     outBuf_.clear();
     opt3->pack(outBuf_);
 
     EXPECT_EQ(52, outBuf_.getLength());
     EXPECT_EQ(0, memcmp(expected3, outBuf_.getData(), 52));
 
-    EXPECT_NO_THROW(
-        delete opt1;
-        delete opt2;
-        delete opt3;
-    );
+    EXPECT_NO_THROW(opt1.reset());
+    EXPECT_NO_THROW(opt2.reset());
+    EXPECT_NO_THROW(opt3.reset());
 }
 
 TEST_F(Option6AddrLstTest, constructors) {
 
-    Option6AddrLst* opt1 = 0;
+    scoped_ptr<Option6AddrLst> opt1;
     EXPECT_NO_THROW(
-        opt1 = new Option6AddrLst(1234, IOAddress("::1"));
+        opt1.reset(new Option6AddrLst(1234, IOAddress("::1")));
     );
     EXPECT_EQ(Option::V6, opt1->getUniverse());
     EXPECT_EQ(1234, opt1->getType());
@@ -195,9 +198,9 @@ TEST_F(Option6AddrLstTest, constructors) {
     addrs.push_back(IOAddress(string("fe80::1234")));
     addrs.push_back(IOAddress(string("2001:db8:1::baca")));
 
-    Option6AddrLst* opt2 = 0;
+    scoped_ptr<Option6AddrLst> opt2;
     EXPECT_NO_THROW(
-        opt2 = new Option6AddrLst(5678, addrs);
+        opt2.reset(new Option6AddrLst(5678, addrs));
     );
 
     Option6AddrLst::AddressContainer check = opt2->getAddresses();
@@ -205,16 +208,14 @@ TEST_F(Option6AddrLstTest, constructors) {
     EXPECT_EQ("fe80::1234", check[0].toText());
     EXPECT_EQ("2001:db8:1::baca", check[1].toText());
 
-    EXPECT_NO_THROW(
-        delete opt1;
-        delete opt2;
-    );
+    EXPECT_NO_THROW(opt1.reset());
+    EXPECT_NO_THROW(opt2.reset());
 }
 
 TEST_F(Option6AddrLstTest, setAddress) {
-    Option6AddrLst* opt1 = 0;
+    scoped_ptr<Option6AddrLst> opt1;
     EXPECT_NO_THROW(
-        opt1 = new Option6AddrLst(1234, IOAddress("::1"));
+        opt1.reset(new Option6AddrLst(1234, IOAddress("::1")));
     );
     opt1->setAddress(IOAddress("2001:db8:1::2"));
     /// TODO It used to be ::2 address, but io_address represents
@@ -227,12 +228,10 @@ TEST_F(Option6AddrLstTest, setAddress) {
     /// a test for IOAddress)
 
     Option6AddrLst::AddressContainer addrs = opt1->getAddresses();
-    ASSERT_EQ(1, addrs.size() );
+    ASSERT_EQ(1, addrs.size());
     EXPECT_EQ("2001:db8:1::2", addrs[0].toText());
 
-    EXPECT_NO_THROW(
-        delete opt1;
-    );
+    EXPECT_NO_THROW(opt1.reset());
 }
 
 } // namespace

+ 28 - 33
src/lib/dhcp/tests/option6_ia_unittest.cc

@@ -20,6 +20,7 @@
 #include <dhcp/option6_iaaddr.h>
 #include <util/buffer.h>
 
+#include <boost/scoped_ptr.hpp>
 #include <gtest/gtest.h>
 
 #include <iostream>
@@ -32,6 +33,7 @@ using namespace isc;
 using namespace isc::dhcp;
 using namespace isc::asiolink;
 using namespace isc::util;
+using boost::scoped_ptr;
 
 namespace {
 class Option6IATest : public ::testing::Test {
@@ -61,11 +63,11 @@ TEST_F(Option6IATest, basic) {
     buf_[10] = 0x02;
     buf_[11] = 0x01;
 
-    // create an option
+    // Create an option
     // unpack() is called from constructor
-    Option6IA* opt = new Option6IA(D6O_IA_NA,
-                                   buf_.begin(),
-                                   buf_.begin() + 12);
+    scoped_ptr<Option6IA> opt(new Option6IA(D6O_IA_NA,
+                                            buf_.begin(),
+                                            buf_.begin() + 12));
 
     EXPECT_EQ(Option::V6, opt->getUniverse());
     EXPECT_EQ(D6O_IA_NA, opt->getType());
@@ -73,10 +75,10 @@ TEST_F(Option6IATest, basic) {
     EXPECT_EQ(0x81020304, opt->getT1());
     EXPECT_EQ(0x84030201, opt->getT2());
 
-    // pack this option again in the same buffer, but in
+    // Pack this option again in the same buffer, but in
     // different place
 
-    // test for pack()
+    // Test for pack()
     opt->pack(outBuf_);
 
     // 12 bytes header + 4 bytes content
@@ -85,31 +87,29 @@ TEST_F(Option6IATest, basic) {
 
     EXPECT_EQ(16, outBuf_.getLength()); // lenght(IA_NA) = 16
 
-    // check if pack worked properly:
+    // Check if pack worked properly:
     InputBuffer out(outBuf_.getData(), outBuf_.getLength());
 
-    // if option type is correct
+    // - if option type is correct
     EXPECT_EQ(D6O_IA_NA, out.readUint16());
 
-    // if option length is correct
+    // - if option length is correct
     EXPECT_EQ(12, out.readUint16());
 
-    // if iaid is correct
+    // - if iaid is correct
     EXPECT_EQ(0xa1a2a3a4, out.readUint32() );
 
-   // if T1 is correct
+   // - if T1 is correct
     EXPECT_EQ(0x81020304, out.readUint32() );
 
-    // if T1 is correct
+    // - if T1 is correct
     EXPECT_EQ(0x84030201, out.readUint32() );
 
-    EXPECT_NO_THROW(
-        delete opt;
-    );
+    EXPECT_NO_THROW(opt.reset());
 }
 
 TEST_F(Option6IATest, simple) {
-    Option6IA* ia = new Option6IA(D6O_IA_NA, 1234);
+    scoped_ptr<Option6IA> ia(new Option6IA(D6O_IA_NA, 1234));
 
     // Check that the values are really different than what we are about
     // to set them to.
@@ -128,9 +128,7 @@ TEST_F(Option6IATest, simple) {
     ia->setIAID(890);
     EXPECT_EQ(890, ia->getIAID());
 
-    EXPECT_NO_THROW(
-        delete ia;
-    );
+    EXPECT_NO_THROW(ia.reset());
 }
 
 
@@ -140,7 +138,7 @@ TEST_F(Option6IATest, suboptions_pack) {
     buf_[1] = 0xfe;
     buf_[2] = 0xfc;
 
-    Option6IA * ia = new Option6IA(D6O_IA_NA, 0x13579ace);
+    scoped_ptr<Option6IA> ia(new Option6IA(D6O_IA_NA, 0x13579ace));
     ia->setT1(0x2345);
     ia->setT2(0x3456);
 
@@ -181,9 +179,7 @@ TEST_F(Option6IATest, suboptions_pack) {
 
     EXPECT_EQ(0, memcmp(outBuf_.getData(), expected, 48));
 
-    EXPECT_NO_THROW(
-        delete ia;
-    );
+    EXPECT_NO_THROW(ia.reset());
 }
 
 
@@ -213,10 +209,11 @@ TEST_F(Option6IATest, suboptions_unpack) {
 
     memcpy(&buf_[0], expected, sizeof(expected));
 
-    Option6IA* ia = 0;
-    EXPECT_NO_THROW({
-        ia = new Option6IA(D6O_IA_NA, buf_.begin() + 4, buf_.begin() + sizeof(expected));
-    });
+    scoped_ptr<Option6IA> ia;
+    EXPECT_NO_THROW(
+        ia.reset(new Option6IA(D6O_IA_NA, buf_.begin() + 4,
+                               buf_.begin() + sizeof(expected)));
+    );
     ASSERT_TRUE(ia);
 
     EXPECT_EQ(D6O_IA_NA, ia->getType());
@@ -227,7 +224,7 @@ TEST_F(Option6IATest, suboptions_unpack) {
     OptionPtr subopt = ia->getOption(D6O_IAADDR);
     ASSERT_NE(OptionPtr(), subopt); // non-NULL
 
-    // checks for address option
+    // Checks for address option
     Option6IAAddr * addr = dynamic_cast<Option6IAAddr*>(subopt.get());
     ASSERT_TRUE(NULL != addr);
 
@@ -237,21 +234,19 @@ TEST_F(Option6IATest, suboptions_unpack) {
     EXPECT_EQ(0x7000, addr->getValid());
     EXPECT_EQ("2001:db8:1234:5678::abcd", addr->getAddress().toText());
 
-    // checks for dummy option
+    // Checks for dummy option
     subopt = ia->getOption(0xcafe);
     ASSERT_TRUE(subopt); // should be non-NULL
 
     EXPECT_EQ(0xcafe, subopt->getType());
     EXPECT_EQ(4, subopt->len());
-    // there should be no data at all
+    // There should be no data at all
     EXPECT_EQ(0, subopt->getData().size());
 
     subopt = ia->getOption(1); // get option 1
     ASSERT_FALSE(subopt); // should be NULL
 
-    EXPECT_NO_THROW(
-        delete ia;
-    );
+    EXPECT_NO_THROW(ia.reset());
 }
 
 }

+ 12 - 11
src/lib/dhcp/tests/option6_iaaddr_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2012 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2013 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -19,6 +19,7 @@
 #include <dhcp/option6_iaaddr.h>
 #include <util/buffer.h>
 
+#include <boost/scoped_ptr.hpp>
 #include <gtest/gtest.h>
 
 #include <iostream>
@@ -68,12 +69,12 @@ TEST_F(Option6IAAddrTest, basic) {
     buf_[22] = 0x5e;
     buf_[23] = 0x00; // 3,000,000,000
 
-    // create an option (unpack content)
-    Option6IAAddr* opt = new Option6IAAddr(D6O_IAADDR,
-                                           buf_.begin(),
-                                           buf_.begin() + 24);
+    // Create an option (unpack content)
+    boost::scoped_ptr<Option6IAAddr> opt(new Option6IAAddr(D6O_IAADDR,
+                                                           buf_.begin(),
+                                                           buf_.begin() + 24));
 
-    // pack this option
+    // Pack this option
     opt->pack(outBuf_);
 
     EXPECT_EQ(28, outBuf_.getLength());
@@ -90,19 +91,19 @@ TEST_F(Option6IAAddrTest, basic) {
     EXPECT_EQ(Option::OPTION6_HDR_LEN + Option6IAAddr::OPTION6_IAADDR_LEN,
               opt->len());
 
-    // check if pack worked properly:
+    // Check if pack worked properly:
     const uint8_t* out = (const uint8_t*)outBuf_.getData();
 
-    // if option type is correct
+    // - if option type is correct
     EXPECT_EQ(D6O_IAADDR, out[0]*256 + out[1]);
 
-    // if option length is correct
+    // - if option length is correct
     EXPECT_EQ(24, out[2]*256 + out[3]);
 
-    // if option content is correct
+    // - if option content is correct
     EXPECT_EQ(0, memcmp(out + 4, &buf_[0], 24));
 
-    EXPECT_NO_THROW( delete opt );
+    EXPECT_NO_THROW(opt.reset());
 }
 
 }

+ 111 - 153
src/lib/dhcp/tests/option_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2012 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2013 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -20,6 +20,7 @@
 #include <util/buffer.h>
 
 #include <boost/shared_ptr.hpp>
+#include <boost/scoped_ptr.hpp>
 #include <gtest/gtest.h>
 
 #include <iostream>
@@ -31,6 +32,7 @@ using namespace std;
 using namespace isc;
 using namespace isc::dhcp;
 using namespace isc::util;
+using boost::scoped_ptr;
 
 namespace {
 class OptionTest : public ::testing::Test {
@@ -44,50 +46,27 @@ public:
     OutputBuffer outBuf_;
 };
 
-// v4 is not really implemented yet. A simple test will do for now
+// V4 is not really implemented yet. A simple test will do for now.
 TEST_F(OptionTest, v4_basic) {
 
-    Option* opt = 0;
-    EXPECT_NO_THROW(
-        opt = new Option(Option::V4, 17);
-    );
+    scoped_ptr<Option> opt;
+    EXPECT_NO_THROW(opt.reset(new Option(Option::V4, 17)));
 
     EXPECT_EQ(Option::V4, opt->getUniverse());
     EXPECT_EQ(17, opt->getType());
     EXPECT_EQ(0, opt->getData().size());
     EXPECT_EQ(2, opt->len()); // just v4 header
 
-    EXPECT_NO_THROW(
-        delete opt;
-    );
-    opt = 0;
+    EXPECT_NO_THROW(opt.reset());
 
     // V4 options have type 0...255
-    EXPECT_THROW(
-        opt = new Option(Option::V4, 256),
-        BadValue
-    );
-
-    delete opt;
-    opt = 0;
+    EXPECT_THROW(opt.reset(new Option(Option::V4, 256)), BadValue);
 
     // 0 is a special PAD option
-    EXPECT_THROW(
-        opt = new Option(Option::V4, 0),
-        BadValue
-    );
-
-    delete opt;
-    opt = 0;
+    EXPECT_THROW(opt.reset(new Option(Option::V4, 0)), BadValue);
 
     // 255 is a special END option
-    EXPECT_THROW(
-        opt = new Option(Option::V4, 255),
-        BadValue
-    );
-
-    delete opt;
-    opt = 0;
+    EXPECT_THROW(opt.reset(new Option(Option::V4, 255)), BadValue);
 }
 
 const uint8_t dummyPayload[] =
@@ -97,15 +76,12 @@ TEST_F(OptionTest, v4_data1) {
 
     vector<uint8_t> data(dummyPayload, dummyPayload + sizeof(dummyPayload));
 
-    Option* opt = 0;
+    scoped_ptr<Option> opt;
 
-    // create DHCPv4 option of type 123
-    // that contains 4 bytes of data
-    ASSERT_NO_THROW(
-        opt= new Option(Option::V4, 123, data);
-    );
+    // Create DHCPv4 option of type 123 that contains 4 bytes of data.
+    ASSERT_NO_THROW(opt.reset(new Option(Option::V4, 123, data)));
 
-    // check that content is reported properly
+    // Check that content is reported properly
     EXPECT_EQ(123, opt->getType());
     vector<uint8_t> optData = opt->getData();
     ASSERT_EQ(optData.size(), data.size());
@@ -113,31 +89,26 @@ TEST_F(OptionTest, v4_data1) {
     EXPECT_EQ(2, opt->getHeaderLen());
     EXPECT_EQ(6, opt->len());
 
-    // now store that option into a buffer
+    // Now store that option into a buffer
     OutputBuffer buf(100);
-    EXPECT_NO_THROW(
-        opt->pack(buf);
-    );
-
-    // check content of that buffer
+    EXPECT_NO_THROW(opt->pack(buf));
 
+    // Check content of that buffer:
     // 2 byte header + 4 bytes data
     ASSERT_EQ(6, buf.getLength());
 
-    // that's how this option is supposed to look like
+    // That's how this option is supposed to look like
     uint8_t exp[] = { 123, 4, 1, 2, 3, 4 };
 
     /// TODO: use vector<uint8_t> getData() when it will be implemented
     EXPECT_EQ(0, memcmp(exp, buf.getData(), 6));
 
-    // check that we can destroy that option
-    EXPECT_NO_THROW(
-        delete opt;
-    );
+    // Check that we can destroy that option
+    EXPECT_NO_THROW(opt.reset());
 }
 
-// this is almost the same test as v4_data1, but it uses
-// different constructor
+// This is almost the same test as v4_data1, but it uses a different
+// constructor
 TEST_F(OptionTest, v4_data2) {
 
     vector<uint8_t> data(dummyPayload, dummyPayload + sizeof(dummyPayload));
@@ -153,16 +124,16 @@ TEST_F(OptionTest, v4_data2) {
     // ignored, as we pass interators to proper data. Only subset (limited by
     // iterators) of the vector should be used.
     // expData contains expected content (just valid data, without garbage).
-
-    Option* opt = 0;
+    scoped_ptr<Option> opt;
 
     // Create DHCPv4 option of type 123 that contains
     // 4 bytes (sizeof(dummyPayload).
     ASSERT_NO_THROW(
-        opt= new Option(Option::V4, 123, data.begin() + 1, data.end() - 1);
+        opt.reset(new Option(Option::V4, 123, data.begin() + 1,
+                             data.end() - 1));
     );
 
-    // check that content is reported properly
+    // Check that content is reported properly
     EXPECT_EQ(123, opt->getType());
     vector<uint8_t> optData = opt->getData();
     ASSERT_EQ(optData.size(), expData.size());
@@ -170,27 +141,23 @@ TEST_F(OptionTest, v4_data2) {
     EXPECT_EQ(2, opt->getHeaderLen());
     EXPECT_EQ(6, opt->len());
 
-    // now store that option into a buffer
+    // Now store that option into a buffer
     OutputBuffer buf(100);
-    EXPECT_NO_THROW(
-        opt->pack(buf);
-    );
+    EXPECT_NO_THROW(opt->pack(buf));
 
-    // check content of that buffer
+    // Check content of that buffer
 
     // 2 byte header + 4 bytes data
     ASSERT_EQ(6, buf.getLength());
 
-    // that's how this option is supposed to look like
+    // That's how this option is supposed to look like
     uint8_t exp[] = { 123, 4, 1, 2, 3, 4 };
 
     /// TODO: use vector<uint8_t> getData() when it will be implemented
     EXPECT_EQ(0, memcmp(exp, buf.getData(), 6));
 
-    // check that we can destroy that option
-    EXPECT_NO_THROW(
-        delete opt;
-    );
+    // Check that we can destroy that option
+    EXPECT_NO_THROW(opt.reset());
 }
 
 TEST_F(OptionTest, v4_toText) {
@@ -205,29 +172,29 @@ TEST_F(OptionTest, v4_toText) {
     EXPECT_EQ("type=253, len=3: 00:0f:ff", opt.toText());
 }
 
-// tests simple constructor
+// Tests simple constructor
 TEST_F(OptionTest, v6_basic) {
 
-    Option* opt = new Option(Option::V6, 1);
+    scoped_ptr<Option> opt(new Option(Option::V6, 1));
 
     EXPECT_EQ(Option::V6, opt->getUniverse());
     EXPECT_EQ(1, opt->getType());
     EXPECT_EQ(0, opt->getData().size());
-    EXPECT_EQ(4, opt->len()); // just v6 header
+    EXPECT_EQ(4, opt->len()); // Just v6 header
 
-    EXPECT_NO_THROW(
-        delete opt;
-    );
+    EXPECT_NO_THROW(opt.reset());
 }
 
-// tests constructor used in pkt reception
-// option contains actual data
+// Tests constructor used in packet reception.  Option contains actual data
 TEST_F(OptionTest, v6_data1) {
-    for (int i = 0; i < 32; i++)
-        buf_[i] = 100+i;
-    Option* opt = new Option(Option::V6, 333,   //type
-                             buf_.begin() + 3,  // begin offset
-                             buf_.begin() + 10); // end offset (7 bytes of data)
+    for (int i = 0; i < 32; i++) {
+        buf_[i] = 100 + i;
+    }
+
+    // Create option with seven bytes of data.
+    scoped_ptr<Option> opt(new Option(Option::V6, 333,   // Type
+                                      buf_.begin() + 3,  // Begin offset
+                                      buf_.begin() + 10)); // End offset
     EXPECT_EQ(333, opt->getType());
 
     ASSERT_EQ(11, opt->len());
@@ -237,23 +204,20 @@ TEST_F(OptionTest, v6_data1) {
     opt->pack(outBuf_);
     EXPECT_EQ(11, outBuf_.getLength());
 
-    const uint8_t* out = (const uint8_t*)outBuf_.getData();
-    EXPECT_EQ(out[0], 333/256); // type
-    EXPECT_EQ(out[1], 333%256);
+    const uint8_t* out = static_cast<const uint8_t*>(outBuf_.getData());
+    EXPECT_EQ(out[0], 333 / 256); // Type
+    EXPECT_EQ(out[1], 333 % 256);
 
-    EXPECT_EQ(out[2], 0); // len
+    EXPECT_EQ(out[2], 0); // Length
     EXPECT_EQ(out[3], 7);
 
-    // payload
-    EXPECT_EQ(0, memcmp(&buf_[3], out+4, 7) );
+    // Payload
+    EXPECT_EQ(0, memcmp(&buf_[3], out + 4, 7));
 
-    EXPECT_NO_THROW(
-        delete opt;
-    );
+    EXPECT_NO_THROW(opt.reset());
 }
 
-// another text that tests the same thing, just
-// with different input parameters
+// Another test that tests the same thing, just with different input parameters.
 TEST_F(OptionTest, v6_data2) {
 
     buf_[0] = 0xa1;
@@ -261,13 +225,11 @@ TEST_F(OptionTest, v6_data2) {
     buf_[2] = 0xa3;
     buf_[3] = 0xa4;
 
-    // create an option (unpack content)
-    Option* opt = new Option(Option::V6,
-                             D6O_CLIENTID,
-                             buf_.begin(),
-                             buf_.begin() + 4);
+    // Create an option (unpack content)
+    scoped_ptr<Option> opt(new Option(Option::V6, D6O_CLIENTID,
+                                      buf_.begin(), buf_.begin() + 4));
 
-    // pack this option
+    // Pack this option
     opt->pack(outBuf_);
 
     // 4 bytes header + 4 bytes content
@@ -276,35 +238,35 @@ TEST_F(OptionTest, v6_data2) {
 
     EXPECT_EQ(8, outBuf_.getLength());
 
-    // check if pack worked properly:
-    // if option type is correct
-    const uint8_t* out = (const uint8_t*)outBuf_.getData();
+    // Check if pack worked properly:
+    // If option type is correct
+    const uint8_t* out = static_cast<const uint8_t*>(outBuf_.getData());
 
-    EXPECT_EQ(D6O_CLIENTID, out[0]*256 + out[1]);
+    EXPECT_EQ(D6O_CLIENTID, out[0] * 256 + out[1]);
 
-    // if option length is correct
-    EXPECT_EQ(4, out[2]*256 + out[3]);
+    // If option length is correct
+    EXPECT_EQ(4, out[2] * 256 + out[3]);
 
-    // if option content is correct
+    // If option content is correct
     EXPECT_EQ(0, memcmp(&buf_[0], out + 4, 4));
 
-    EXPECT_NO_THROW(
-        delete opt;
-    );
+    EXPECT_NO_THROW(opt.reset());
 }
 
-// check that an option can contain 2 suboptions:
+// Check that an option can contain 2 suboptions:
 // opt1
 //  +----opt2
 //  |
 //  +----opt3
 //
 TEST_F(OptionTest, v6_suboptions1) {
-    for (int i=0; i<128; i++)
-        buf_[i] = 100+i;
-    Option* opt1 = new Option(Option::V6, 65535, //type
-                              buf_.begin(), // 3 bytes of data
-                              buf_.begin() + 3);
+    for (int i = 0; i < 128; i++) {
+        buf_[i] = 100 + i;
+    }
+
+    scoped_ptr<Option> opt1(new Option(Option::V6, 65535, // Type
+                                       buf_.begin(), // 3 bytes of data
+                                       buf_.begin() + 3));
     OptionPtr opt2(new Option(Option::V6, 13));
     OptionPtr opt3(new Option(Option::V6, 7,
                               buf_.begin() + 3,
@@ -328,30 +290,29 @@ TEST_F(OptionTest, v6_suboptions1) {
     opt1->pack(outBuf_);
     EXPECT_EQ(20, outBuf_.getLength());
 
-    // payload
+    // Payload
     EXPECT_EQ(0, memcmp(outBuf_.getData(), expected, 20) );
 
-    EXPECT_NO_THROW(
-        delete opt1;
-    );
+    EXPECT_NO_THROW(opt1.reset());
 }
 
-// check that an option can contain nested suboptions:
+// Check that an option can contain nested suboptions:
 // opt1
 //  +----opt2
 //        |
 //        +----opt3
 //
 TEST_F(OptionTest, v6_suboptions2) {
-    for (int i=0; i<128; i++)
-        buf_[i] = 100+i;
-    Option* opt1 = new Option(Option::V6, 65535, //type
-                              buf_.begin(),
-                              buf_.begin() + 3);
+    for (int i = 0; i < 128; i++) {
+        buf_[i] = 100 + i;
+    }
+
+    scoped_ptr<Option> opt1(new Option(Option::V6, 65535, // Type
+                                       buf_.begin(), buf_.begin() + 3));
     OptionPtr opt2(new Option(Option::V6, 13));
     OptionPtr opt3(new Option(Option::V6, 7,
-                                              buf_.begin() + 3,
-                                              buf_.begin() + 8));
+                              buf_.begin() + 3,
+                              buf_.begin() + 8));
     opt1->addOption(opt2);
     opt2->addOption(opt3);
     // opt3 len = 9 4(header)+5(data)
@@ -367,18 +328,18 @@ TEST_F(OptionTest, v6_suboptions2) {
     opt1->pack(outBuf_);
     EXPECT_EQ(20, outBuf_.getLength());
 
-    // payload
+    // Payload
     EXPECT_EQ(0, memcmp(outBuf_.getData(), expected, 20) );
 
-    EXPECT_NO_THROW(
-        delete opt1;
-    );
+    EXPECT_NO_THROW(opt1.reset());
 }
 
 TEST_F(OptionTest, v6_addgetdel) {
-    for (int i=0; i<128; i++)
-        buf_[i] = 100+i;
-    Option* parent = new Option(Option::V6, 65535); //type
+    for (int i = 0; i < 128; i++) {
+        buf_[i] = 100 + i;
+    }
+
+    scoped_ptr<Option> parent(new Option(Option::V6, 65535)); // Type
     OptionPtr opt1(new Option(Option::V6, 1));
     OptionPtr opt2(new Option(Option::V6, 2));
     OptionPtr opt3(new Option(Option::V6, 2));
@@ -390,28 +351,26 @@ TEST_F(OptionTest, v6_addgetdel) {
     EXPECT_EQ(opt1, parent->getOption(1));
     EXPECT_EQ(opt2, parent->getOption(2));
 
-    // expect NULL
+    // Expect NULL
     EXPECT_EQ(OptionPtr(), parent->getOption(4));
 
-    // now there are 2 options of type 2
+    // Now there are 2 options of type 2
     parent->addOption(opt3);
 
-    // let's delete one of them
+    // Let's delete one of them
     EXPECT_EQ(true, parent->delOption(2));
 
-    // there still should be the other option 2
+    // There still should be the other option 2
     EXPECT_NE(OptionPtr(), parent->getOption(2));
 
-    // let's delete the other option 2
+    // Let's delete the other option 2
     EXPECT_EQ(true, parent->delOption(2));
 
-    // no more options with type=2
+    // No more options with type=2
     EXPECT_EQ(OptionPtr(), parent->getOption(2));
 
-    // let's try to delete - should fail
+    // Let's try to delete - should fail
     EXPECT_TRUE(false ==  parent->delOption(2));
-
-    delete parent;
 }
 
 TEST_F(OptionTest, v6_toText) {
@@ -419,8 +378,7 @@ TEST_F(OptionTest, v6_toText) {
     buf_[1] = 0xf;
     buf_[2] = 0xff;
 
-    OptionPtr opt(new Option(Option::V6, 258,  buf_.begin(), buf_.begin() + 3 ));
-
+    OptionPtr opt(new Option(Option::V6, 258, buf_.begin(), buf_.begin() + 3 ));
     EXPECT_EQ("type=258, len=3: 00:0f:ff", opt->toText());
 }
 
@@ -433,7 +391,7 @@ TEST_F(OptionTest, getUintX) {
     buf_[3] = 0x2;
     buf_[4] = 0x1;
 
-    // five options with varying lengths
+    // Five options with varying lengths
     OptionPtr opt1(new Option(Option::V6, 258, buf_.begin(), buf_.begin() + 1));
     OptionPtr opt2(new Option(Option::V6, 258, buf_.begin(), buf_.begin() + 2));
     OptionPtr opt3(new Option(Option::V6, 258, buf_.begin(), buf_.begin() + 3));
@@ -456,7 +414,7 @@ TEST_F(OptionTest, getUintX) {
     EXPECT_EQ(0x0504, opt4->getUint16());
     EXPECT_EQ(0x05040302, opt4->getUint32());
 
-    // the same as for 4-byte long, just get first 1,2 or 4 bytes
+    // The same as for 4-byte long, just get first 1,2 or 4 bytes
     EXPECT_EQ(5, opt5->getUint8());
     EXPECT_EQ(0x0504, opt5->getUint16());
     EXPECT_EQ(0x05040302, opt5->getUint32());
@@ -468,7 +426,7 @@ TEST_F(OptionTest, setUintX) {
     OptionPtr opt2(new Option(Option::V4, 125));
     OptionPtr opt4(new Option(Option::V4, 125));
 
-    // verify setUint8
+    // Verify setUint8
     opt1->setUint8(255);
     EXPECT_EQ(255, opt1->getUint8());
     opt1->pack(outBuf_);
@@ -477,7 +435,7 @@ TEST_F(OptionTest, setUintX) {
     uint8_t exp1[] = {125, 1, 255};
     EXPECT_TRUE(0 == memcmp(exp1, outBuf_.getData(), 3));
 
-    // verify getUint16
+    // Verify getUint16
     outBuf_.clear();
     opt2->setUint16(12345);
     opt2->pack(outBuf_);
@@ -487,7 +445,7 @@ TEST_F(OptionTest, setUintX) {
     uint8_t exp2[] = {125, 2, 12345/256, 12345%256};
     EXPECT_TRUE(0 == memcmp(exp2, outBuf_.getData(), 4));
 
-    // verify getUint32
+    // Verify getUint32
     outBuf_.clear();
     opt4->setUint32(0x12345678);
     opt4->pack(outBuf_);
@@ -499,8 +457,8 @@ TEST_F(OptionTest, setUintX) {
 }
 
 TEST_F(OptionTest, setData) {
-    // verify data override with new buffer larger than
-    // initial option buffer size
+    // Verify data override with new buffer larger than initial option buffer
+    // size.
     OptionPtr opt1(new Option(Option::V4, 125,
                               buf_.begin(), buf_.begin() + 10));
     buf_.resize(20, 1);
@@ -511,8 +469,8 @@ TEST_F(OptionTest, setData) {
     EXPECT_TRUE(0 == memcmp(&buf_[0], test_data + opt1->getHeaderLen(),
                             buf_.size()));
 
-    // verify data override with new buffer shorter than
-    // initial option buffer size
+    // Verify data override with new buffer shorter than initial option buffer
+    // size.
     OptionPtr opt2(new Option(Option::V4, 125,
                               buf_.begin(), buf_.begin() + 10));
     outBuf_.clear();
@@ -528,15 +486,15 @@ TEST_F(OptionTest, setData) {
 // This test verifies that options can be compared using equal() method.
 TEST_F(OptionTest, equal) {
 
-    // five options with varying lengths
+    // Five options with varying lengths
     OptionPtr opt1(new Option(Option::V6, 258, buf_.begin(), buf_.begin() + 1));
     OptionPtr opt2(new Option(Option::V6, 258, buf_.begin(), buf_.begin() + 2));
     OptionPtr opt3(new Option(Option::V6, 258, buf_.begin(), buf_.begin() + 3));
 
-    // the same content as opt2, but different type
+    // The same content as opt2, but different type
     OptionPtr opt4(new Option(Option::V6, 1, buf_.begin(), buf_.begin() + 2));
 
-    // another instance with the same type and content as opt2
+    // Another instance with the same type and content as opt2
     OptionPtr opt5(new Option(Option::V6, 258, buf_.begin(), buf_.begin() + 2));
 
     EXPECT_TRUE(opt1->equal(opt1));

+ 57 - 83
src/lib/dhcp/tests/pkt4_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2012  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2013  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -35,7 +35,7 @@ using namespace isc;
 using namespace isc::asiolink;
 using namespace isc::dhcp;
 using namespace isc::util;
-// don't import the entire boost namespace.  It will unexpectedly hide uint8_t
+// Don't import the entire boost namespace.  It will unexpectedly hide uint8_t
 // for some systems.
 using boost::scoped_ptr;
 
@@ -44,55 +44,39 @@ namespace {
 TEST(Pkt4Test, constructor) {
 
     ASSERT_EQ(236U, static_cast<size_t>(Pkt4::DHCPV4_PKT_HDR_LEN) );
-    Pkt4* pkt = 0;
+    scoped_ptr<Pkt4> pkt;
 
     // Just some dummy payload.
     uint8_t testData[250];
     for (int i = 0; i < 250; i++) {
-        testData[i]=i;
+        testData[i] = i;
     }
 
     // Positive case1. Normal received packet.
-    EXPECT_NO_THROW(
-        pkt = new Pkt4(testData, Pkt4::DHCPV4_PKT_HDR_LEN);
-    );
+    EXPECT_NO_THROW(pkt.reset(new Pkt4(testData, Pkt4::DHCPV4_PKT_HDR_LEN)));
 
     EXPECT_EQ(static_cast<size_t>(Pkt4::DHCPV4_PKT_HDR_LEN), pkt->len());
 
-    EXPECT_NO_THROW(
-        delete pkt;
-        pkt = 0;
-    );
+    EXPECT_NO_THROW(pkt.reset());
 
     // Positive case2. Normal outgoing packet.
-    EXPECT_NO_THROW(
-        pkt = new Pkt4(DHCPDISCOVER, 0xffffffff);
-    );
+    EXPECT_NO_THROW(pkt.reset(new Pkt4(DHCPDISCOVER, 0xffffffff)));
 
     // DHCPv4 packet must be at least 236 bytes long, with Message Type
     // Option taking extra 3 bytes it is 239
     EXPECT_EQ(static_cast<size_t>(Pkt4::DHCPV4_PKT_HDR_LEN) + 3, pkt->len());
     EXPECT_EQ(DHCPDISCOVER, pkt->getType());
     EXPECT_EQ(0xffffffff, pkt->getTransid());
-    EXPECT_NO_THROW(
-        delete pkt;
-        pkt = 0;
-    );
+    EXPECT_NO_THROW(pkt.reset());
 
     // Negative case. Should drop truncated messages.
     EXPECT_THROW(
-        pkt = new Pkt4(testData, Pkt4::DHCPV4_PKT_HDR_LEN-1),
+        pkt.reset(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.
-        delete pkt;
-        pkt = 0;
-    }
 }
 
-// a sample data
+// Sample data
 const uint8_t dummyOp = BOOTREQUEST;
 const uint8_t dummyHtype = 6;
 const uint8_t dummyHlen = 6;
@@ -109,16 +93,16 @@ const IOAddress dummyGiaddr("255.255.255.255");
 // a dummy MAC address
 const uint8_t dummyMacAddr[] = {0, 1, 2, 3, 4, 5};
 
-// a dummy MAC address, padded with 0s
+// A dummy MAC address, padded with 0s
 const uint8_t dummyChaddr[16] = {0, 1, 2, 3, 4, 5, 0, 0,
                                  0, 0, 0, 0, 0, 0, 0, 0 };
 
-// let's use some creative test content here (128 chars + \0)
+// Let's use some creative test content here (128 chars + \0)
 const uint8_t dummyFile[] = "Lorem ipsum dolor sit amet, consectetur "
     "adipiscing elit. Proin mollis placerat metus, at "
     "lacinia orci ornare vitae. Mauris amet.";
 
-// yet another type of test content (64 chars + \0)
+// Yet another type of test content (64 chars + \0)
 const uint8_t dummySname[] = "Lorem ipsum dolor sit amet, consectetur "
     "adipiscing elit posuere.";
 
@@ -127,12 +111,11 @@ BOOST_STATIC_ASSERT(sizeof(dummySname) == Pkt4::MAX_SNAME_LEN + 1);
 
 /// @brief Generates test packet.
 ///
-/// Allocates and generates test packet, with all fixed
-/// fields set to non-zero values. Content is not always
-/// reasonable.
+/// Allocates and generates test packet, with all fixed fields set to non-zero
+/// values. Content is not always reasonable.
 ///
-/// See generateTestPacket2() function that returns
-/// exactly the same packet in on-wire format.
+/// See generateTestPacket2() function that returns exactly the same packet in
+/// on-wire format.
 ///
 /// @return pointer to allocated Pkt4 object.
 boost::shared_ptr<Pkt4>
@@ -162,12 +145,11 @@ generateTestPacket1() {
 
 /// @brief Generates test packet.
 ///
-/// Allocates and generates on-wire buffer that represents
-/// test packet, with all fixed fields set to non-zero values.
-/// Content is not always reasonable.
+/// Allocates and generates on-wire buffer that represents test packet, with all
+/// fixed fields set to non-zero values.  Content is not always reasonable.
 ///
-/// See generateTestPacket1() function that returns
-/// exactly the same packet as Pkt4 object.
+/// See generateTestPacket1() function that returns exactly the same packet as
+/// Pkt4 object.
 ///
 /// @return pointer to allocated Pkt4 object
 // Returns a vector containing a DHCPv4 packet header.
@@ -206,7 +188,7 @@ TEST(Pkt4Test, fixedFields) {
 
     boost::shared_ptr<Pkt4> pkt = generateTestPacket1();
 
-    // ok, let's check packet values
+    // OK, let's check packet values
     EXPECT_EQ(dummyOp, pkt->getOp());
     EXPECT_EQ(dummyHtype, pkt->getHtype());
     EXPECT_EQ(dummyHlen, pkt->getHlen());
@@ -244,7 +226,7 @@ TEST(Pkt4Test, fixedFieldsPack) {
     // DHCP Message Type Option
     ASSERT_EQ(static_cast<size_t>(Pkt4::DHCPV4_PKT_HDR_LEN) + 3, pkt->len());
 
-    // redundant but MUCH easier for debug in gdb
+    // 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());
 
@@ -272,7 +254,7 @@ TEST(Pkt4Test, fixedFieldsUnpack) {
         pkt->unpack()
     );
 
-    // ok, let's check packet values
+    // OK, let's check packet values
     EXPECT_EQ(dummyOp, pkt->getOp());
     EXPECT_EQ(dummyHtype, pkt->getHtype());
     EXPECT_EQ(dummyHlen, pkt->getHlen());
@@ -298,7 +280,7 @@ TEST(Pkt4Test, fixedFieldsUnpack) {
     EXPECT_EQ(DHCPDISCOVER, pkt->getType());
 }
 
-// this test is for hardware addresses (htype, hlen and chaddr fields)
+// This test is for hardware addresses (htype, hlen and chaddr fields)
 TEST(Pkt4Test, hwAddr) {
 
     vector<uint8_t> mac;
@@ -309,7 +291,7 @@ TEST(Pkt4Test, hwAddr) {
     // (growing length back to MAX_CHADDR_LEN).
     mac.resize(Pkt4::MAX_CHADDR_LEN);
 
-    Pkt4* pkt = 0;
+    scoped_ptr<Pkt4> pkt;
     // 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++) {
@@ -322,7 +304,7 @@ TEST(Pkt4Test, hwAddr) {
         }
 
         // type and transaction doesn't matter in this test
-        pkt = new Pkt4(DHCPOFFER, 1234);
+        pkt.reset(new Pkt4(DHCPOFFER, 1234));
         pkt->setHWAddr(255-macLen*10, // just weird htype
                        macLen,
                        mac);
@@ -339,7 +321,7 @@ TEST(Pkt4Test, hwAddr) {
 
         EXPECT_EQ(0, memcmp(ptr, expectedChaddr, Pkt4::MAX_CHADDR_LEN));
 
-        delete pkt;
+        pkt.reset();
     }
 
     /// TODO: extend this test once options support is implemented. HW address
@@ -368,35 +350,28 @@ TEST(Pkt4Test, msgTypes) {
         {DHCPLEASEACTIVE, BOOTREPLY}
     };
 
-    Pkt4* pkt = 0;
+    scoped_ptr<Pkt4> pkt;
     for (int i = 0; i < sizeof(types) / sizeof(msgType); i++) {
-
-        pkt = new Pkt4(types[i].dhcp, 0);
+        pkt.reset(new Pkt4(types[i].dhcp, 0));
         EXPECT_EQ(types[i].dhcp, pkt->getType());
-
         EXPECT_EQ(types[i].bootp, pkt->getOp());
-
-        delete pkt;
-        pkt = 0;
+        pkt.reset();
     }
 
     EXPECT_THROW(
-        pkt = new Pkt4(100, 0), // there's no message type 100
+        pkt.reset(new Pkt4(100, 0)), // There's no message type 100
         OutOfRange
     );
-    if (pkt) {
-        delete pkt;
-    }
 }
 
-// this test verifies handling of sname field
+// This test verifies handling of sname field
 TEST(Pkt4Test, sname) {
 
     uint8_t sname[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++) {
+    scoped_ptr<Pkt4> pkt;
+    // 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++) {
             sname[i] = 0;
         }
@@ -404,8 +379,8 @@ TEST(Pkt4Test, sname) {
             sname[i] = i;
         }
 
-        // type and transaction doesn't matter in this test
-        pkt = new Pkt4(DHCPOFFER, 1234);
+        // Type and transaction doesn't matter in this test
+        pkt.reset(new Pkt4(DHCPOFFER, 1234));
         pkt->setSname(sname, snameLen);
 
         EXPECT_EQ(0, memcmp(sname, &pkt->getSname()[0], Pkt4::MAX_SNAME_LEN));
@@ -419,7 +394,7 @@ TEST(Pkt4Test, sname) {
             static_cast<const uint8_t*>(pkt->getBuffer().getData())+44;
         EXPECT_EQ(0, memcmp(ptr, sname, Pkt4::MAX_SNAME_LEN));
 
-        delete pkt;
+        pkt.reset();
     }
 
     // Check that a null argument generates an exception.
@@ -432,7 +407,7 @@ TEST(Pkt4Test, file) {
 
     uint8_t file[Pkt4::MAX_FILE_LEN];
 
-    Pkt4* pkt = 0;
+    scoped_ptr<Pkt4> pkt;
     // 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++) {
@@ -443,12 +418,11 @@ TEST(Pkt4Test, file) {
         }
 
         // Type and transaction doesn't matter in this test.
-        pkt = new Pkt4(DHCPOFFER, 1234);
+        pkt.reset(new Pkt4(DHCPOFFER, 1234));
         pkt->setFile(file, fileLen);
 
         EXPECT_EQ(0, memcmp(file, &pkt->getFile()[0], Pkt4::MAX_FILE_LEN));
 
-        //
         EXPECT_NO_THROW(
             pkt->pack();
         );
@@ -458,7 +432,7 @@ TEST(Pkt4Test, file) {
             static_cast<const uint8_t*>(pkt->getBuffer().getData())+108;
         EXPECT_EQ(0, memcmp(ptr, file, Pkt4::MAX_FILE_LEN));
 
-        delete pkt;
+        pkt.reset();
     }
 
     // Check that a null argument generates an exception.
@@ -481,7 +455,7 @@ static uint8_t v4Opts[] = {
 };
 
 TEST(Pkt4Test, options) {
-    Pkt4* pkt = new Pkt4(DHCPOFFER, 0);
+    scoped_ptr<Pkt4> pkt(new Pkt4(DHCPOFFER, 0));
 
     vector<uint8_t> payload[5];
     for (int i = 0; i < 5; i++) {
@@ -509,7 +483,7 @@ TEST(Pkt4Test, options) {
     EXPECT_TRUE(pkt->getOption(254));
     EXPECT_FALSE(pkt->getOption(127)); //  no such option
 
-    // options are unique in DHCPv4. It should not be possible
+    // Options are unique in DHCPv4. It should not be possible
     // to add more than one option of the same type.
     EXPECT_THROW(
         pkt->addOption(opt1),
@@ -521,26 +495,28 @@ TEST(Pkt4Test, options) {
     );
 
     const OutputBuffer& buf = pkt->getBuffer();
-    // check that all options are stored, they should take sizeof(v4Opts),
+    // Check that all options are stored, they should take sizeof(v4Opts),
     // DHCP magic cookie (4 bytes), and OPTION_END added (just one byte)
-    ASSERT_EQ(static_cast<size_t>(Pkt4::DHCPV4_PKT_HDR_LEN) + sizeof(DHCP_OPTIONS_COOKIE)
-              + sizeof(v4Opts) + 1, buf.getLength());
+    ASSERT_EQ(static_cast<size_t>(Pkt4::DHCPV4_PKT_HDR_LEN) +
+              sizeof(DHCP_OPTIONS_COOKIE) + sizeof(v4Opts) + 1,
+              buf.getLength());
 
-    // that that this extra data actually contain our options
+    // That that this extra data actually contain our options
     const uint8_t* ptr = static_cast<const uint8_t*>(buf.getData());
-    ptr += Pkt4::DHCPV4_PKT_HDR_LEN + sizeof(DHCP_OPTIONS_COOKIE); // rewind to end of fixed part
+
+    // Rewind to end of fixed part.
+    ptr += Pkt4::DHCPV4_PKT_HDR_LEN + sizeof(DHCP_OPTIONS_COOKIE);
+
     EXPECT_EQ(0, memcmp(ptr, v4Opts, sizeof(v4Opts)));
     EXPECT_EQ(DHO_END, static_cast<uint8_t>(*(ptr + sizeof(v4Opts))));
 
     // delOption() checks
-    EXPECT_TRUE(pkt->getOption(12)); // Sanity check: option 12 is still there
-    EXPECT_TRUE(pkt->delOption(12)); // We should be able to remove it
+    EXPECT_TRUE(pkt->getOption(12));  // Sanity check: option 12 is still there
+    EXPECT_TRUE(pkt->delOption(12));  // We should be able to remove it
     EXPECT_FALSE(pkt->getOption(12)); // It should not be there anymore
     EXPECT_FALSE(pkt->delOption(12)); // And removal should fail
 
-    EXPECT_NO_THROW(
-        delete pkt;
-    );
+    EXPECT_NO_THROW(pkt.reset());
 }
 
 TEST(Pkt4Test, unpackOptions) {
@@ -611,7 +587,7 @@ TEST(Pkt4Test, unpackOptions) {
 // i.e. fields that are not part of DHCPv4 (e.g. interface name).
 TEST(Pkt4Test, metaFields) {
 
-    Pkt4* pkt = new Pkt4(DHCPOFFER, 1234);
+    scoped_ptr<Pkt4> pkt(new Pkt4(DHCPOFFER, 1234));
     pkt->setIface("loooopback");
     pkt->setIndex(42);
     pkt->setRemoteAddr(IOAddress("1.2.3.4"));
@@ -621,8 +597,6 @@ TEST(Pkt4Test, metaFields) {
     EXPECT_EQ(42, pkt->getIndex());
     EXPECT_EQ("1.2.3.4", pkt->getRemoteAddr().toText());
     EXPECT_EQ("4.3.2.1", pkt->getLocalAddr().toText());
-
-    delete pkt;
 }
 
 TEST(Pkt4Test, Timestamp) {

+ 46 - 52
src/lib/dhcp/tests/pkt6_unittest.cc

@@ -37,6 +37,7 @@ using namespace std;
 using namespace isc;
 using namespace isc::asiolink;
 using namespace isc::dhcp;
+using boost::scoped_ptr;
 
 namespace {
 // empty class for now, but may be extended once Addr6 becomes bigger
@@ -48,12 +49,10 @@ public:
 
 TEST_F(Pkt6Test, constructor) {
     uint8_t data[] = { 0, 1, 2, 3, 4, 5 };
-    Pkt6 * pkt1 = new Pkt6(data, sizeof(data) );
+    scoped_ptr<Pkt6> pkt1(new Pkt6(data, sizeof(data)));
 
     EXPECT_EQ(6, pkt1->getData().size());
     EXPECT_EQ(0, memcmp( &pkt1->getData()[0], data, sizeof(data)));
-
-    delete pkt1;
 }
 
 /// @brief returns captured actual SOLICIT packet
@@ -168,39 +167,36 @@ Pkt6* capture2() {
 }
 
 TEST_F(Pkt6Test, unpack_solicit1) {
-    Pkt6* sol = capture1();
+    scoped_ptr<Pkt6> sol(capture1());
 
     ASSERT_EQ(true, sol->unpack());
 
-    // check for length
+    // Check for length
     EXPECT_EQ(98, sol->len() );
 
-    // check for type
+    // Check for type
     EXPECT_EQ(DHCPV6_SOLICIT, sol->getType() );
 
-    // check that all present options are returned
+    // Check that all present options are returned
     EXPECT_TRUE(sol->getOption(D6O_CLIENTID)); // client-id is present
     EXPECT_TRUE(sol->getOption(D6O_IA_NA));    // IA_NA is present
     EXPECT_TRUE(sol->getOption(D6O_ELAPSED_TIME));  // elapsed is present
     EXPECT_TRUE(sol->getOption(D6O_NAME_SERVERS));
     EXPECT_TRUE(sol->getOption(D6O_ORO));
 
-    // let's check that non-present options are not returned
+    // Let's check that non-present options are not returned
     EXPECT_FALSE(sol->getOption(D6O_SERVERID)); // server-id is missing
     EXPECT_FALSE(sol->getOption(D6O_IA_TA));
     EXPECT_FALSE(sol->getOption(D6O_IAADDR));
-
-    delete sol;
 }
 
 TEST_F(Pkt6Test, packUnpack) {
-
-    Pkt6* parent = new Pkt6(DHCPV6_SOLICIT, 0x020304);
+    scoped_ptr<Pkt6> parent(new Pkt6(DHCPV6_SOLICIT, 0x020304));
 
     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
+    // Let's not use zero-length option type 3 as it is IA_NA
 
     parent->addOption(opt1);
     parent->addOption(opt2);
@@ -208,7 +204,7 @@ TEST_F(Pkt6Test, packUnpack) {
 
     EXPECT_EQ(DHCPV6_SOLICIT, parent->getType());
 
-    // calculated length should be 16
+    // Calculated length should be 16
     EXPECT_EQ(Pkt6::DHCPV6_PKT_HDR_LEN + 3 * Option::OPTION6_HDR_LEN,
               parent->len());
 
@@ -217,30 +213,28 @@ TEST_F(Pkt6Test, packUnpack) {
     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(static_cast<const uint8_t*>(parent->getBuffer().getData()),
-                           parent->getBuffer().getLength());
+    // Create second packet,based on assembled data from the first one
+    scoped_ptr<Pkt6> clone(new Pkt6(
+        static_cast<const uint8_t*>(parent->getBuffer().getData()),
+                                    parent->getBuffer().getLength()));
 
-    // now recreate options list
+    // Now recreate options list
     EXPECT_TRUE( clone->unpack() );
 
     // transid, message-type should be the same as before
     EXPECT_EQ(parent->getTransid(), parent->getTransid());
     EXPECT_EQ(DHCPV6_SOLICIT, clone->getType());
 
-    EXPECT_TRUE( clone->getOption(1));
-    EXPECT_TRUE( clone->getOption(2));
-    EXPECT_TRUE( clone->getOption(100));
-    EXPECT_FALSE( clone->getOption(4));
-
-    delete parent;
-    delete clone;
+    EXPECT_TRUE(clone->getOption(1));
+    EXPECT_TRUE(clone->getOption(2));
+    EXPECT_TRUE(clone->getOption(100));
+    EXPECT_FALSE(clone->getOption(4));
 }
 
 // This test verifies that options can be added (addOption()), retrieved
 // (getOption(), getOptions()) and deleted (delOption()).
 TEST_F(Pkt6Test, addGetDelOptions) {
-    Pkt6* parent = new Pkt6(DHCPV6_SOLICIT, random() );
+    scoped_ptr<Pkt6> parent(new Pkt6(DHCPV6_SOLICIT, random()));
 
     OptionPtr opt1(new Option(Option::V6, 1));
     OptionPtr opt2(new Option(Option::V6, 2));
@@ -253,16 +247,16 @@ TEST_F(Pkt6Test, addGetDelOptions) {
     EXPECT_EQ(opt1, parent->getOption(1));
     EXPECT_EQ(opt2, parent->getOption(2));
 
-    // expect NULL
+    // Expect NULL
     EXPECT_EQ(OptionPtr(), parent->getOption(4));
 
-    // now there are 2 options of type 2
+    // Now there are 2 options of type 2
     parent->addOption(opt3);
 
     Option::OptionCollection options = parent->getOptions(2);
     EXPECT_EQ(2, options.size()); // there should be 2 instances
 
-    // both options must be of type 2 and there must not be
+    // Both options must be of type 2 and there must not be
     // any other type returned
     for (Option::OptionCollection::const_iterator x= options.begin();
          x != options.end(); ++x) {
@@ -278,26 +272,24 @@ TEST_F(Pkt6Test, addGetDelOptions) {
     EXPECT_EQ(1, (*options.begin()).second->getType());
     EXPECT_EQ(opt1, options.begin()->second);
 
-    // let's delete one of them
+    // Let's delete one of them
     EXPECT_EQ(true, parent->delOption(2));
 
-    // there still should be the other option 2
+    // There still should be the other option 2
     EXPECT_NE(OptionPtr(), parent->getOption(2));
 
-    // let's delete the other option 2
+    // Let's delete the other option 2
     EXPECT_EQ(true, parent->delOption(2));
 
-    // no more options with type=2
+    // No more options with type=2
     EXPECT_EQ(OptionPtr(), parent->getOption(2));
 
-    // let's try to delete - should fail
+    // Let's try to delete - should fail
     EXPECT_TRUE(false ==  parent->delOption(2));
 
     // Finally try to get a non-existent option
     options = parent->getOptions(1234);
     EXPECT_EQ(0, options.size());
-
-    delete parent;
 }
 
 TEST_F(Pkt6Test, Timestamp) {
@@ -388,7 +380,7 @@ TEST_F(Pkt6Test, relayUnpack) {
 
     OptionPtr opt;
 
-    // part 1: Check options inserted by the first relay
+    // Part 1: Check options inserted by the first relay
 
     // There should be 2 options in first relay
     EXPECT_EQ(2, msg->relay_info_[0].options_.size());
@@ -401,7 +393,7 @@ TEST_F(Pkt6Test, relayUnpack) {
     // That's a strange interface-id, but this is a real life example
     EXPECT_TRUE(0 == memcmp("ISAM144|299|ipv6|nt:vp:1:110", &data[0], 28));
 
-    // get the remote-id option
+    // Get the remote-id option
     ASSERT_TRUE(opt = msg->getRelayOption(D6O_REMOTE_ID, 0));
     EXPECT_EQ(22, opt->len()); // 18 bytes of data + 4 bytes header
     boost::shared_ptr<OptionCustom> custom = boost::dynamic_pointer_cast<OptionCustom>(opt);
@@ -415,16 +407,16 @@ TEST_F(Pkt6Test, relayUnpack) {
     ASSERT_EQ(sizeof(expected_remote_id), remote_id.size());
     ASSERT_EQ(0, memcmp(expected_remote_id, &remote_id[0], remote_id.size()));
 
-    // part 2: Check options inserted by the second relay
+    // Part 2: Check options inserted by the second relay
 
-    // get the interface-id from the second relay
+    // Get the interface-id from the second relay
     ASSERT_TRUE(opt = msg->getRelayOption(D6O_INTERFACE_ID, 1));
     data = opt->getData();
     EXPECT_EQ(25, opt->len()); // 21 bytes + 4 bytes header
     EXPECT_EQ(data.size(), 21);
     EXPECT_TRUE(0 == memcmp("ISAM144 eth 1/1/05/01", &data[0], 21));
 
-    // get the remote-id option
+    // Get the remote-id option
     ASSERT_TRUE(opt = msg->getRelayOption(D6O_REMOTE_ID, 1));
     EXPECT_EQ(8, opt->len());
     custom = boost::dynamic_pointer_cast<OptionCustom>(opt);
@@ -481,7 +473,7 @@ TEST_F(Pkt6Test, relayUnpack) {
 // packed and then unpacked.
 TEST_F(Pkt6Test, relayPack) {
 
-    boost::scoped_ptr<Pkt6> parent(new Pkt6(DHCPV6_ADVERTISE, 0x020304));
+    scoped_ptr<Pkt6> parent(new Pkt6(DHCPV6_ADVERTISE, 0x020304));
 
     Pkt6::RelayInfo relay1;
     relay1.msg_type_ = DHCPV6_RELAY_REPL;
@@ -490,16 +482,18 @@ TEST_F(Pkt6Test, relayPack) {
     relay1.peeraddr_ = IOAddress("fe80::abcd");
 
     uint8_t relay_opt_data[] = { 1, 2, 3, 4, 5, 6, 7, 8};
-    vector<uint8_t> relay_data(relay_opt_data, relay_opt_data + sizeof(relay_opt_data));
+    vector<uint8_t> relay_data(relay_opt_data,
+                               relay_opt_data + sizeof(relay_opt_data));
 
     OptionPtr optRelay1(new Option(Option::V6, 200, relay_data));
 
-    relay1.options_.insert(pair<int, boost::shared_ptr<Option> >(optRelay1->getType(), optRelay1));
+    relay1.options_.insert(pair<int, boost::shared_ptr<Option> >(
+                           optRelay1->getType(), optRelay1));
 
     OptionPtr opt1(new Option(Option::V6, 100));
     OptionPtr opt2(new Option(Option::V6, 101));
     OptionPtr opt3(new Option(Option::V6, 102));
-    // let's not use zero-length option type 3 as it is IA_NA
+    // Let's not use zero-length option type 3 as it is IA_NA
 
     parent->addRelayInfo(relay1);
 
@@ -512,17 +506,17 @@ TEST_F(Pkt6Test, relayPack) {
     EXPECT_TRUE(parent->pack());
 
     EXPECT_EQ(Pkt6::DHCPV6_PKT_HDR_LEN + 3 * Option::OPTION6_HDR_LEN // ADVERTISE
-              + Pkt6::DHCPV6_RELAY_HDR_LEN // relay header
-              + Option::OPTION6_HDR_LEN // relay-msg
+              + Pkt6::DHCPV6_RELAY_HDR_LEN // Relay header
+              + Option::OPTION6_HDR_LEN // Relay-msg
               + optRelay1->len(),
               parent->len());
 
-    // create second packet,based on assembled data from the first one
-    boost::scoped_ptr<Pkt6> clone(new Pkt6(static_cast<const uint8_t*>(
-                                           parent->getBuffer().getData()),
-                                           parent->getBuffer().getLength()));
+    // Create second packet,based on assembled data from the first one
+    scoped_ptr<Pkt6> clone(new Pkt6(static_cast<const uint8_t*>(
+                                    parent->getBuffer().getData()),
+                                    parent->getBuffer().getLength()));
 
-    // now recreate options list
+    // Now recreate options list
     EXPECT_TRUE( clone->unpack() );
 
     // transid, message-type should be the same as before