Merge pull request #12676 from slava77/CMSSW_8_0_X_2015-11-30-1100/si…

…gn643/codeRules

resolve some issues from static analyzer (mostly in RECO)

MagneticField/Interpolation/src/InterpolationDebug.cc

@@ -1,5 +1,5 @@
 #include "InterpolationDebug.h"
 
 #ifdef DEBUG_LinearGridInterpolator3D
-bool InterpolationDebug::debug=false;
+const bool InterpolationDebug::debug=false;
 #endif

MagneticField/Interpolation/src/InterpolationDebug.h

@@ -10,7 +10,7 @@
 
 #include "FWCore/Utilities/interface/Visibility.h"
 struct dso_internal InterpolationDebug {
-  static bool debug;
+  static const bool debug;
 };
 
 #endif

MagneticField/Interpolation/src/MagneticFieldGrid.cc

@@ -1,5 +1,6 @@
 // include header for MagneticFieldGrid (regular + extension for some trapezoids)
 #include "MagneticFieldGrid.h"
+#include <cassert>
 
 using namespace std;
 
@@ -48,6 +49,8 @@ void MagneticFieldGrid::load(const string& name){
     inFile >> BasicDistance0[0]    >> BasicDistance0[1]    >> BasicDistance0[2];
     inFile >> RParAsFunOfPhi[0]    >> RParAsFunOfPhi[1]    >> RParAsFunOfPhi[2]    >> RParAsFunOfPhi[3];
     break;
+  default:
+    assert(0); //this is a bug
   }
   //reading the field
   float Bx, By, Bz;
