From 069d7a8a46526273c5d95ff20ed3088d7d8050b6 Mon Sep 17 00:00:00 2001 From: "Peter D. Barnes, Jr." Date: Tue, 28 May 2013 16:53:09 -0700 Subject: [PATCH 1/5] [Coverity] Structurally dead code (UNREACHABLE) --- src/dsr/model/dsr-rreq-table.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dsr/model/dsr-rreq-table.cc b/src/dsr/model/dsr-rreq-table.cc index 9ef5015de..362292639 100644 --- a/src/dsr/model/dsr-rreq-table.cc +++ b/src/dsr/model/dsr-rreq-table.cc @@ -225,7 +225,7 @@ bool RreqTable::MarkLinkAsUnidirectional (Ipv4Address neighbor, Time blacklistTimeout) { NS_LOG_LOGIC ("Add neighbor address in blacklist " << m_blackList.size ()); - for (std::vector::iterator i = m_blackList.begin (); i != m_blackList.end (); ++i) + for (std::vector::iterator i = m_blackList.begin (); i != m_blackList.end (); i++) { if (i->m_neighborAddress == neighbor) { From ab03dfed2a799eca25c60cc06949c3dc5dda5704 Mon Sep 17 00:00:00 2001 From: "Peter D. Barnes, Jr." Date: Tue, 28 May 2013 17:29:48 -0700 Subject: [PATCH 2/5] [Coverity] Out-of-bounds read (OVERRUN) --- src/core/model/test.cc | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/core/model/test.cc b/src/core/model/test.cc index c0524317c..30fe7bade 100644 --- a/src/core/model/test.cc +++ b/src/core/model/test.cc @@ -484,22 +484,30 @@ std::string TestRunnerImpl::ReplaceXmlSpecialCharacters (std::string xml) const { NS_LOG_FUNCTION (this << xml); - std::string specials = "<>&\"'"; - std::string replacements[] = {"<", ">", "&", "'", """}; + typedef std::map specials_map; + specials_map specials; + specials['<'] = "<"; + specials['>'] = ">"; + specials['&'] = "&"; + specials['"'] = "'"; + specials['\''] = """; + std::string result; - std::size_t index, length = xml.length (); + std::size_t length = xml.length (); for (size_t i = 0; i < length; ++i) { char character = xml[i]; - if ((index = specials.find (character)) == std::string::npos) + specials_map::const_iterator it = specials.find (character); + + if (it == specials.end ()) { result.push_back (character); } else { - result += replacements[index]; + result += it->second; } } return result; From dac21f54137bd07ef68eff69bd9bc340eab771cd Mon Sep 17 00:00:00 2001 From: "Peter D. Barnes, Jr." Date: Tue, 28 May 2013 17:39:46 -0700 Subject: [PATCH 3/5] [Coverity] Not restoring ostream format (STREAM_FORMAT_STATE) --- src/core/model/test.cc | 5 +++-- src/spectrum/test/spectrum-ideal-phy-test.cc | 3 ++- src/wifi/model/ctrl-headers.cc | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/core/model/test.cc b/src/core/model/test.cc index 30fe7bade..24cbdab22 100644 --- a/src/core/model/test.cc +++ b/src/core/model/test.cc @@ -547,7 +547,7 @@ TestRunnerImpl::PrintReport (TestCase *test, std::ostream *os, bool xml, int lev double user = test->m_result->clock.GetElapsedUser () / MS_PER_SEC; double system = test->m_result->clock.GetElapsedSystem () / MS_PER_SEC; - (*os).precision (3); + std::streamsize oldPrecision = (*os).precision (3); *os << std::fixed; std::string statusString = test->IsFailed ()?"FAIL":"PASS"; @@ -604,7 +604,8 @@ TestRunnerImpl::PrintReport (TestCase *test, std::ostream *os, bool xml, int lev } } - os->unsetf(std::ios_base::floatfield); + (*os).unsetf(std::ios_base::floatfield); + (*os).precision (oldPrecision); } void diff --git a/src/spectrum/test/spectrum-ideal-phy-test.cc b/src/spectrum/test/spectrum-ideal-phy-test.cc index a3a30d4ce..0cb90c938 100644 --- a/src/spectrum/test/spectrum-ideal-phy-test.cc +++ b/src/spectrum/test/spectrum-ideal-phy-test.cc @@ -194,6 +194,8 @@ SpectrumIdealPhyTestCase::DoRun (void) Simulator::Run (); double throughputBps = (g_rxBytes * 8.0) / testDuration; + std::clog.unsetf(std::ios_base::floatfield); + if (m_rateIsAchievable) { NS_TEST_ASSERT_MSG_EQ_TOL (throughputBps, m_phyRate, m_phyRate*0.01, "throughput does not match PHY rate"); @@ -203,7 +205,6 @@ SpectrumIdealPhyTestCase::DoRun (void) NS_TEST_ASSERT_MSG_EQ (throughputBps, 0.0, "PHY rate is not achievable but throughput is non-zero"); } - std::clog.unsetf(std::ios_base::floatfield); Simulator::Destroy (); } diff --git a/src/wifi/model/ctrl-headers.cc b/src/wifi/model/ctrl-headers.cc index 9d9efe1d9..bd37fc699 100644 --- a/src/wifi/model/ctrl-headers.cc +++ b/src/wifi/model/ctrl-headers.cc @@ -314,7 +314,7 @@ void CtrlBAckResponseHeader::Print (std::ostream &os) const { NS_LOG_FUNCTION (this << &os); - os << "TID_INFO=" << m_tidInfo << ", StartingSeq=" << std::hex << m_startingSeq; + os << "TID_INFO=" << m_tidInfo << ", StartingSeq=" << std::hex << m_startingSeq << std::dec; } uint32_t From f14f746227a3c7da8899d70d2a08445a484be6b5 Mon Sep 17 00:00:00 2001 From: "Peter D. Barnes, Jr." Date: Wed, 26 Jun 2013 17:15:56 -0700 Subject: [PATCH 4/5] [Coverity] Logically dead code (DEADCODE) --- src/network/model/nix-vector.cc | 3 ++- src/network/model/packet-metadata.cc | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/network/model/nix-vector.cc b/src/network/model/nix-vector.cc index 546a8642e..982cdbbfd 100644 --- a/src/network/model/nix-vector.cc +++ b/src/network/model/nix-vector.cc @@ -311,7 +311,8 @@ NixVector::Deserialize (const uint32_t* buffer, uint32_t size) // return zero if an entire nix-vector was // not deserialized - return (sizeCheck != 0) ? 0 : 1; + // This check is obviated by prior assert + return 1; } void diff --git a/src/network/model/packet-metadata.cc b/src/network/model/packet-metadata.cc index 51f73fb92..a54dc93fa 100644 --- a/src/network/model/packet-metadata.cc +++ b/src/network/model/packet-metadata.cc @@ -1349,7 +1349,9 @@ PacketMetadata::Deserialize (const uint8_t* buffer, uint32_t size) UpdateTail (tmp); } NS_ASSERT (desSize == 0); - return (desSize !=0) ? 0 : 1; + // Return zero if entire meta data was not deserialized + // This check is obviated by prior assert + return 1; } uint8_t* From 351249c5594d535ee10036f8ebd0a5a91533d730 Mon Sep 17 00:00:00 2001 From: "Peter D. Barnes, Jr." Date: Wed, 26 Jun 2013 17:26:56 -0700 Subject: [PATCH 5/5] [Coverity] "erase" invalidates iterator --- src/core/model/test.cc | 2 +- src/dsr/model/dsr-rcache.cc | 8 ++++++-- src/dsr/model/dsr-routing.cc | 2 +- src/internet/model/ipv6-static-routing.cc | 8 ++++++-- 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/core/model/test.cc b/src/core/model/test.cc index 24cbdab22..69cc6cac5 100644 --- a/src/core/model/test.cc +++ b/src/core/model/test.cc @@ -720,7 +720,7 @@ TestRunnerImpl::FilterTests (std::string testName, delete *j; // Remove this test case from the test suite. - test->m_children.erase (j); + j = test->m_children.erase (j); } else { diff --git a/src/dsr/model/dsr-rcache.cc b/src/dsr/model/dsr-rcache.cc index 0c569d894..744fac5e0 100644 --- a/src/dsr/model/dsr-rcache.cc +++ b/src/dsr/model/dsr-rcache.cc @@ -1141,14 +1141,18 @@ void RouteCache::AddNeighbor (std::vector nodeList, Ipv4Address ownAddress, Time expire) { NS_LOG_LOGIC ("Add neighbor number " << nodeList.size ()); - for (std::vector::iterator j = nodeList.begin (); j != nodeList.end (); ++j) + for (std::vector::iterator j = nodeList.begin (); j != nodeList.end ();) { Ipv4Address addr = *j; if (addr == ownAddress) { - nodeList.erase (j); + j = nodeList.erase (j); NS_LOG_DEBUG ("The node list size " << nodeList.size ()); } + else + { + ++j; + } Neighbor neighbor (addr, LookupMacAddress (addr), expire + Simulator::Now ()); m_nb.push_back (neighbor); PurgeMac (); diff --git a/src/dsr/model/dsr-routing.cc b/src/dsr/model/dsr-routing.cc index c66b1d24b..85d6cf41c 100644 --- a/src/dsr/model/dsr-routing.cc +++ b/src/dsr/model/dsr-routing.cc @@ -1014,7 +1014,7 @@ void DsrRouting::CheckSendBuffer () Ptr cleanP = packet->Copy (); uint8_t protocol = i->GetProtocol (); - m_sendBuffer.GetBuffer ().erase (i); + i = m_sendBuffer.GetBuffer ().erase (i); DsrRoutingHeader dsrRoutingHeader; Ptr copyP = packet->Copy (); diff --git a/src/internet/model/ipv6-static-routing.cc b/src/internet/model/ipv6-static-routing.cc index 2eb41f5a9..a6d5b1412 100644 --- a/src/internet/model/ipv6-static-routing.cc +++ b/src/internet/model/ipv6-static-routing.cc @@ -728,7 +728,7 @@ void Ipv6StaticRouting::NotifyRemoveRoute (Ipv6Address dst, Ipv6Prefix mask, Ipv NS_LOG_FUNCTION (this << dst << mask << nextHop << interface); if (dst != Ipv6Address::GetZero ()) { - for (NetworkRoutesI j = m_networkRoutes.begin (); j != m_networkRoutes.end (); j++) + for (NetworkRoutesI j = m_networkRoutes.begin (); j != m_networkRoutes.end ();) { Ipv6RoutingTableEntry* rtentry = j->first; Ipv6Prefix prefix = rtentry->GetDestNetworkPrefix (); @@ -737,7 +737,11 @@ void Ipv6StaticRouting::NotifyRemoveRoute (Ipv6Address dst, Ipv6Prefix mask, Ipv if (dst == entry && prefix == mask && rtentry->GetInterface () == interface) { delete j->first; - m_networkRoutes.erase (j); + j = m_networkRoutes.erase (j); + } + else + { + ++j; } } }