The battle agains coverity

AIDASoft · May 23, 2024 · bf08cba · bf08cba
1 parent d72b4e9
commit bf08cba
Show file tree

Hide file tree

Showing 35 changed files with 283 additions and 214 deletions.
diff --git a/DDCond/src/ConditionsContent.cpp b/DDCond/src/ConditionsContent.cpp
@@ -146,7 +146,7 @@ ConditionsContent::addDependency(DetElement de,
                                  Condition::itemkey_type item,
                                  std::shared_ptr<ConditionUpdateCall> callback)
 {
-  ConditionDependency* dep = new ConditionDependency(de, item, callback);
+  ConditionDependency* dep = new ConditionDependency(de, item, std::move(callback));
   return addDependency(dep);
 }
 
diff --git a/DDCond/src/plugins/ConditionsPlugins.cpp b/DDCond/src/plugins/ConditionsPlugins.cpp
@@ -436,7 +436,7 @@ static void* create_printer(Detector& description, int argc,char** argv)  {
   }
   PRINTER* p = (flags) ? new PRINTER(slice, prefix, flags) : new PRINTER(slice, prefix);
   p->printLevel = print_level;
-  if ( !name.empty() ) p->name = name;
+  if ( !name.empty() ) p->name = std::move(name);
   return (void*)dynamic_cast<WRAPPER*>(createProcessorWrapper(p));
 }
 

diff --git a/DDCond/src/plugins/ConditionsXmlLoader.cpp b/DDCond/src/plugins/ConditionsXmlLoader.cpp
@@ -175,6 +175,6 @@ std::size_t ConditionsXmlLoader::load_range(key_type key,
     }
     keep.emplace_back(condition);
   }
-  m_buffer = keep;
+  m_buffer = std::move(keep);
   return conditions.size()-len;
 }
