From dd2fc25e4be6a606292699123fc58e2dd01e25c2 Mon Sep 17 00:00:00 2001
From: scott snyder <sss@karma>
Date: Thu, 17 Sep 2020 20:52:39 -0400
Subject: [PATCH] EventContainers: Fix cppcheck warnings.
- Pass class instances by const reference, not by value.
- Prefer using an initializer list to assigning members
in a ctor body.
- Prefer preincrement (or range for) to postincrement for iterators.
- Forbid assignment where copy is already forbidden.
---
.../IdentifiableValueContainer.h | 3 ++-
.../src/IdentifiableContainerBase.cxx | 9 +++++----
Event/EventContainers/test/IDC_Benchmark.cxx | 6 +++---
.../test/IDC_Realistic_Test.cxx | 6 +++---
.../test/IDMT_ContainerTest.cxx | 20 ++++++++-----------
.../EventContainers/test/ID_ContainerTest.cxx | 2 +-
6 files changed, 22 insertions(+), 24 deletions(-)
diff --git a/Event/EventContainers/EventContainers/IdentifiableValueContainer.h b/Event/EventContainers/EventContainers/IdentifiableValueContainer.h
index 2b8c1b3f782..8e20549f046 100644
--- a/Event/EventContainers/EventContainers/IdentifiableValueContainer.h
+++ b/Event/EventContainers/EventContainers/IdentifiableValueContainer.h
@@ -1,5 +1,5 @@
/*
- Copyright (C) 2002-2019 CERN for the benefit of the ATLAS collaboration
+ Copyright (C) 2002-2020 CERN for the benefit of the ATLAS collaboration
*/
#ifndef EVENTCONTAINERS_IDENTIFIABLEVALUECONTAINER_H
@@ -25,6 +25,7 @@ public:
//Prevent accidental copying
IdentifiableValueContainer(const IdentifiableValueContainer<T>&) = delete;
+ IdentifiableValueContainer& operator=(const IdentifiableValueContainer&) = delete;
~IdentifiableValueContainer() { if(m_own) delete m_cache; }
diff --git a/Event/EventContainers/src/IdentifiableContainerBase.cxx b/Event/EventContainers/src/IdentifiableContainerBase.cxx
index 7ed316fa335..a37c589ca64 100644
--- a/Event/EventContainers/src/IdentifiableContainerBase.cxx
+++ b/Event/EventContainers/src/IdentifiableContainerBase.cxx
@@ -1,5 +1,5 @@
/*
- Copyright (C) 2002-2018 CERN for the benefit of the ATLAS collaboration
+ Copyright (C) 2002-2020 CERN for the benefit of the ATLAS collaboration
*/
#include "EventContainers/IdentifiableContainerBase.h"
#include <algorithm>
@@ -18,9 +18,10 @@ using namespace EventContainers;
m_OnlineMode = true;
}
- IdentifiableContainerBase::IdentifiableContainerBase(size_t max){
- m_OnlineMode = false;
- m_link = std::make_unique<EventContainers::InternalOffline>(max);
+ IdentifiableContainerBase::IdentifiableContainerBase(size_t max)
+ : m_OnlineMode (false),
+ m_link (std::make_unique<EventContainers::InternalOffline>(max))
+ {
}
diff --git a/Event/EventContainers/test/IDC_Benchmark.cxx b/Event/EventContainers/test/IDC_Benchmark.cxx
index 85c66d3dc51..e31be9f85f8 100644
--- a/Event/EventContainers/test/IDC_Benchmark.cxx
+++ b/Event/EventContainers/test/IDC_Benchmark.cxx
@@ -9,7 +9,7 @@ using namespace EventContainers;
template<typename T>
-void timedelete(std::string name, T* ptr){
+void timedelete(const std::string& name, T* ptr){
auto start1 = std::chrono::steady_clock::now();
delete ptr;
auto end1 = std::chrono::steady_clock::now();
@@ -17,7 +17,7 @@ void timedelete(std::string name, T* ptr){
std::cout << name << " delete time " << time.count() << std::endl;
}
-void timebackwardsfill(std::string name, IdentifiableContainerMT<long unsigned int> *ptr){
+void timebackwardsfill(const std::string& name, IdentifiableContainerMT<long unsigned int> *ptr){
auto start1 = std::chrono::steady_clock::now();
for(size_t i =50000-2;i>=3;i-=3){
ptr->addCollection(new long unsigned int(i) ,i).ignore();
@@ -28,7 +28,7 @@ void timebackwardsfill(std::string name, IdentifiableContainerMT<long unsigned i
}
-void accessTime(std::string name, IdentifiableContainerMT<long unsigned int>& container){
+void accessTime(const std::string& name, IdentifiableContainerMT<long unsigned int>& container){
auto startwait = std::chrono::steady_clock::now();
container.prepareItr();
diff --git a/Event/EventContainers/test/IDC_Realistic_Test.cxx b/Event/EventContainers/test/IDC_Realistic_Test.cxx
index 4d314ac2062..e991dc9ac2d 100644
--- a/Event/EventContainers/test/IDC_Realistic_Test.cxx
+++ b/Event/EventContainers/test/IDC_Realistic_Test.cxx
@@ -69,9 +69,9 @@ public:
MyCollection( ) :m_id(0) {
return;
}
- MyCollection(const MyID& id ) {
- m_id=id;
- return;
+ MyCollection(const MyID& id )
+ : m_id(id)
+ {
}
~MyCollection() {
std::vector<DIGIT*>::const_iterator it = m_vector.begin();
diff --git a/Event/EventContainers/test/IDMT_ContainerTest.cxx b/Event/EventContainers/test/IDMT_ContainerTest.cxx
index a9a5fc1ff69..32df3de4345 100644
--- a/Event/EventContainers/test/IDMT_ContainerTest.cxx
+++ b/Event/EventContainers/test/IDMT_ContainerTest.cxx
@@ -4,6 +4,7 @@
// This is a test cxx file for IdentifiableContainerMT.
//
+#undef NDEBUG
#include "EventContainers/IdentifiableContainerMT.h"
#include "EventContainers/SelectAllObject.h"
#include "EventContainers/IdentifiableContTemp.h"
@@ -53,7 +54,7 @@ namespace IDC_TEST
typedef std::vector<DIGIT*>::const_iterator const_iterator;
MyCollection( ) :m_id(0) { return; }
- MyCollection(const MyID& id ){ m_id=id; return; }
+ MyCollection(const MyID& id ) : m_id(id) { }
~MyCollection() {
std::vector<DIGIT*>::const_iterator it = m_vector.begin();
std::vector<DIGIT*>::const_iterator it_end = m_vector.end();
@@ -344,11 +345,8 @@ int ID_ContainerTest::execute(EventContainers::Mode mode){
}
{//Repeat with post incrementor operator
SELECTOR select(m_container);
- digit_const_iterator it1 = select.begin();
- digit_const_iterator it2 = select.end();
- nd = 0 ;
- for(; it1!=it2; it1++){
- const MyDigit* digit = *it1;
+ nd = 0 ;
+ for (const MyDigit* digit : select) {
volatile float t = digit->val();
t = t + 1.;
++nd;
@@ -517,11 +515,8 @@ int ID_ContainerTest::execute(EventContainers::Mode mode){
}
SELECTOR select(container2);
- digit_const_iterator it1 = select.begin();
- digit_const_iterator it2 = select.end();
- nd = 0 ;
- for(; it1!=it2; it1++){
- const MyDigit* digit = *it1;
+ nd = 0 ;
+ for (const MyDigit* digit : select) {
volatile float t = digit->val();
t = t + 1.;
++nd;
@@ -560,7 +555,8 @@ int ID_ContainerTest::execute(EventContainers::Mode mode){
// std::cout << "error:addCollection->" << p->identifyHash() << std::endl;
std::vector<IdentifierHash> cacheshouldcontain = { IdentifierHash(0), IdentifierHash(3), IdentifierHash(10) };
std::vector<IdentifierHash> IDCshouldContain = { IdentifierHash(0), IdentifierHash(10) };
- assert(cache->ids().size() == 3);
+ size_t sz = cache->ids().size();
+ assert(sz == 3);
if(cache->ids() != cacheshouldcontain){
std::cout << __FILE__ << " cache does not contain correct elements" << std::endl;
std::abort();
diff --git a/Event/EventContainers/test/ID_ContainerTest.cxx b/Event/EventContainers/test/ID_ContainerTest.cxx
index 86204e92b04..f9452f52eaf 100644
--- a/Event/EventContainers/test/ID_ContainerTest.cxx
+++ b/Event/EventContainers/test/ID_ContainerTest.cxx
@@ -46,7 +46,7 @@ namespace IDC_TEST
typedef std::vector<DIGIT*>::const_iterator const_iterator;
MyCollection( ) :m_id(0) { return; }
- MyCollection(MyID& id ){ m_id=id; return; }
+ MyCollection(MyID& id ) : m_id(id) { }
~MyCollection() {
std::vector<DIGIT*>::const_iterator it = m_vector.begin();
std::vector<DIGIT*>::const_iterator it_end = m_vector.end();
--
GitLab