[master] Merge branch 'trac2275'

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,10 +107,12 @@ internal::RequestCheckCreator::create(const string& name,
 
 RequestLoader&
 getRequestLoader() {
-    static RequestLoader* loader(NULL);
-    if (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 an auto pointer in order to provide the strong exception
+        // in a second auto pointer in order to provide the strong exception
         // guarantee.
         auto_ptr<RequestLoader> loader_ptr =
             auto_ptr<RequestLoader>(new RequestLoader(REJECT));
@@ -129,7 +132,7 @@ getRequestLoader() {
                 new LogicCreator<AllOfSpec, RequestContext>("ALL")));
 
         // From this point there shouldn't be any exception thrown
-        loader = loader_ptr.release();
+        loader.reset(loader_ptr.release());
     }
 
     return (*loader);

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,8 +692,10 @@ 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));
 }
 
 TEST_F(SQLite3AccessorTest, clone) {

src/lib/dns/tests/labelsequence_unittest.cc

@@ -860,8 +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
-    const uint8_t offsets_toolonglabel[] = { 2, 0, 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,14 +76,17 @@ using namespace std;
 /// directed to one or other of the "servers" in the RecursiveQueryTest2 class,
 /// regardless of the glue returned in referrals.
 
+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* const WWW_EXAMPLE_ORG = "192.0.2.254";
+                                              ///< Address of www.example.org
+} //end anonymous namespace
+
 namespace isc {
 namespace asiodns {
 
-const std::string 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
-
 // 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
 // enable them.

src/lib/resolve/tests/recursive_query_unittest_3.cc

@@ -69,15 +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 {
-
-const std::string TEST_ADDRESS3 = "127.0.0.1"; 
-                                               ///< Servers are on this address
+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 size_t BUFFER_SIZE = 1024;               ///< For all buffers
 
-const std::string DUMMY_ADDR3 = "1.2.3.4";     ///< address to return as A
+const char* const DUMMY_ADDR3 = "1.2.3.4";     ///< address to return as A
+} // end anonymous namespace
+
+namespace isc {
+namespace asiodns {
 
 class MockResolver3 : public isc::resolve::ResolverInterface {
 public:

src/lib/server_common/tests/socket_requestor_test.cc

@@ -410,7 +410,8 @@ private:
             isc_throw(Unexpected,
                       "mkstemp() created a filename too long for sun_path");
         }
-        strncpy(socket_address.sun_path, path_, len);
+        strncpy(socket_address.sun_path, path_,
+                sizeof(socket_address.sun_path));
 #ifdef HAVE_SA_LEN
         socket_address.sun_len = len;
 #endif
@@ -542,7 +543,6 @@ TEST_F(SocketRequestorTest, testSocketPassing) {
         EXPECT_EQ("foo", socket_id.second);
         EXPECT_EQ(0, close(socket_id.first));
     }
-
     // Create a second socket server, to test that multiple different
     // domains sockets would work as well (even though we don't actually
     // use that feature)

src/lib/util/io/fd_share.cc

@@ -142,6 +142,7 @@ send_fd(const int sock, const int fd) {
     if (msghdr.msg_control == NULL) {
         return (FD_OTHER_ERROR);
     }
+    memset(msghdr.msg_control, 0, msghdr.msg_controllen);
 
     struct cmsghdr* cmsg = CMSG_FIRSTHDR(&msghdr);
     cmsg->cmsg_len = cmsg_len(sizeof(int));

src/lib/util/tests/fd_tests.cc

@@ -34,7 +34,9 @@ class FDTest : public ::testing::Test {
             // We do not care what is inside, we just need it to be the same
             data(new unsigned char[TEST_DATA_SIZE]),
             buffer(NULL)
-        { }
+        {
+            memset(data, 0, TEST_DATA_SIZE);
+        }
         ~ FDTest() {
             delete[] data;
             delete[] buffer;