From 9cfb62cf07851671d47b3bc7bfacd45118326401 Mon Sep 17 00:00:00 2001 From: Christopher Jones Date: Tue, 24 Sep 2024 11:57:57 -0500 Subject: [PATCH] Improve memory handling in CmsDetConstruction static analyzer identified a code path that would leak memory. Changing to std::unique_ptr solves the issue. --- .../plugins/CmsDetConstruction.cc | 45 ++++++++++--------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/Geometry/TrackerNumberingBuilder/plugins/CmsDetConstruction.cc b/Geometry/TrackerNumberingBuilder/plugins/CmsDetConstruction.cc index 6da97e59f249f..738484d5835d1 100644 --- a/Geometry/TrackerNumberingBuilder/plugins/CmsDetConstruction.cc +++ b/Geometry/TrackerNumberingBuilder/plugins/CmsDetConstruction.cc @@ -4,14 +4,15 @@ #include "Geometry/TrackerNumberingBuilder/plugins/ExtractStringFromDDD.h" #include "DataFormats/DetId/interface/DetId.h" #include "FWCore/MessageLogger/interface/MessageLogger.h" +#include template void CmsDetConstruction::buildSmallDetsforGlued(FilteredView& fv, GeometricDet* mother, const std::string& attribute) { - GeometricDet* det = new GeometricDet(&fv, - CmsTrackerLevelBuilder::theCmsTrackerStringToEnum.type( - ExtractStringFromDDD::getString(attribute, &fv))); + auto det = std::make_unique(&fv, + CmsTrackerLevelBuilder::theCmsTrackerStringToEnum.type( + ExtractStringFromDDD::getString(attribute, &fv))); if (det->stereo()) { uint32_t temp = 1; det->setGeographicalID(DetId(temp)); @@ -20,16 +21,16 @@ void CmsDetConstruction::buildSmallDetsforGlued(FilteredView& fv, det->setGeographicalID(DetId(temp)); } - mother->addComponent(det); + mother->addComponent(det.release()); } template void CmsDetConstruction::buildSmallDetsforStack(FilteredView& fv, GeometricDet* mother, const std::string& attribute) { - GeometricDet* det = new GeometricDet(&fv, - CmsTrackerLevelBuilder::theCmsTrackerStringToEnum.type( - ExtractStringFromDDD::getString(attribute, &fv))); + auto det = std::make_unique(&fv, + CmsTrackerLevelBuilder::theCmsTrackerStringToEnum.type( + ExtractStringFromDDD::getString(attribute, &fv))); if (det->isLowerSensor()) { uint32_t temp = 1; @@ -40,16 +41,16 @@ void CmsDetConstruction::buildSmallDetsforStack(FilteredView& fv, } else { edm::LogError("DetConstruction") << " module defined in a Stack but not upper either lower!? "; } - mother->addComponent(det); + mother->addComponent(det.release()); } template void CmsDetConstruction::buildSmallDetsfor3D(FilteredView& fv, GeometricDet* mother, const std::string& attribute) { - GeometricDet* det = new GeometricDet(&fv, - CmsTrackerLevelBuilder::theCmsTrackerStringToEnum.type( - ExtractStringFromDDD::getString(attribute, &fv))); + auto det = std::make_unique(&fv, + CmsTrackerLevelBuilder::theCmsTrackerStringToEnum.type( + ExtractStringFromDDD::getString(attribute, &fv))); if (det->isFirstSensor()) { uint32_t temp = 1; @@ -60,7 +61,7 @@ void CmsDetConstruction::buildSmallDetsfor3D(FilteredView& fv, } else { edm::LogError("DetConstruction") << " module defined in a 3D module but not first or second sensor!? "; } - mother->addComponent(det); + mother->addComponent(det.release()); } /* @@ -79,7 +80,7 @@ void CmsDetConstruction::buildComponent(DDFilteredView& fv, const GeometricDet::GDEnumType& myTopologicalType = CmsTrackerLevelBuilder::theCmsTrackerStringToEnum.type(myTopologicalNameInXMLs); - GeometricDet* det = new GeometricDet(&fv, myTopologicalType); + auto det = std::make_unique(&fv, myTopologicalType); const bool isPhase1ModuleWith2Sensors = (myTopologicalType == GeometricDet::mergedDet); const bool isPhase2ModuleWith2Sensors = (myTopologicalType == GeometricDet::OTPhase2Stack); @@ -92,13 +93,13 @@ void CmsDetConstruction::buildComponent(DDFilteredView& fv, while (dodets) { // PHASE 1 (MERGEDDET) if (isPhase1ModuleWith2Sensors) { - buildSmallDetsforGlued(fv, det, attribute); + buildSmallDetsforGlued(fv, det.get(), attribute); } // PHASE 2 (STACKDET) else if (isPhase2ModuleWith2Sensors) { - buildSmallDetsforStack(fv, det, attribute); + buildSmallDetsforStack(fv, det.get(), attribute); } else if (isPhase2BarrelModuleWith2Sensors) { - buildSmallDetsfor3D(fv, det, attribute); + buildSmallDetsfor3D(fv, det.get(), attribute); } dodets = fv.nextSibling(); @@ -111,7 +112,7 @@ void CmsDetConstruction::buildComponent(DDFilteredView& fv, // Indeed, we are not going to sort sensors within module, if there is only 1 sensor! // ALL CASES: add sensor to its mother volume (module or ladder). - mother->addComponent(det); + mother->addComponent(det.release()); } /* @@ -129,7 +130,7 @@ void CmsDetConstruction::buildComponent(cms::DDFilteredView const std::string& myTopologicalNameInXMLs = ExtractStringFromDDD::getString(attribute, &fv); const GeometricDet::GDEnumType& myTopologicalType = CmsTrackerLevelBuilder::theCmsTrackerStringToEnum.type(myTopologicalNameInXMLs); - GeometricDet* det = new GeometricDet(&fv, myTopologicalType); + auto det = std::make_unique(&fv, myTopologicalType); const bool isPhase1ModuleWith2Sensors = (myTopologicalType == GeometricDet::mergedDet); const bool isPhase2ModuleWith2Sensors = (myTopologicalType == GeometricDet::OTPhase2Stack); @@ -150,13 +151,13 @@ void CmsDetConstruction::buildComponent(cms::DDFilteredView while (fv.level() == sensorHierarchyLevel) { // PHASE 1 (MERGEDDET) if (isPhase1ModuleWith2Sensors) { - buildSmallDetsforGlued(fv, det, attribute); + buildSmallDetsforGlued(fv, det.get(), attribute); } // PHASE 2 (STACKDET) else if (isPhase2ModuleWith2Sensors) { - buildSmallDetsforStack(fv, det, attribute); + buildSmallDetsforStack(fv, det.get(), attribute); } else if (isPhase2BarrelModuleWith2Sensors) { - buildSmallDetsfor3D(fv, det, attribute); + buildSmallDetsfor3D(fv, det.get(), attribute); } // Go to the next volume in FilteredView. @@ -174,5 +175,5 @@ void CmsDetConstruction::buildComponent(cms::DDFilteredView } // ALL CASES: add sensor to its mother volume (module or ladder). - mother->addComponent(det); + mother->addComponent(det.release()); }