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

Also tidy up a few comments.

1 files changed, 111 insertions, 153 deletions

diff --git a/src/lib/dhcp/tests/option_unittest.cc b/src/lib/dhcp/tests/option_unittest.cc
index 2ce3bc6a37..9442ce58d4 100644
--- a/src/lib/dhcp/tests/option_unittest.cc
+++ b/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));