Merge branch 'trac613'

Makefile.am

@@ -70,6 +70,13 @@ clean-coverage: clean-cpp-coverage clean-python-coverage
 
 report-coverage: report-cpp-coverage report-python-coverage
 
+# for static C++ check using cppcheck (when available)
+cppcheck:
+	cppcheck --enable=all --suppressions src/cppcheck-suppress.lst \
+		--quiet --error-exitcode=1 \
+		--template '{file}:{line}: check_fail: {message} ({severity},{id})' \
+		src
+
 #### include external sources in the distributed tarball:
 EXTRA_DIST = ext/asio/README
 EXTRA_DIST += ext/asio/asio/local/stream_protocol.hpp

src/bin/auth/config.cc

@@ -271,7 +271,7 @@ configureAuthServer(AuthSrv& server, ConstElementPtr config_set) {
             parsers.push_back(parser);
         }
     } catch (const AuthConfigError& ex) {
-        throw ex;                  // simply rethrowing it
+        throw;                  // simply rethrowing it
     } catch (const isc::Exception& ex) {
         isc_throw(AuthConfigError, "Server configuration failed: " <<
                   ex.what());

src/bin/host/host.cc

@@ -70,12 +70,15 @@ host_lookup(const char* const name, const char* const type) {
     msg.toWire(renderer);
 
     struct addrinfo hints, *res;
-    int e;
     memset(&hints, 0, sizeof(hints));
     hints.ai_family = AF_UNSPEC;
     hints.ai_socktype = SOCK_DGRAM;
     hints.ai_flags = 0; // not using AI_NUMERICHOST in case to bootstrap
-    e = getaddrinfo(server, server_port, &hints, &res);
+    if (getaddrinfo(server, server_port, &hints, &res) != 0) {
+        cerr << "address/port conversion for " << server << ":"
+             << server_port << " failed" << endl;
+        return (1);
+    }
 
     if (verbose) {
         cout << "Trying \"" << name << "\"\n";

src/cppcheck-suppress.lst

@@ -0,0 +1,15 @@
+// On some systems cppcheck produces false alarms about 'missing includes'.
+// the following two will suppress, depending on the cppcheck version
+debug
+missingInclude
+// This is a template, and should be excluded from the check
+unreadVariable:src/lib/dns/rdata/template.cc:59
+// These two trigger warnings due to the incomplete implementation.  This is
+// our problem, but we need to suppress the warnings for now.
+functionConst:src/lib/cache/resolver_cache.h
+functionConst:src/lib/cache/message_cache.h
+functionConst:src/lib/cache/rrset_cache.h
+// Intentional self assignment tests.  Suppress warning about them.
+selfAssignment:src/lib/dns/tests/name_unittest.cc:292
+selfAssignment:src/lib/dns/tests/rdata_unittest.cc:227
+selfAssignment:src/lib/dns/tests/tsigkey_unittest.cc:104

src/lib/asiolink/tests/interval_timer_unittest.cc

@@ -33,7 +33,9 @@ using namespace asiolink;
 // or not.
 class IntervalTimerTest : public ::testing::Test {
 protected:
-    IntervalTimerTest() : io_service_() {}
+    IntervalTimerTest() :
+        io_service_(), timer_called_(false), timer_cancel_success_(false)
+    {}
     ~IntervalTimerTest() {}
     class TimerCallBack : public std::unary_function<void, void> {
     public:
@@ -64,7 +66,8 @@ protected:
         TimerCallBackCancelDeleter(IntervalTimerTest* test_obj,
                                    IntervalTimer* timer,
                                    TimerCallBackCounter& counter)
-            : test_obj_(test_obj), timer_(timer), counter_(counter), count_(0)
+            : test_obj_(test_obj), timer_(timer), counter_(counter), count_(0),
+              prev_counter_(-1)
         {}
         void operator()() {
             ++count_;

src/lib/asiolink/tests/io_fetch_unittest.cc

@@ -16,6 +16,7 @@
 #include <boost/bind.hpp>
 #include <cstdlib>
 #include <string>
+#include <vector>
 
 #include <string.h>
 
@@ -57,7 +58,7 @@ public:
 
     // The next member is the buffer iin which the "server" (implemented by the
     // response handler method) receives the question sent by the fetch object.
-    char            server_buff_[512];  ///< Server buffer
+    std::vector<char>      server_buff_;  ///< Server buffer
 
     /// \brief Constructor
     IOFetchTest() :
@@ -67,7 +68,8 @@ public:
         question_(Name("example.net"), RRClass::IN(), RRType::A()),
         buff_(new OutputBuffer(512)),
         udp_fetch_(IPPROTO_UDP, service_, question_, IOAddress(TEST_HOST),
-            TEST_PORT, buff_, this, 100)
+                   TEST_PORT, buff_, this, 100),
+        server_buff_(512)
         // tcp_fetch_(service_, question_, IOAddress(TEST_HOST), TEST_PORT,
         //    buff_, this, 100, IPPROTO_UDP)
         { }
@@ -105,11 +107,12 @@ public:
         // The QID in the incoming data is random so set it to 0 for the
         // data comparison check. (It was set to 0 when the buffer containing
         // the expected data was constructed above.)
-        server_buff_[0] = server_buff_[1] = 0;
+        server_buff_[0] = 0;
+        server_buff_[1] = 0;
 
         // Check that lengths are identical.
         EXPECT_EQ(msgbuf.getLength(), length);
-        EXPECT_TRUE(memcmp(msgbuf.getData(), server_buff_, length) == 0);
+        EXPECT_TRUE(memcmp(msgbuf.getData(), &server_buff_[0], length) == 0);
 
         // ... and return a message back.
         socket->send_to(asio::buffer(TEST_DATA, sizeof TEST_DATA), *remote);
@@ -172,7 +175,7 @@ TEST_F(IOFetchTest, UdpReceive) {
     socket.bind(udp::endpoint(TEST_HOST, TEST_PORT));
 
     udp::endpoint remote;
-    socket.async_receive_from(asio::buffer(server_buff_, sizeof(server_buff_)),
+    socket.async_receive_from(asio::buffer(server_buff_),
         remote,
         boost::bind(&IOFetchTest::respond, this, &remote, &socket, _1, _2));
     service_.get_io_service().post(udp_fetch_);

src/lib/asiolink/tests/recursive_query_unittest.cc

@@ -279,6 +279,7 @@ protected:
                             DNSLookup* lookup = NULL,
                             DNSAnswer* answer = NULL) :
             io_(io_service),
+            done_(false),
             message_(new Message(Message::PARSE)),
             answer_message_(new Message(Message::RENDER)),
             respbuf_(new OutputBuffer(0)),
@@ -412,7 +413,8 @@ protected:
 };
 
 RecursiveQueryTest::RecursiveQueryTest() :
-    dns_service_(NULL), callback_(NULL), sock_(-1), res_(NULL)
+    dns_service_(NULL), callback_(NULL), callback_protocol_(0),
+    callback_native_(-1), sock_(-1), res_(NULL)
 {
     io_service_ = new IOService();
     setDNSService(true, true);

src/lib/asiolink/tests/udp_socket_unittest.cc

@@ -138,7 +138,7 @@ public:
     }
 
     /// \brief Get number of bytes transferred in I/O
-    size_t getLength() {
+    size_t getLength() const {
         return (ptr_->length_);
     }
 
@@ -150,7 +150,7 @@ public:
     }
 
     /// \brief Get flag to say when callback was called
-    bool getCalled() {
+    bool getCalled() const {
         return (ptr_->called_);
     }
 
@@ -162,7 +162,7 @@ public:
     }
 
     /// \brief Return instance of callback name
-    std::string getName() {
+    std::string getName() const {
         return (ptr_->name_);
     }
 

src/lib/bench/tests/benchmark_unittest.cc

@@ -12,7 +12,7 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <unistd.h>             // for usleep
+#include <time.h>               // for nanosleep
 
 #include <bench/benchmark.h>
 
@@ -26,16 +26,17 @@ namespace {
 // number of iterations.
 class TestBenchMark {
 public:
-    TestBenchMark(const int sub_iterations, const int sleep_time) :
+    TestBenchMark(const int sub_iterations,
+                  const struct timespec& sleep_time) :
         sub_iterations_(sub_iterations), sleep_time_(sleep_time),
         setup_completed_(false), teardown_completed_(false)
     {}
     unsigned int run() {
-        usleep(sleep_time_);
+        nanosleep(&sleep_time_, NULL);
         return (sub_iterations_);
     }
     const int sub_iterations_;
-    const int sleep_time_;
+    const struct timespec sleep_time_;
     bool setup_completed_;
     bool teardown_completed_;
 };
@@ -67,6 +68,7 @@ TEST(BenchMarkTest, run) {
     // use some uncommon iterations for testing purpose:
     const int sub_iterations = 23;
     const int sleep_time = 50000; // will sleep for 50ms
+    const struct timespec sleep_timespec = { 0, sleep_time * 1000 };
     // we cannot expect particular accuracy on the measured duration, so
     // we'll include some conservative margin (25%) and perform range
     // comparison below.
@@ -75,12 +77,12 @@ TEST(BenchMarkTest, run) {
 
     // Prerequisite check: since the tests in this case may depend on subtle
     // timing, it may result in false positives.  There are reportedly systems
-    // where usleep() doesn't work as this test expects.  So we check the
+    // where sleeping doesn't work as this test expects.  So we check the
     // conditions before the tests, and if it fails skip the tests at the
     // risk of overlooking possible bugs.
     struct timeval check_begin, check_end;
     gettimeofday(&check_begin, NULL);
-    usleep(sleep_time);
+    nanosleep(&sleep_timespec, 0);
     gettimeofday(&check_end, NULL);
     check_end.tv_sec -= check_begin.tv_sec;
     if (check_end.tv_usec >= check_begin.tv_usec) {
@@ -97,7 +99,7 @@ TEST(BenchMarkTest, run) {
         return;
     }
 
-    TestBenchMark test_bench(sub_iterations, sleep_time);
+    TestBenchMark test_bench(sub_iterations, sleep_timespec);
     BenchMark<TestBenchMark> bench(1, test_bench, false);
     // Check pre-test conditions.
     EXPECT_FALSE(test_bench.setup_completed_);
@@ -130,7 +132,8 @@ TEST(BenchMarkTest, run) {
 TEST(BenchMarkTest, runWithNoIteration) {
     // we'll lie on the number of iteration (0).  it will result in
     // meaningless result, but at least it shouldn't crash.
-    TestBenchMark test_bench(0, 0);
+    const struct timespec null_timespec = { 0, 0 };
+    TestBenchMark test_bench(0, null_timespec);
     BenchMark<TestBenchMark> bench(1, test_bench, false);
     bench.run();
     EXPECT_EQ(0, bench.getIteration());

src/lib/bench/tests/loadquery_unittest.cc

@@ -55,7 +55,7 @@ const char* const LoadQueryTest::DATA_DIR = TEST_DATA_DIR;
 class QueryInserter {
 public:
     QueryInserter(stringstream& stream) : stream_(stream) {}
-    void operator()(const QueryParam& query) {
+    void operator()(const QueryParam& query) const {
         stream_ << query.first << " " << query.second << endl;
     }
 private:

src/lib/cache/message_cache.cc

@@ -74,6 +74,7 @@ MessageCache::update(const Message& msg) {
     return (message_table_.add(msg_entry, entry_key, true));
 }
 
+#if 0
 void
 MessageCache::dump(const std::string&) {
     //TODO
@@ -89,6 +90,7 @@ MessageCache::resize(uint32_t) {
     //TODO
     return (true);
 }
+#endif
 
 } // namespace cache
 } // namespace isc

src/lib/cache/message_cache.h

@@ -57,6 +57,7 @@ public:
     /// directly.
     bool update(const isc::dns::Message& msg);
 
+#if 0
     /// \brief Dump the message cache to specified file.
     /// \todo It should can be dumped to one configured database.
     void dump(const std::string& file_name);
@@ -67,6 +68,7 @@ public:
 
     /// \brief Resize the size of message cache in runtime.
     bool resize(uint32_t size);
+#endif
 
 protected:
     /// \brief Get the hash key for the message entry in the cache.

src/lib/cache/rrset_cache.cc

@@ -83,6 +83,7 @@ RRsetCache::update(const isc::dns::RRset& rrset, const RRsetTrustLevel& level) {
     }
 }
 
+#if 0
 void
 RRsetCache::dump(const std::string&) {
     //TODO
@@ -98,6 +99,7 @@ RRsetCache::resize(uint32_t) {
     //TODO
     return (true);
 }
+#endif
 
 } // namespace cache
 } // namespace isc

src/lib/cache/rrset_cache.h

@@ -70,6 +70,7 @@ public:
     RRsetEntryPtr update(const isc::dns::RRset& rrset,
                          const RRsetTrustLevel& level);
 
+#if 0
     /// \brief Dump the rrset cache to specified file.
     ///
     /// \param file_name The file to write to
@@ -89,6 +90,7 @@ public:
     /// \param The size to resize to
     /// \return true
     bool resize(uint32_t size);
+#endif
 
 private:
     uint16_t class_; // The class of the rrset cache.

src/lib/cc/tests/session_unittests.cc

@@ -74,7 +74,7 @@ public:
     }
 
     void
-    acceptHandler(const asio::error_code&) {
+    acceptHandler(const asio::error_code&) const {
     }
 
     void

src/lib/config/ccsession.cc

@@ -170,10 +170,10 @@ ModuleCCSession::readModuleSpecification(const std::string& filename) {
 
     try {
         module_spec = moduleSpecFromFile(file, true);
-    } catch (JSONError pe) {
+    } catch (const JSONError& pe) {
         cout << "Error parsing module specification file: " << pe.what() << endl;
         exit(1);
-    } catch (ModuleSpecError dde) {
+    } catch (const ModuleSpecError& dde) {
         cout << "Error reading module specification file: " << dde.what() << endl;
         exit(1);
     }

src/lib/config/module_spec.cc

@@ -121,7 +121,7 @@ void
 check_module_specification(ConstElementPtr def) {
     try {
         check_data_specification(def);
-    } catch (TypeError te) {
+    } catch (const TypeError& te) {
         throw ModuleSpecError(te.what());
     }
 }

src/lib/config/tests/ccsession_unittests.cc

@@ -31,7 +31,7 @@ using namespace std;
 
 namespace {
 std::string
-ccspecfile(const std::string name) {
+ccspecfile(const std::string& name) {
     return (std::string(TEST_DATA_PATH) + "/" + name);
 }
 

src/lib/config/tests/module_spec_unittests.cc

@@ -23,7 +23,7 @@
 using namespace isc::data;
 using namespace isc::config;
 
-std::string specfile(const std::string name) {
+std::string specfile(const std::string& name) {
     return (std::string(TEST_DATA_PATH) + "/" + name);
 }
 
@@ -36,7 +36,7 @@ moduleSpecError(const std::string& file,
     EXPECT_THROW(moduleSpecFromFile(specfile(file)), ModuleSpecError);
     try {
         ModuleSpec dd = moduleSpecFromFile(specfile(file));
-    } catch (ModuleSpecError dde) {
+    } catch (const ModuleSpecError& dde) {
         std::string ddew = dde.what();
         EXPECT_EQ(error1 + error2 + error3, ddew);
     }

src/lib/datasrc/memory_datasrc.cc

@@ -472,7 +472,7 @@ struct MemoryZone::MemoryZoneImpl {
                      * Otherwise, why would the DOMAINFLAG_WILD be there if
                      * there was no wildcard under it?
                      */
-                    assert(result = DomainTree::EXACTMATCH);
+                    assert(result == DomainTree::EXACTMATCH);
                     /*
                      * We have the wildcard node now. Jump below the switch,
                      * where handling of the common (exact-match) case is.

src/lib/datasrc/tests/rbtree_unittest.cc

@@ -56,7 +56,7 @@ const size_t Name::MAX_LABELS;
 namespace {
 class RBTreeTest : public::testing::Test {
 protected:
-    RBTreeTest() : rbtree_expose_empty_node(true) {
+    RBTreeTest() : rbtree_expose_empty_node(true), crbtnode(NULL) {
         const char* const domain_names[] = {
             "c", "b", "a", "x.d.e.f", "z.d.e.f", "g.h", "i.g.h", "o.w.y.d.e.f",
             "j.z.d.e.f", "p.w.y.d.e.f", "q.w.y.d.e.f"};

src/lib/dns/message.cc

@@ -113,10 +113,8 @@ public:
     vector<RRsetPtr> rrsets_[NUM_SECTIONS];
     ConstEDNSPtr edns_;
 
-#ifdef notyet
     // tsig/sig0: TODO
-    RRsetsSorter* sorter_;
-#endif
+    // RRsetsSorter* sorter_; : TODO
 
     void init();
     void setOpcode(const Opcode& opcode);

src/lib/dns/rdata/generic/nsec3_50.cc

@@ -117,11 +117,10 @@ NSEC3::NSEC3(const string& nsec3_str) :
     memset(bitmap, 0, sizeof(bitmap));
     do { 
         string type;
-        int code;
         iss >> type;
         if (type.length() != 0) {
             try {
-                code = RRType(type).getCode();
+                const int code = RRType(type).getCode();
                 bitmap[code / 8] |= (0x80 >> (code % 8));
             } catch (...) {
                 isc_throw(InvalidRdataText, "Invalid RRtype in NSEC3");

src/lib/dns/rdata/generic/nsec_47.cc

@@ -63,10 +63,9 @@ NSEC::NSEC(const string& nsec_str) :
     memset(bitmap, 0, sizeof(bitmap));
     do { 
         string type;
-        int code;
         iss >> type;
         try {
-            code = RRType(type).getCode();
+            const int code = RRType(type).getCode();
             bitmap[code / 8] |= (0x80 >> (code % 8));
         } catch (...) {
             isc_throw(InvalidRdataText, "Invalid RRtype in NSEC");

src/lib/dns/tests/name_unittest.cc

@@ -290,7 +290,7 @@ TEST_F(NameTest, assignment) {
 
     // Self assignment
     copy = copy;
-    EXPECT_EQ(copy, example_name);
+    EXPECT_EQ(example_name, copy);
 }
 
 TEST_F(NameTest, toText) {

src/lib/log/strutil.cc

@@ -62,7 +62,7 @@ trim(const string& instring) {
 // another dependency on a Boost library.
 
 vector<string>
-tokens(const std::string text, const std::string& delim) {
+tokens(const std::string& text, const std::string& delim) {
     vector<string> result;
 
     // Search for the first non-delimiter character

src/lib/log/strutil.h

@@ -71,7 +71,7 @@ std::string trim(const std::string& instring);
 /// \param delim Delimiter characters
 ///
 /// \return Vector of tokens.
-std::vector<std::string> tokens(const std::string text,
+std::vector<std::string> tokens(const std::string& text,
         const std::string& delim = std::string(" \t\n"));
 
 

src/lib/nsas/asiolink.h

@@ -46,7 +46,7 @@ public:
     }
 
     /// \return true if two addresses are equal
-    bool equal(const IOAddress& address)
+    bool equal(const IOAddress& address) const
     {return (toText() == address.toText());}
 
 private:

src/lib/nsas/nameserver_entry.h

@@ -105,7 +105,14 @@ public:
         name_(name),
         classCode_(class_code),
         expiration_(0)
-    {}
+    {
+        has_address_[V4_ONLY] = false;
+        has_address_[V6_ONLY] = false;
+        has_address_[ANY_OK] = false;
+        expect_address_[V4_ONLY] = false;
+        expect_address_[V6_ONLY] = false;
+        expect_address_[ANY_OK] = false;
+    }
 
     /*
      * \brief Return Address