diff --git a/DDCore/include/DD4hep/GeoHandler.h b/DDCore/include/DD4hep/GeoHandler.h
@@ -51,17 +51,6 @@ namespace dd4hep {
      */
     class GeoHandlerTypes {
     public:
-#if 0
-      typedef std::set<const TGeoVolume*> ConstVolumeSet;
-      typedef std::map<SensitiveDetector, ConstVolumeSet> SensitiveVolumes;
-      typedef std::map<Region,   ConstVolumeSet>          RegionVolumes;
-      typedef std::map<LimitSet, ConstVolumeSet>          LimitVolumes;
-      typedef std::map<int, std::set<const TGeoNode*> >   Data;
-      typedef std::set<SensitiveDetector>                 SensitiveDetectorSet;
-      typedef std::set<Region>                            RegionSet;
-      typedef std::set<LimitSet>                          LimitSetSet;
-      typedef std::set<TNamed*>                           ObjectSet;
-#endif
       /// Data container to store information obtained during the geometry scan
       /**
        *  \author  M.Frank
@@ -97,13 +86,13 @@ namespace dd4hep {
     class GeoHandler: public GeoHandlerTypes {
 
     protected:
-      bool  m_propagateRegions;
-      std::map<int, std::set<const TGeoNode*> >* m_data;
-
+      bool  m_propagateRegions { false };
+      std::map<int, std::set<const TGeoNode*> >*    m_data      { nullptr };
+      std::map<const TGeoNode*, std::vector<TGeoNode*> >* m_daughters { nullptr };
       /// Internal helper to collect geometry information from traversal
       GeoHandler& i_collect(const TGeoNode* parent,
-			    const TGeoNode* node,
-			    int level, Region rg, LimitSet ls);
+                            const TGeoNode* node,
+                            int level, Region rg, LimitSet ls);
 
     private:
       /// Never call Copy constructor
@@ -118,7 +107,8 @@ namespace dd4hep {
       /// Default constructor
       GeoHandler();
       /// Initializing constructor
-      GeoHandler(std::map<int, std::set<const TGeoNode*> >* ptr);
+      GeoHandler(std::map<int, std::set<const TGeoNode*> >* ptr,
+                 std::map<const TGeoNode*, std::vector<TGeoNode*> >* daus = nullptr);
       /// Default destructor
       virtual ~GeoHandler();
       /// Propagate regions. Returns the previous value

diff --git a/DDCore/src/GeoHandler.cpp b/DDCore/src/GeoHandler.cpp
@@ -50,13 +50,15 @@ namespace {
 }
 
 /// Default constructor
-detail::GeoHandler::GeoHandler() : m_propagateRegions(false)  {
+detail::GeoHandler::GeoHandler()  {
   m_data = new std::map<int, std::set<const TGeoNode*> >();
 }
 
 /// Initializing constructor
-detail::GeoHandler::GeoHandler(std::map<int, std::set<const TGeoNode*> >* ptr)
-  : m_propagateRegions(false), m_data(ptr) {
+detail::GeoHandler::GeoHandler(std::map<int, std::set<const TGeoNode*> >* ptr,
+                               std::map<const TGeoNode*, std::vector<TGeoNode*> >* daus)
+  : m_data(ptr), m_daughters(daus)
+{
 }
 
 /// Default destructor
@@ -91,15 +93,15 @@ detail::GeoHandler& detail::GeoHandler::collect(DetElement element, GeometryInfo
   TGeoNode* par_node = par.isValid() ? par.placement().ptr() : nullptr;
   m_data->clear();
   i_collect(par_node, element.placement().ptr(), 0, Region(), LimitSet());
-  for (auto i = m_data->rbegin(); i != m_data->rend(); ++i) {
+  for ( auto i = m_data->rbegin(); i != m_data->rend(); ++i ) {
     const auto& mapped = (*i).second;
-    for (const TGeoNode* n : mapped )  {
+    for ( const TGeoNode* n : mapped )  {
       TGeoVolume* v = n->GetVolume();
-      if (v) {
+      if ( v ) {
         Material mat(v->GetMedium());
         Volume   vol(v);
         // Note : assemblies and the world do not have a real volume nor a material
-        if (info.volumeSet.find(vol) == info.volumeSet.end()) {
+        if ( info.volumeSet.find(vol) == info.volumeSet.end() ) {
           info.volumeSet.emplace(vol);
           info.volumes.emplace_back(vol);
         }
@@ -128,35 +130,38 @@ detail::GeoHandler& detail::GeoHandler::i_collect(const TGeoNode* /* parent */,
                                                   const TGeoNode*    current,
                                                   int level, Region rg, LimitSet ls)
 {
-  TGeoVolume* volume = current->GetVolume();
-  TObjArray*  nodes = volume->GetNodes();
-  int         num_children = nodes ? nodes->GetEntriesFast() : 0;
-  Volume      vol(volume);
-  Region      region = vol.region();
-  LimitSet    limits = vol.limitSet();
+  TGeoVolume* vol    = current->GetVolume();
+  TObjArray*  nodes  = vol->GetNodes();
+  Volume      volume = vol;
+  Region      region = volume.region();
+  LimitSet    limits = volume.limitSet();
 
   if ( m_propagateRegions )  {
     if ( !region.isValid() && rg.isValid() )   {
       region = rg;
-      vol.setRegion(region);
+      volume.setRegion(region);
     }
     if ( !limits.isValid() && ls.isValid() )  {
       limits = ls;
-      vol.setLimitSet(limits);
+      volume.setLimitSet(limits);
     }
   }
+  /// Collect the hierarchy of placements
   (*m_data)[level].emplace(current);
-  if (num_children > 0) {
-    for (int i = 0; i < num_children; ++i) {
-      TGeoNode* node = (TGeoNode*) nodes->At(i);
-      i_collect(current, node, level + 1, region, limits);
-    }
+  int num = nodes ? nodes->GetEntriesFast() : 0;
+  for (int i = 0; i < num; ++i)
+    i_collect(current, (TGeoNode*)nodes->At(i), level + 1, region, limits);
+  /// Now collect all the daughters of this volume, so that we can reconnect them in the correct order
+  if ( m_daughters && m_daughters->find(current) == m_daughters->end() )  {
+    auto [idau,success] = m_daughters->emplace(current, std::vector<TGeoNode*>());
+    for (int i = 0; i < num; ++i)
+      idau->second.push_back((TGeoNode*)nodes->At(i));
   }
   return *this;
 }
 
 /// Initializing constructor
-detail::GeoScan::GeoScan(DetElement e) {
+detail::GeoScan::GeoScan(DetElement e)  {
   m_data = GeoHandler().collect(e).release();
 }
 

diff --git a/DDCore/src/plugins/CodeGenerator.cpp b/DDCore/src/plugins/CodeGenerator.cpp
@@ -302,7 +302,7 @@ namespace {
         log << "\t structure[" << pointer(de) << "] = de; " << newline
             << "}"   << newline;
       }
-      for(auto d : de.children() )  {
+      for(const auto& d : de.children() )  {
         handleStructure(log, de, d.second);
       }
       if ( !parent.isValid() )   {

diff --git a/DDCore/src/plugins/DetectorChecksum.cpp b/DDCore/src/plugins/DetectorChecksum.cpp
@@ -138,13 +138,17 @@ DetectorChecksum::~DetectorChecksum() {
 template <typename T> std::string DetectorChecksum::refName(T handle)  const  {
   std::string nam = handle->GetName();
   std::size_t idx = nam.find("_0x");
-  return idx == std::string::npos ? nam : nam.substr(0, idx);
+  if ( idx == std::string::npos ) return nam;
+  return nam.substr(0, idx);
 }
+
 template <> std::string DetectorChecksum::refName(Segmentation handle)  const  {
   std::string nam = handle->name();
   std::size_t idx = nam.find("_0x");
-  return idx == std::string::npos ? nam : nam.substr(0, idx);
+  if ( idx == std::string::npos ) return nam;
+  return nam.substr(0, idx);
 }
+
 template <typename T> std::string DetectorChecksum::attr_name(T handle)  const  {
   std::string n = " name=\"" + refName(handle)+"\"";
   return n;

diff --git a/DDCore/src/plugins/VisProcessor.cpp b/DDCore/src/plugins/VisProcessor.cpp
@@ -228,8 +228,7 @@ static void* create_object(Detector& description, int argc, char** argv)   {
         printout(ERROR,"VisMaterialProcessor","++ Invalid DetElement path: %s",path.c_str());
       }
       else if ( ::strncmp(argv[i],"-name",4) == 0 )   {
-        std::string name = argv[++i];
-        proc->name = name;
+        proc->name = argv[++i];
         continue;
       }
       else if ( ::strncmp(argv[i],"-show",4) == 0 )   {

diff --git a/DDDigi/include/DDDigi/DigiData.h b/DDDigi/include/DDDigi/DigiData.h
@@ -74,6 +74,7 @@ namespace dd4hep {
         mask_type    mask;
         itemkey_type item;
       } values;
+
     public:
       /// Default constructor
       Key();
@@ -884,13 +885,13 @@ namespace dd4hep {
     public: 
       template <typename T> using map_type_t = std::map<std::string, std::vector<T> >;
       struct header_data  {
-	map_type_t<std::string> stringParams;
-	map_type_t<float>       floatParams;
-	map_type_t<int>         intParams;
-	std::int32_t            run_number    {  -1 };
-	std::int32_t            event_number  {  -1 };
-	std::int64_t            time_stamp    {   0 };
-	double                  event_weight  { 0e0 };
+        map_type_t<std::string> stringParams;
+        map_type_t<float>       floatParams;
+        map_type_t<int>         intParams;
+        std::int32_t            run_number    {  -1 };
+        std::int32_t            event_number  {  -1 };
+        std::int64_t            time_stamp    {   0 };
+        double                  event_weight  { 0e0 };
       };
       std::shared_ptr<header_data> data;
     public: 
@@ -912,35 +913,35 @@ namespace dd4hep {
       std::size_t size()  const;
       /// Access the event number
       std::int32_t getEventNumber() const   {
-	return data->event_number;
+        return data->event_number;
       }
       /// Access the run number
       std::int32_t getRunNumber() const   {
-	return data->run_number;
+        return data->run_number;
       }
       /// Access the time stamp
       std::uint64_t getTimeStamp() const   {
-	return data->time_stamp;
+        return data->time_stamp;
       }
       /// Access the event weight
       float getWeight() const   {
-	return data->event_weight;
+        return data->event_weight;
       }
       /// Set the event number
       void setEventNumber(std::int32_t value)   {
-	data->event_number = value;
+        data->event_number = value;
       }
       /// Set the run number
       void setRunNumber(std::int32_t value)   {
-	data->run_number = value;
+        data->run_number = value;
       }
       /// Set the time stamp
       void setTimeStamp(std::uint64_t value)   {
-	data->time_stamp = value;
+        data->time_stamp = value;
       }
       /// Set the event weight
       void setWeight(float value)   {
-	data->event_weight = value;
+        data->event_weight = value;
       }
     };
 
@@ -1053,24 +1054,24 @@ namespace dd4hep {
     template<typename DATA> inline DATA& DataSegment::get(Key key)     {
       if ( DATA* ptr = std::any_cast<DATA>(this->get_item(key, true)) )
         return *ptr;
-      throw std::runtime_error(this->invalid_cast(key, typeid(DATA)));
+      throw std::runtime_error(this->invalid_cast(std::move(key), typeid(DATA)));
     }
     /// Access data as reference by key. If not existing, an exception is thrown
     template<typename DATA> inline const DATA& DataSegment::get(Key key)  const   {
       if ( const DATA* ptr = std::any_cast<DATA>(this->get_item(key, true)) )
         return *ptr;
-      throw std::runtime_error(this->invalid_cast(key, typeid(DATA)));
+      throw std::runtime_error(this->invalid_cast(std::move(key), typeid(DATA)));
     }
 
     /// Access data as pointers by key. If not existing, nullptr is returned
     template<typename DATA> inline DATA* DataSegment::pointer(Key key)     {
-      if ( DATA* ptr = std::any_cast<DATA>(this->get_item(key, false)) )
+      if ( DATA* ptr = std::any_cast<DATA>(this->get_item(std::move(key), false)) )
         return ptr;
       return nullptr;
     }
     /// Access data as pointers by key. If not existing, nullptr is returned
     template<typename DATA> inline const DATA* DataSegment::pointer(Key key)  const   {
-      if ( const DATA* ptr = std::any_cast<DATA>(this->get_item(key, false)) )
+      if ( const DATA* ptr = std::any_cast<DATA>(this->get_item(std::move(key), false)) )
         return ptr;
       return nullptr;
     }
@@ -1080,14 +1081,14 @@ namespace dd4hep {
       key.set_segment(this->id);
       value.key.set_segment(this->id);
       std::any item = std::make_any<DATA>(std::move(value));
-      return this->emplace_any(key, std::move(item));
+      return this->emplace_any(std::move(key), std::move(item));
     }
 
     /// Helper to place data to data segment
     template <typename KEY, typename DATA> 
     bool put_data(DataSegment& segment, KEY key, DATA&& value)    {
       std::any item = std::make_any<DATA>(std::move(value));
-      return segment.emplace_any(key, std::move(item));
+      return segment.emplace_any(std::move(key), std::move(item));
     }
 
     ///  User event data for DDDigi

diff --git a/DDDigi/io/DigiIO.cpp b/DDDigi/io/DigiIO.cpp
@@ -498,18 +498,16 @@ namespace dd4hep {
       Key history_key;
       EnergyDeposit dep { };
       const auto* h = depo.second.get();
-      Position pos = h->position;
-      pos *= 1./dd4hep::mm;
 
       dep.flag = h->flag;
       dep.deposit = h->energyDeposit;
-      dep.position = pos;
+      dep.position = (h->position / dd4hep::mm);
 
       history_key.set_mask(key.mask());
       history_key.set_item(out.size());
       history_key.set_segment(key.segment());
       dep.history.hits.emplace_back(history_key, dep.deposit);
-      add_particle_history(h, history_key, dep.history);
+      add_particle_history(h, std::move(history_key), dep.history);
       out.emplace(depo.first, std::move(dep));
     }