[2275] address review comments

src/lib/acl/dns.cc

@@ -12,12 +12,6 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <memory>
-#include <string>
-#include <vector>
-
-#include <boost/shared_ptr.hpp>
-
 #include <exceptions/exceptions.h>
 
 #include <dns/name.h>
@@ -31,6 +25,13 @@
 #include <acl/loader.h>
 #include <acl/logic_check.h>
 
+#include <boost/shared_ptr.hpp>
+#include <boost/scoped_ptr.hpp>
+
+#include <memory>
+#include <string>
+#include <vector>
+
 using namespace std;
 using namespace isc::dns;
 using namespace isc::data;
@@ -106,7 +107,9 @@ internal::RequestCheckCreator::create(const string& name,
 
 RequestLoader&
 getRequestLoader() {
-    static auto_ptr<RequestLoader> loader(NULL);
+    // To ensure that the singleton gets destroyed at the end of the
+    // program's lifetime, we put it in a static scoped_ptr.
+    static boost::scoped_ptr<RequestLoader> loader(NULL);
     if (loader.get() == NULL) {
         // Creator registration may throw, so we first store the new loader
         // in a second auto pointer in order to provide the strong exception

src/lib/datasrc/tests/sqlite3_accessor_unittest.cc

@@ -665,16 +665,16 @@ TEST_F(SQLite3Create, creationtest) {
 TEST_F(SQLite3Create, emptytest) {
     ASSERT_FALSE(isReadable(SQLITE_NEW_DBFILE));
 
-    // open one manualle
+    // open one manually
     sqlite3* db;
     ASSERT_EQ(SQLITE_OK, sqlite3_open(SQLITE_NEW_DBFILE, &db));
 
-    // empty, but not locked, so creating it now should work
+    // empty, but not locked, so creating another accessor should work
     SQLite3Accessor accessor2(SQLITE_NEW_DBFILE, "IN");
 
     sqlite3_close(db);
 
-    // should work now that we closed it
+    // should still work now that we closed it
     SQLite3Accessor accessor3(SQLITE_NEW_DBFILE, "IN");
 }
 
@@ -692,7 +692,7 @@ TEST_F(SQLite3Create, lockedtest) {
 
     sqlite3_exec(db, "ROLLBACK TRANSACTION", NULL, NULL, NULL);
 
-    // should work now that we closed it
+    // should work now that the transaction has been rolled back
     SQLite3Accessor accessor3(SQLITE_NEW_DBFILE, "IN");
 
     ASSERT_EQ(SQLITE_OK, sqlite3_close(db));

src/lib/dns/tests/labelsequence_unittest.cc

@@ -860,12 +860,12 @@ TEST_F(LabelSequenceTest, badDeserialize) {
     const uint8_t toomany_offsets[] = { Name::MAX_LABELS + 1 };
     EXPECT_THROW(LabelSequence ls(toomany_offsets), isc::BadValue);
 
-    // exceed MAX_LABEL_LEN
-    uint8_t offsets_toolonglabel[LabelSequence::MAX_SERIALIZED_LENGTH];
-    memset(&offsets_toolonglabel, 0, LabelSequence::MAX_SERIALIZED_LENGTH);
-    offsets_toolonglabel[0] = 2;
-    offsets_toolonglabel[1] = 0;
-    offsets_toolonglabel[2] = 64;
+    // (second) offset does not match actual label length
+    const uint8_t offsets_wrongoffset[] = { 2, 0, 64, 1 };
+    EXPECT_THROW(LabelSequence ls(offsets_wrongoffset), isc::BadValue);
+
+    // offset matches, but exceeds MAX_LABEL_LEN
+    const uint8_t offsets_toolonglabel[] = { 2, 0, 64, 64 };
     EXPECT_THROW(LabelSequence ls(offsets_toolonglabel), isc::BadValue);
 
     // Inconsistent data: an offset is lower than the previous offset

src/lib/resolve/tests/recursive_query_unittest_2.cc

@@ -76,13 +76,16 @@ using namespace std;
 /// directed to one or other of the "servers" in the RecursiveQueryTest2 class,
 /// regardless of the glue returned in referrals.
 
-namespace isc {
-namespace asiodns {
-
-const char* TEST_ADDRESS = "127.0.0.1";       ///< Servers are on this address
+namespace {
+const char* const TEST_ADDRESS = "127.0.0.1"; ///< Servers are on this address
 const uint16_t TEST_PORT = 5301;              ///< ... and this port
 const size_t BUFFER_SIZE = 1024;              ///< For all buffers
-const char* WWW_EXAMPLE_ORG = "192.0.2.254";  ///< Address of www.example.org
+const char* const WWW_EXAMPLE_ORG = "192.0.2.254";
+                                              ///< Address of www.example.org
+} //end anonymous namespace
+
+namespace isc {
+namespace asiodns {
 
 // As the test is fairly long and complex, debugging "print" statements have
 // been left in although they are disabled.  Set the following to "true" to

src/lib/resolve/tests/recursive_query_unittest_3.cc

@@ -69,14 +69,16 @@ using namespace std;
 /// By using the "test_server_" element of RecursiveQuery, all queries are
 /// directed to one or other of the "servers" in the RecursiveQueryTest3 class.
 
-namespace isc {
-namespace asiodns {
+namespace {
+const char* const TEST_ADDRESS3 = "127.0.0.1"; ///< Servers are on this address
+const uint16_t TEST_PORT3 = 5303;              ///< ... and this port
+const size_t BUFFER_SIZE = 1024;               ///< For all buffers
 
-const char* TEST_ADDRESS3 = "127.0.0.1";      ///< Servers are on this address
-const uint16_t TEST_PORT3 = 5303;             ///< ... and this port
-const size_t BUFFER_SIZE = 1024;              ///< For all buffers
+const char* const DUMMY_ADDR3 = "1.2.3.4";     ///< address to return as A
+} // end anonymous namespace
 
-const char* DUMMY_ADDR3 = "1.2.3.4";          ///< address to return as A
+namespace isc {
+namespace asiodns {
 
 class MockResolver3 : public isc::resolve::ResolverInterface {
 public: