[master] Merge branch 'trac2278'

src/cppcheck-suppress.lst

@@ -11,3 +11,18 @@ missingInclude
 //
 //    // cppcheck-suppress duplicateExpression
 //    EXPECT_FALSE(small_name < small_name);
+
+// With cppcheck 1.56, there are a number of false positives, which
+// All of these should be checked and hopefully removed after upgrading
+// cppcheck past 1.56
+
+// eraseDereference: This is a known false positive, which has been
+// fixed in the current development version of cppcheck
+eraseDereference
+
+// Unused functions: there suddenly are a lot of unused function errors
+// We could address those by adding for instance early declarations or
+// (unnecessary) header files, but they were all somewhat false positives
+// When we upgrade past 1.56, we should re-check this, and perhaps enable
+// unused-functions again.
+unusedFunction

src/lib/cache/tests/rrset_entry_unittest.cc

@@ -50,9 +50,6 @@ class DerivedRRsetEntry: public RRsetEntry {
 public:
     DerivedRRsetEntry(const isc::dns::RRset& rrset, const RRsetTrustLevel& level) : RRsetEntry(rrset, level) {};
 
-    void updateTTLForTest() {
-
-    }
 };
 
 #define TEST_TTL 100

src/lib/cc/data.cc

@@ -413,7 +413,7 @@ from_stringstream_number(std::istream &in, int &pos) {
             isc_throw(JSONError, std::string("Number overflow: ") + number);
         }
     }
-    
+
     if (is_double) {
         return (Element::create(d));
     } else {
@@ -501,7 +501,7 @@ from_stringstream_map(std::istream &in, const std::string& file, int& line,
 
             ConstElementPtr value = Element::fromJSON(in, file, line, pos);
             map->set(key, value);
-            
+
             skip_to(in, file, line, pos, ",}", WHITESPACE);
             c = in.get();
             pos++;
@@ -942,7 +942,7 @@ removeIdentical(ConstElementPtr a, ConstElementPtr b) {
     if (!b) {
         return (result);
     }
-    
+
     if (a->getType() != Element::map || b->getType() != Element::map) {
         isc_throw(TypeError, "Non-map Elements passed to removeIdentical");
     }
@@ -965,7 +965,7 @@ merge(ElementPtr element, ConstElementPtr other) {
         other->getType() != Element::map) {
         isc_throw(TypeError, "merge arguments not MapElements");
     }
-    
+
     const std::map<std::string, ConstElementPtr>& m = other->mapValue();
     for (std::map<std::string, ConstElementPtr>::const_iterator it = m.begin();
          it != m.end() ; ++it) {

src/lib/dhcp/tests/iface_mgr_unittest.cc

@@ -1096,7 +1096,7 @@ void parse_ifconfig(const std::string& textFile, IfaceMgr::IfaceCollection& ifac
             addr = addr.substr(0, addr.find_first_of(" "));
             IOAddress a(addr);
             iface->addAddress(a);
-        } else if(line.find("Metric")) {
+        } else if(line.find("Metric") != string::npos) {
             // flags
             if (line.find("UP") != string::npos) {
                 iface->flag_up_ = true;

src/lib/dns/benchmarks/message_renderer_bench.cc

@@ -39,7 +39,7 @@ public:
         renderer_(NULL),
         names_(names)
     {}
-    MessageRendererBenchMark() {
+    ~MessageRendererBenchMark() {
         delete renderer_;
     }
     unsigned int run() {

src/lib/dns/benchmarks/rdatarender_bench.cc

@@ -42,9 +42,10 @@ template <typename T>
 class RdataRenderBenchMark {
 public:
     RdataRenderBenchMark(const vector<T>& dataset) :
-        dataset_(dataset)
+        dataset_(dataset),
+        renderer_(NULL)
     {}
-    RdataRenderBenchMark() {
+    ~RdataRenderBenchMark() {
         delete renderer_;
     }
     unsigned int run() {

src/lib/dns/python/edns_python.cc

@@ -269,7 +269,6 @@ EDNS_createFromRR(const s_EDNS* null_self, PyObject* args) {
     const PyObject* rrtype;
     const PyObject* rrttl;
     const PyObject* rdata;
-    s_EDNS* edns_obj = NULL;
 
     assert(null_self == NULL);
 
@@ -277,7 +276,7 @@ EDNS_createFromRR(const s_EDNS* null_self, PyObject* args) {
                          &rrclass_type, &rrclass, &rrtype_type, &rrtype,
                          &rrttl_type, &rrttl, &rdata_type, &rdata)) {
         uint8_t extended_rcode;
-        edns_obj = PyObject_New(s_EDNS, &edns_type);
+        s_EDNS* edns_obj = PyObject_New(s_EDNS, &edns_type);
         if (edns_obj == NULL) {
             return (NULL);
         }

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

@@ -57,7 +57,6 @@ AFSDB::AFSDB(const std::string& afsdb_str) :
     try {
         const uint32_t subtype = tokenToNum<int32_t, 16>(getToken(iss));
         const Name servername(getToken(iss));
-        string server;
 
         if (!iss.eof()) {
             isc_throw(InvalidRdataText, "Unexpected input for AFSDB"

src/lib/dns/tests/rrparamregistry_unittest.cc

@@ -37,6 +37,7 @@ protected:
     {
         ostringstream oss1;
         oss1 << test_class_code;
+        // cppcheck-suppress useInitializationList
         test_class_unknown_str = "CLASS" + oss1.str();
 
         ostringstream oss2;
@@ -60,7 +61,7 @@ protected:
 
     // we assume class/type numbers are officially unassigned.  If not we'll
     // need to update the test cases.
-    static const uint16_t test_class_code = 65533; 
+    static const uint16_t test_class_code = 65533;
     static const uint16_t test_type_code = 65534;
     static const string test_class_str;
     static const string test_type_str;
@@ -77,7 +78,7 @@ TEST_F(RRParamRegistryTest, addRemove) {
 
     // the first removal attempt should succeed
     EXPECT_TRUE(RRParamRegistry::getRegistry().removeType(test_type_code));
-    // then toText() should treat it as an "unknown" 
+    // then toText() should treat it as an "unknown"
     EXPECT_EQ(test_type_unknown_str, RRType(test_type_code).toText());
     // attempt of removing non-existent mapping should result in 'false'
     EXPECT_FALSE(RRParamRegistry::getRegistry().removeType(test_type_code));

src/lib/log/compiler/message.cc

@@ -109,11 +109,13 @@ currentTime() {
     // Get a text representation of the current time.
     time_t curtime;
     time(&curtime);
-    char* buffer = ctime(&curtime);
+    struct tm* timeinfo;
+    timeinfo = localtime(&curtime);
 
-    // Convert to string and strip out the trailing newline
-    string current_time = buffer;
-    return (isc::util::str::trim(current_time));
+    char buffer[80];
+    strftime(buffer, 80, "%a %b %d %Y %H:%M", timeinfo);
+
+    return (std::string(buffer));
 }
 
 

src/lib/log/tests/logger_example.cc

@@ -118,7 +118,6 @@ int main(int argc, char** argv) {
     LoggerSpecification cur_spec(ROOT_NAME);// Current specification
     OutputOption        cur_opt;            // Current output option
     vector<LoggerSpecification> loggers;    // Set of logger specifications
-    vector<OutputOption>        options;    // Output options for logger
     std::string                 severity;   // Severity set for logger
 
     // Initialize logging system - set the root logger name.

src/lib/resolve/recursive_query.cc

@@ -176,7 +176,7 @@ class RunningQuery : public IOFetch::Callback {
 class ResolverNSASCallback : public isc::nsas::AddressRequestCallback {
 public:
     ResolverNSASCallback(RunningQuery* rq) : rq_(rq) {}
-    
+
     void success(const isc::nsas::NameserverAddress& address) {
         // Success callback, send query to found namesever
         LOG_DEBUG(isc::resolve::logger, RESLIB_DBG_CB, RESLIB_RUNQ_SUCCESS)
@@ -184,7 +184,7 @@ public:
         rq_->nsasCallbackCalled();
         rq_->sendTo(address);
     }
-    
+
     void unreachable() {
         // Nameservers unreachable: drop query or send servfail?
         LOG_DEBUG(isc::resolve::logger, RESLIB_DBG_CB, RESLIB_RUNQ_FAIL);
@@ -261,7 +261,7 @@ private:
     bool done_;
 
     // If we have a client timeout, we call back with a failure message,
-    // but we do not stop yet. We use this variable to make sure we 
+    // but we do not stop yet. We use this variable to make sure we
     // don't call back a second time later
     bool callback_called_;
 
@@ -270,7 +270,7 @@ private:
 
     // Reference to our cache
     isc::cache::ResolverCache& cache_;
-    
+
     // the 'current' zone we are in (i.e.) we start out at the root,
     // and for each delegation this gets updated with the zone the
     // delegation points to.
@@ -278,7 +278,7 @@ private:
     // of the call we use it in take a string, we need update those
     // too).
     std::string cur_zone_;
-    
+
     // This is the handler we pass on to the NSAS; it is called when
     // the NSAS has an address for us to query
     boost::shared_ptr<ResolverNSASCallback> nsas_callback_;
@@ -295,7 +295,7 @@ private:
 
     // The moment in time we sent a query to the nameserver above.
     struct timeval current_ns_qsent_time;
-    
+
     // RunningQuery deletes itself when it is done. In order for us
     // to do this safely, we must make sure that there are no events
     // that might call back to it. There are two types of events in
@@ -365,7 +365,7 @@ private:
             io_.get_io_service().post(query);
         }
     }
-    
+
     // 'general' send, ask the NSAS to give us an address.
     void send(IOFetch::Protocol protocol = IOFetch::UDP, bool edns = true) {
         protocol_ = protocol;   // Store protocol being used for this
@@ -397,7 +397,7 @@ private:
             nsas_.lookup(cur_zone_, question_.getClass(), nsas_callback_);
         }
     }
-    
+
     // Called by our NSAS callback handler so we know we do not have
     // an outstanding NSAS call anymore.
     void nsasCallbackCalled() {
@@ -422,13 +422,13 @@ private:
         // here (classify() will set it when it walks through
         // the cname chain to verify it).
         Name cname_target(question_.getName());
-        
+
         isc::resolve::ResponseClassifier::Category category =
             isc::resolve::ResponseClassifier::classify(
                 question_, incoming, cname_target, cname_count_);
 
         bool found_ns = false;
-            
+
         switch (category) {
         case isc::resolve::ResponseClassifier::ANSWER:
         case isc::resolve::ResponseClassifier::ANSWERCNAME:
@@ -569,7 +569,7 @@ private:
                 // SERVFAIL if we get FORMERR instead
             }
             goto SERVFAIL;
-            
+
         default:
 SERVFAIL:
             // Some error in received packet it.  Report it and return SERVFAIL
@@ -718,7 +718,7 @@ public:
             ++outstanding_events_;
             lookup_timer.async_wait(boost::bind(&RunningQuery::lookupTimeout, this));
         }
-        
+
         // Setup the timer to send an answer (client_timeout)
         if (client_timeout >= 0) {
             client_timer.expires_from_now(
@@ -726,7 +726,7 @@ public:
             ++outstanding_events_;
             client_timer.async_wait(boost::bind(&RunningQuery::clientTimeout, this));
         }
-        
+
         doLookup();
     }
 
@@ -741,7 +741,7 @@ public:
         --outstanding_events_;
         stop();
     }
-    
+
     // called if we have a client timeout; if our callback has
     // not been called, call it now. But do not stop.
     void clientTimeout() {
@@ -810,7 +810,7 @@ public:
         // XXX is this the place for TCP retry?
         assert(outstanding_events_ > 0);
         --outstanding_events_;
-        
+
         if (!done_ && result != IOFetch::TIME_OUT) {
             // we got an answer
 
@@ -890,7 +890,7 @@ public:
             stop();
         }
     }
-    
+
     // Clear the answer parts of answer_message, and set the rcode
     // to servfail
     void makeSERVFAIL() {
@@ -1096,7 +1096,7 @@ RecursiveQuery::resolve(const QuestionPtr& question,
         // Message found, return that
         LOG_DEBUG(isc::resolve::logger, RESLIB_DBG_CACHE, RESLIB_RECQ_CACHE_FIND)
                   .arg(questionText(*question)).arg(1);
-        
+
         // TODO: err, should cache set rcode as well?
         answer_message->setRcode(Rcode::NOERROR());
         callback->success(answer_message);
@@ -1146,11 +1146,11 @@ RecursiveQuery::resolve(const Question& question,
     // TODO: general 'prepareinitialanswer'
     answer_message->setOpcode(isc::dns::Opcode::QUERY());
     answer_message->addQuestion(question);
-    
+
     // First try to see if we have something cached in the messagecache
     LOG_DEBUG(isc::resolve::logger, RESLIB_DBG_TRACE, RESLIB_RESOLVE)
               .arg(questionText(question)).arg(2);
-    
+
     if (cache_.lookup(question.getName(), question.getType(),
                       question.getClass(), *answer_message) &&
         answer_message->getRRCount(Message::SECTION_ANSWER) > 0) {
@@ -1181,7 +1181,7 @@ RecursiveQuery::resolve(const Question& question,
             // delete itself when it is done
             LOG_DEBUG(isc::resolve::logger, RESLIB_DBG_TRACE, RESLIB_RECQ_CACHE_NO_FIND)
                       .arg(questionText(question)).arg(2);
-            new RunningQuery(io, question, answer_message, 
+            new RunningQuery(io, question, answer_message,
                              test_server_, buffer, crs, query_timeout_,
                              client_timeout_, lookup_timeout_, retries_,
                              nsas_, cache_, rtt_recorder_);

src/lib/resolve/tests/recursive_query_unittest.cc

@@ -218,7 +218,7 @@ protected:
     }
 
     // Receive a UDP packet from a mock server; used for testing
-    // recursive lookup.  The caller must place a RecursiveQuery 
+    // recursive lookup.  The caller must place a RecursiveQuery
     // on the IO Service queue before running this routine.
     void recvUDP(const int family, void* buffer, size_t& size) {
         ScopedAddrInfo sai(resolveAddress(family, IPPROTO_UDP, true));
@@ -267,7 +267,7 @@ protected:
         if (ret < 0) {
             isc_throw(IOError, "recvfrom failed: " << strerror(errno));
         }
-        
+
         // Pass the message size back via the size parameter
         size = ret;
     }
@@ -693,37 +693,6 @@ createTestSocket() {
     return (sock.release());
 }
 
-int
-setSocketTimeout(int sock, size_t tv_sec, size_t tv_usec) {
-    const struct timeval timeo = { tv_sec, tv_usec };
-    int recv_options = 0;
-    if (setsockopt(sock, SOL_SOCKET, SO_RCVTIMEO, &timeo, sizeof(timeo))) {
-        if (errno == ENOPROTOOPT) { // see RecursiveQueryTest::recvUDP()
-            recv_options = MSG_DONTWAIT;
-        } else {
-            isc_throw(IOError, "set RCVTIMEO failed: " << strerror(errno));
-        }
-    }
-    return (recv_options);
-}
-
-// try to read from the socket max time
-// *num is incremented for every succesfull read
-// returns true if it can read max times, false otherwise
-bool tryRead(int sock, int recv_options, size_t max, int* num) {
-    size_t i = 0;
-    do {
-        char inbuff[512];
-        if (recv(sock, inbuff, sizeof(inbuff), recv_options) < 0) {
-            return false;
-        } else {
-            ++i;
-            ++*num;
-        }
-    } while (i < max);
-    return true;
-}
-
 // Mock resolver callback for testing forward query.
 class MockResolverCallback : public isc::resolve::ResolverInterface::Callback {
 public:
@@ -904,7 +873,7 @@ TEST_F(RecursiveQueryTest, lowtimeouts) {
 TEST_F(RecursiveQueryTest, DISABLED_recursiveSendOk) {
     setDNSService(true, false);
     bool done;
-    
+
     MockServerStop server(io_service_, &done);
     vector<pair<string, uint16_t> > empty_vector;
     RecursiveQuery rq(*dns_service_, *nsas_, cache_, empty_vector,
@@ -930,7 +899,7 @@ TEST_F(RecursiveQueryTest, DISABLED_recursiveSendOk) {
 TEST_F(RecursiveQueryTest, DISABLED_recursiveSendNXDOMAIN) {
     setDNSService(true, false);
     bool done;
-    
+
     MockServerStop server(io_service_, &done);
     vector<pair<string, uint16_t> > empty_vector;
     RecursiveQuery rq(*dns_service_, *nsas_, cache_, empty_vector,

src/lib/util/hash/sha1.cc

@@ -48,7 +48,7 @@
  *      merchantability of this software or the suitability of this
  *      software for any particular purpose.  It is provided "as is"
  *      without express or implied warranty of any kind.
- *      
+ *
  */
 #include <util/hash/sha1.h>
 
@@ -79,7 +79,7 @@ SHA_Parity(const uint32_t x, const uint32_t y, const uint32_t z) {
     return ((x) ^ (y) ^ (z));
 }
 
-static inline int 
+static inline int
 SHA1CircularShift(uint8_t bits, uint32_t word) {
     return ((word << bits) | (word >> (32 - bits)));
 }

src/lib/util/tests/lru_list_unittest.cc

@@ -168,7 +168,7 @@ protected:
         entry7_(new TestEntry("eta", 1))
     {}
 
-    virtual ~LruListTest() 
+    virtual ~LruListTest()
     {}
 
     boost::shared_ptr<TestEntry>    entry1_;
@@ -355,7 +355,7 @@ TEST_F(LruListTest, Dropped) {
     lru.add(entry5_);
     EXPECT_NE(0, (entry2_->getCode() & 0x8000));
 
-    // Delete an entry and check that the handler does not run. 
+    // Delete an entry and check that the handler does not run.
     EXPECT_EQ(0, (entry3_->getCode() & 0x8000));
     lru.remove(entry3_);
     EXPECT_EQ(0, (entry3_->getCode() & 0x8000));
@@ -386,7 +386,7 @@ TEST_F(LruListTest, Clear) {
     EXPECT_NE(0, (entry1_->getCode() & 0x8000));
     EXPECT_NE(0, (entry2_->getCode() & 0x8000));
     EXPECT_NE(0, (entry3_->getCode() & 0x8000));
- 
+
     EXPECT_EQ(0, lru.size());
 }