@@ -146,17 +149,17 @@ void MagneticFieldGrid::putCoordGetInd(double X1, double X2, double X3, int &Ind
   double pnt[3] = {X1,X2,X3};
   int index[3];
   switch (GridType){
-  case 1:
+  case 1:{
     for (int i=0; i<3; ++i){
       index[i] = int((pnt[i]-ReferencePoint[i])/BasicDistance0[i]);
     }
-    break;
-  case 2:
+    break;}
+  case 2:{
     // FIXME: Should use else!
     for (int i=0; i<3; ++i){
       if (EasyCoordinate[i]){
 	index[i] = int((pnt[i]-ReferencePoint[i])/BasicDistance0[i]);
-      }
+      } else index[i] = 0;//computed below
     }
     for (int i=0; i<3; ++i){
       if (!EasyCoordinate[i]){
@@ -169,18 +172,18 @@ void MagneticFieldGrid::putCoordGetInd(double X1, double X2, double X3, int &Ind
 	index[i] = int((pnt[i]-(ReferencePoint[i] + offset))/stepSize);
       }
     }
-    break;
-  case 3:
+    break;}
+  case 3:{
     for (int i=0; i<3; ++i){
       index[i] = int((pnt[i]-ReferencePoint[i])/BasicDistance0[i]);
     }
-    break;
-  case 4:
+    break;}
+  case 4:{
     // FIXME: should use else!
     for (int i=0; i<3; ++i){
       if (EasyCoordinate[i]){
 	index[i] = int((pnt[i]-ReferencePoint[i])/BasicDistance0[i]);
-      }
+      } else index[i] = 0;//computed below
     }
     for (int i=0; i<3; ++i){
       if (!EasyCoordinate[i]){
@@ -193,16 +196,18 @@ void MagneticFieldGrid::putCoordGetInd(double X1, double X2, double X3, int &Ind
 	index[i] = int((pnt[i]-(ReferencePoint[i] + offset))/stepSize);
       }
     }
-    break;
-  case 5:
+    break;}
+  case 5:{
     double sinPhi = sin(pnt[1]);
     double stepSize = RParAsFunOfPhi[0] + RParAsFunOfPhi[1]/sinPhi - RParAsFunOfPhi[2] - RParAsFunOfPhi[3]/sinPhi;
     stepSize =  stepSize/(NumberOfPoints[0]-1);
     double startingPoint = RParAsFunOfPhi[2] + RParAsFunOfPhi[3]/sinPhi;
     index[0] = int((pnt[0]-startingPoint)/stepSize);
     index[1] = int((pnt[1]-ReferencePoint[1])/BasicDistance0[1]);
     index[2] = int((pnt[2]-ReferencePoint[2])/BasicDistance0[2]);
-    break;
+    break;}
+  default:
+    assert(0); //shouldn't be here
   }
   Index1 = index[0];
   Index2 = index[1];
@@ -224,12 +229,12 @@ void MagneticFieldGrid::putIndGetCoord(int Index1, int Index2, int Index3, doubl
   int index[3] = {Index1, Index2, Index3};
   double pnt[3];
   switch (GridType){
-  case 1:
+  case 1:{
     for (int i=0; i<3; ++i){
       pnt[i] = ReferencePoint[i] + BasicDistance0[i]*index[i];
     }
-    break;
-  case 2:
+    break;}
+  case 2:{
     for (int i=0; i<3; ++i){
       if (EasyCoordinate[i]){
 	pnt[i] = ReferencePoint[i] + BasicDistance0[i]*index[i];
@@ -244,13 +249,13 @@ void MagneticFieldGrid::putIndGetCoord(int Index1, int Index2, int Index3, doubl
 	pnt[i] = ReferencePoint[i] + offset + stepSize*index[i];
       }
     }
-    break;
-  case 3:
+    break;}
+  case 3:{
     for (int i=0; i<3; ++i){
       pnt[i] = ReferencePoint[i] + BasicDistance0[i]*index[i];
     }
-    break;
-  case 4:
+    break;}
+  case 4:{
     for (int i=0; i<3; ++i){
       if (EasyCoordinate[i]){
 	pnt[i] = ReferencePoint[i] + BasicDistance0[i]*index[i];
@@ -265,16 +270,18 @@ void MagneticFieldGrid::putIndGetCoord(int Index1, int Index2, int Index3, doubl
 	pnt[i] = ReferencePoint[i] + offset + stepSize*index[i];
       }
     }
-    break;
-  case 5:
+    break;}
+  case 5:{
     pnt[2] = ReferencePoint[2] + BasicDistance0[2]*index[2];
     pnt[1] = ReferencePoint[1] + BasicDistance0[1]*index[1];
     double sinPhi = sin(pnt[1]);
     double stepSize = RParAsFunOfPhi[0] + RParAsFunOfPhi[1]/sinPhi - RParAsFunOfPhi[2] - RParAsFunOfPhi[3]/sinPhi;
     stepSize =  stepSize/(NumberOfPoints[0]-1);
     double startingPoint = RParAsFunOfPhi[2] + RParAsFunOfPhi[3]/sinPhi;
     pnt[0] = startingPoint + stepSize*index[0];
-    break;
+    break;}
+  default:
+    assert(0);//bug if make it here
   }
   X1 = pnt[0];
   X2 = pnt[1];

RecoEcal/EgammaClusterAlgos/interface/Multi5x5BremRecoveryClusterAlgo.h

@@ -56,7 +56,7 @@ class Multi5x5BremRecoveryClusterAlgo
 
 
   // the method called from outside to do the SuperClustering - returns a vector of SCs:
-  reco::SuperClusterCollection makeSuperClusters(reco::CaloClusterPtrVector & clusters);
+  reco::SuperClusterCollection makeSuperClusters(const reco::CaloClusterPtrVector & clusters);
 
  private:
 

RecoEcal/EgammaClusterAlgos/src/EndcapPiZeroDiscriminatorAlgo.cc

@@ -292,6 +292,7 @@ void EndcapPiZeroDiscriminatorAlgo::readWeightFile(const char *Weights_file){
         }
    }
    fclose(weights);
+   delete[] line;
 }
 
 //=====================================================================================

RecoEcal/EgammaClusterAlgos/src/Multi5x5BremRecoveryClusterAlgo.cc

@@ -2,7 +2,7 @@
 #include "RecoEcal/EgammaCoreTools/interface/BremRecoveryPhiRoadAlgo.h"
 #include "FWCore/MessageLogger/interface/MessageLogger.h"
 
-reco::SuperClusterCollection Multi5x5BremRecoveryClusterAlgo::makeSuperClusters(reco::CaloClusterPtrVector & clustersCollection)
+reco::SuperClusterCollection Multi5x5BremRecoveryClusterAlgo::makeSuperClusters(const reco::CaloClusterPtrVector & clustersCollection)
 {
 
 	const float etaBorder = 1.479;
@@ -13,9 +13,8 @@ reco::SuperClusterCollection Multi5x5BremRecoveryClusterAlgo::makeSuperClusters(
 	reco::CaloClusterPtrVector islandClustersEndCap_v;
 
 	// ...and populate them:
-	for (reco::CaloCluster_iterator it = clustersCollection.begin(); it != clustersCollection.end(); it++)
+	for (auto const& cluster_p : clustersCollection)
 	{
-		reco::CaloClusterPtr cluster_p = *it;
 		if (cluster_p->algo() == reco::CaloCluster::multi5x5) 
 		{
 			if (fabs(cluster_p->position().eta()) < etaBorder)
@@ -44,21 +43,20 @@ void Multi5x5BremRecoveryClusterAlgo::makeIslandSuperClusters(reco::CaloClusterP
 {
   if(clusters_v.empty()) return;
 
-  bool usedSeed[clusters_v.size()];
-  for (auto ic=0U; ic<clusters_v.size(); ++ic) usedSeed[ic]=false;
+  const auto clustersSize = clusters_v.size();
+  assert(clustersSize>0);
+
+  bool usedSeed[clustersSize];
+  for (auto ic=0U; ic<clustersSize; ++ic) usedSeed[ic]=false;
 
-  #ifdef __clang__ 
-  std::vector<float> eta(clusters_v.size()), phi(clusters_v.size()), et(clusters_v.size());
-  #else
-  float eta[clusters_v.size()], phi[clusters_v.size()], et[clusters_v.size()];
-  #endif
-  for (auto ic=0U; ic<clusters_v.size(); ++ic) {
+  float eta[clustersSize], phi[clustersSize], et[clustersSize];
+  for (auto ic=0U; ic<clustersSize; ++ic) {
     eta[ic]=clusters_v[ic]->eta();
     phi[ic]=clusters_v[ic]->phi();
     et[ic]=clusters_v[ic]->energy() * sin(clusters_v[ic]->position().theta());
   }
 
-  for (auto is=0U; is<clusters_v.size(); ++is) {
+  for (auto is=0U; is<clustersSize; ++is) {
     // check this seed was not already used
     if (usedSeed[is]) continue;
     auto const & currentSeed = clusters_v[is];
@@ -83,7 +81,7 @@ void Multi5x5BremRecoveryClusterAlgo::makeIslandSuperClusters(reco::CaloClusterP
     constituentClusters.push_back(currentSeed);
     auto ic = is + 1;
 
-    while (ic < clusters_v.size()) {
+    while (ic < clustersSize) {
       auto const & currentCluster = clusters_v[ic];
 
       // if dynamic phi road is enabled then compute the phi road for a cluster

RecoEcal/EgammaCoreTools/src/ClusterShapeAlgo.cc

@@ -55,21 +55,20 @@ void ClusterShapeAlgo::Calculate_TopEnergy(const reco::BasicCluster &passedClust
   double eMax=0;
   DetId eMaxId(0);
 
-  std::vector< std::pair<DetId, float> > clusterDetIds = passedCluster.hitsAndFractions();
-  std::vector< std::pair<DetId, float> >::iterator posCurrent;
+  const std::vector< std::pair<DetId, float> >& clusterDetIds = passedCluster.hitsAndFractions();
 
   EcalRecHit testEcalRecHit;
 
-  for(posCurrent = clusterDetIds.begin(); posCurrent != clusterDetIds.end(); posCurrent++)
+  for(auto const& posCurrent : clusterDetIds)
   {
-    if (((*posCurrent).first != DetId(0)) && (hits->find((*posCurrent).first) != hits->end()))
+    if ((posCurrent.first != DetId(0)) && (hits->find(posCurrent.first) != hits->end()))
     {
-      EcalRecHitCollection::const_iterator itt = hits->find((*posCurrent).first);
+      EcalRecHitCollection::const_iterator itt = hits->find(posCurrent.first);
       testEcalRecHit = *itt;
 
-      if(testEcalRecHit.energy() * (*posCurrent).second > eMax)
+      if(testEcalRecHit.energy() * posCurrent.second > eMax)
       {
-        eMax = testEcalRecHit.energy() * (*posCurrent).second;
+        eMax = testEcalRecHit.energy() * posCurrent.second;
         eMaxId = testEcalRecHit.id();
       }
     }
@@ -84,21 +83,20 @@ void ClusterShapeAlgo::Calculate_2ndEnergy(const reco::BasicCluster &passedClust
   double e2nd=0;
   DetId e2ndId(0);
 
-  std::vector< std::pair<DetId, float> > clusterDetIds = passedCluster.hitsAndFractions();
-  std::vector< std::pair<DetId, float> >::iterator posCurrent;
+  const std::vector< std::pair<DetId, float> >& clusterDetIds = passedCluster.hitsAndFractions();
 
   EcalRecHit testEcalRecHit;
 
-  for(posCurrent = clusterDetIds.begin(); posCurrent != clusterDetIds.end(); posCurrent++)
+  for(auto const& posCurrent : clusterDetIds)
   { 
-    if (( (*posCurrent).first != DetId(0)) && (hits->find( (*posCurrent).first ) != hits->end()))
+    if (( posCurrent.first != DetId(0)) && (hits->find( posCurrent.first ) != hits->end()))
     {
-      EcalRecHitCollection::const_iterator itt = hits->find( (*posCurrent).first );
+      EcalRecHitCollection::const_iterator itt = hits->find( posCurrent.first );
       testEcalRecHit = *itt;
 
-      if(testEcalRecHit.energy() * (*posCurrent).second > e2nd && testEcalRecHit.id() != eMaxId_)
+      if(testEcalRecHit.energy() * posCurrent.second > e2nd && testEcalRecHit.id() != eMaxId_)
       {
-        e2nd = testEcalRecHit.energy() * (*posCurrent).second;
+        e2nd = testEcalRecHit.energy() * posCurrent.second;
         e2ndId = testEcalRecHit.id();
       }
     }
@@ -386,21 +384,21 @@ void ClusterShapeAlgo::Calculate_BarrelBasketEnergyFraction(const reco::BasicClu
                                                             const int EtaPhi,
                                                             const CaloSubdetectorGeometry* geometry) 
 {
-  if(  (hits!=0) && ( ((*hits)[0]).id().subdetId() != EcalBarrel )  ) {
+  if(  (hits!=nullptr) && ( ((*hits)[0]).id().subdetId() != EcalBarrel )  ) {
      //std::cout << "No basket correction for endacap!" << std::endl;
      return;
   }
 
   std::map<int,double> indexedBasketEnergy;
-  std::vector< std::pair<DetId, float> > clusterDetIds = passedCluster.hitsAndFractions();
+  const std::vector< std::pair<DetId, float> >& clusterDetIds = passedCluster.hitsAndFractions();
   const EcalBarrelGeometry* subDetGeometry = (const EcalBarrelGeometry*) geometry;
 
-  for(std::vector< std::pair<DetId, float> >::iterator posCurrent = clusterDetIds.begin(); posCurrent != clusterDetIds.end(); posCurrent++)
+  for(auto const& posCurrent : clusterDetIds)
   {
     int basketIndex = 999;
 
     if(EtaPhi == Eta) {
-      int unsignedIEta = abs(EBDetId( (*posCurrent).first ).ieta());
+      int unsignedIEta = abs(EBDetId( posCurrent.first ).ieta());
       std::vector<int> etaBasketSize = subDetGeometry->getEtaBaskets();
 
       for(unsigned int i = 0; i < etaBasketSize.size(); i++) {
@@ -411,20 +409,20 @@ void ClusterShapeAlgo::Calculate_BarrelBasketEnergyFraction(const reco::BasicClu
           break;
         }
       }
-      basketIndex = (basketIndex+1)*(EBDetId( (*posCurrent).first ).ieta() > 0 ? 1 : -1);
+      basketIndex = (basketIndex+1)*(EBDetId( posCurrent.first ).ieta() > 0 ? 1 : -1);
 
     } else if(EtaPhi == Phi) {
       int halfNumBasketInPhi = (EBDetId::MAX_IPHI - EBDetId::MIN_IPHI + 1)/subDetGeometry->getBasketSizeInPhi()/2;
 
-      basketIndex = (EBDetId( (*posCurrent).first ).iphi() - 1)/subDetGeometry->getBasketSizeInPhi()
+      basketIndex = (EBDetId( posCurrent.first ).iphi() - 1)/subDetGeometry->getBasketSizeInPhi()
                   - (EBDetId( (clusterDetIds[0]).first ).iphi() - 1)/subDetGeometry->getBasketSizeInPhi();
 
       if(basketIndex >= halfNumBasketInPhi)             basketIndex -= 2*halfNumBasketInPhi;
       else if(basketIndex <  -1*halfNumBasketInPhi)     basketIndex += 2*halfNumBasketInPhi;
 
     } else throw(std::runtime_error("\n\nOh No! Calculate_BarrelBasketEnergyFraction called on invalid index.\n\n"));
 
-    indexedBasketEnergy[basketIndex] += (hits->find( (*posCurrent).first ))->energy();
+    indexedBasketEnergy[basketIndex] += (hits->find( posCurrent.first ))->energy();
   }
 
   std::vector<double> energyFraction;
@@ -609,17 +607,16 @@ void ClusterShapeAlgo::Calculate_EnergyDepTopology (const reco::BasicCluster &pa
   theta_axis *= 1.0/theta_axis.mag();
   CLHEP::Hep3Vector phi_axis = theta_axis.cross(clDir);
 
-  std::vector< std::pair<DetId, float> > clusterDetIds = passedCluster.hitsAndFractions();
+  const std::vector< std::pair<DetId, float> >& clusterDetIds = passedCluster.hitsAndFractions();
 
   EcalClusterEnergyDeposition clEdep;
   EcalRecHit testEcalRecHit;
-  std::vector< std::pair<DetId, float> >::iterator posCurrent;
   // loop over crystals
-  for(posCurrent=clusterDetIds.begin(); posCurrent!=clusterDetIds.end(); ++posCurrent) {
-    EcalRecHitCollection::const_iterator itt = hits->find( (*posCurrent).first );
+  for(auto const& posCurrent : clusterDetIds) {
+    EcalRecHitCollection::const_iterator itt = hits->find( posCurrent.first );
     testEcalRecHit=*itt;
 
-    if(( (*posCurrent).first != DetId(0)) && (hits->find( (*posCurrent).first ) != hits->end())) {
+    if(( posCurrent.first != DetId(0)) && (hits->find( posCurrent.first ) != hits->end())) {
       clEdep.deposited_energy = testEcalRecHit.energy();
 
       // if logarithmic weight is requested, apply cut on minimum energy of the recHit
@@ -638,7 +635,7 @@ void ClusterShapeAlgo::Calculate_EnergyDepTopology (const reco::BasicCluster &pa
         }
         else LogDebug("ClusterShapeAlgo") << "===> got crystal. Energy = " << clEdep.deposited_energy << " GeV. ";
       }
-      DetId id_ = (*posCurrent).first;
+      DetId id_ = posCurrent.first;
       const CaloCellGeometry *this_cell = geometry->getGeometry(id_);
       GlobalPoint cellPos = this_cell->getPosition();
       CLHEP::Hep3Vector gblPos (cellPos.x(),cellPos.y(),cellPos.z()); //surface position?

RecoParticleFlow/PFClusterProducer/interface/PFRecHitQTests.h

@@ -6,7 +6,6 @@
 #include "Geometry/Records/interface/HcalRecNumberingRecord.h"
 
 #include <iostream>
-using namespace std;
 
 //
 //  Quality test that checks threshold

RecoParticleFlow/PFClusterProducer/src/Basic2DGenericPFlowPositionCalc.cc

@@ -82,16 +82,18 @@ calculateAndSetPositionActual(reco::PFCluster& cluster) const {
   double depth = 0.0;  
   double position_norm = 0.0;
   double x(0.0),y(0.0),z(0.0);
-  const reco::PFRecHitRefVector* seedNeighbours = NULL;
+  const reco::PFRecHitRefVector* seedNeighbours = nullptr;
   switch( _posCalcNCrystals ) {
   case 5:
     seedNeighbours = &refseed->neighbours4();
     break;
   case 9:
     seedNeighbours = &refseed->neighbours8();
     break;
-  default:
+  case -1:
     break;
+  default:
+    assert(0); //bug
   }
 
   for( const reco::PFRecHitFraction& rhf : cluster.recHitFractions() ) {