From 2f76a4299dde96d6dff4ba9bb2d810c65d40d372 Mon Sep 17 00:00:00 2001 From: Hans Johnson Date: Tue, 31 Mar 2026 09:47:58 -0500 Subject: [PATCH 1/3] COMP: Use std::unique_ptr in FEMLinearSystemWrapperDenseVNL Replace raw pointer members and new/delete with std::unique_ptr and std::make_unique. Use std::swap for all Swap* methods. Destructor is defaulted since unique_ptr handles cleanup. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../itkFEMLinearSystemWrapperDenseVNL.h | 14 ++-- .../src/itkFEMLinearSystemWrapperDenseVNL.cxx | 68 ++++--------------- 2 files changed, 19 insertions(+), 63 deletions(-) diff --git a/Modules/Numerics/FEM/include/itkFEMLinearSystemWrapperDenseVNL.h b/Modules/Numerics/FEM/include/itkFEMLinearSystemWrapperDenseVNL.h index 5218adb20b0..2b782e15a98 100644 --- a/Modules/Numerics/FEM/include/itkFEMLinearSystemWrapperDenseVNL.h +++ b/Modules/Numerics/FEM/include/itkFEMLinearSystemWrapperDenseVNL.h @@ -48,13 +48,13 @@ class ITKFEM_EXPORT LinearSystemWrapperDenseVNL : public LinearSystemWrapper using MatrixRepresentation = vnl_matrix; /* matrix holder type alias */ - using MatrixHolder = std::vector; + using MatrixHolder = std::vector>; /* constructor & destructor */ LinearSystemWrapperDenseVNL() : LinearSystemWrapper() {} - ~LinearSystemWrapperDenseVNL() override; + ~LinearSystemWrapperDenseVNL() override = default; /* memory management routines */ void @@ -169,15 +169,11 @@ class ITKFEM_EXPORT LinearSystemWrapperDenseVNL : public LinearSystemWrapper MultiplyMatrixVector(unsigned int resultVectorIndex, unsigned int matrixIndex, unsigned int vectorIndex) override; private: - /** vector of pointers to VNL sparse matrices */ - // std::vector< vnl_sparse_matrix* > *m_Matrices; - MatrixHolder * m_Matrices{ nullptr }; + std::unique_ptr m_Matrices{}; - /** vector of pointers to VNL vectors */ - std::vector *> * m_Vectors{ nullptr }; + std::unique_ptr>>> m_Vectors{}; - /** vector of pointers to VNL vectors */ - std::vector *> * m_Solutions{ nullptr }; + std::unique_ptr>>> m_Solutions{}; }; } // namespace itk::fem diff --git a/Modules/Numerics/FEM/src/itkFEMLinearSystemWrapperDenseVNL.cxx b/Modules/Numerics/FEM/src/itkFEMLinearSystemWrapperDenseVNL.cxx index 7a4da10d054..4dd11cf8fc5 100644 --- a/Modules/Numerics/FEM/src/itkFEMLinearSystemWrapperDenseVNL.cxx +++ b/Modules/Numerics/FEM/src/itkFEMLinearSystemWrapperDenseVNL.cxx @@ -26,13 +26,10 @@ LinearSystemWrapperDenseVNL::InitializeMatrix(unsigned int matrixIndex) // allocate if necessary if (m_Matrices == nullptr) { - m_Matrices = new MatrixHolder(m_NumberOfMatrices); + m_Matrices = std::make_unique(m_NumberOfMatrices); } - // out with old, in with new - delete (*m_Matrices)[matrixIndex]; - - (*m_Matrices)[matrixIndex] = new MatrixRepresentation(this->GetSystemOrder(), this->GetSystemOrder()); + (*m_Matrices)[matrixIndex] = std::make_unique(this->GetSystemOrder(), this->GetSystemOrder()); (*m_Matrices)[matrixIndex]->fill(0.0); } @@ -56,8 +53,7 @@ LinearSystemWrapperDenseVNL::DestroyMatrix(unsigned int matrixIndex) { if (m_Matrices) { - delete (*m_Matrices)[matrixIndex]; - (*m_Matrices)[matrixIndex] = nullptr; + (*m_Matrices)[matrixIndex].reset(); } } @@ -67,13 +63,10 @@ LinearSystemWrapperDenseVNL::InitializeVector(unsigned int vectorIndex) // allocate if necessary if (m_Vectors == nullptr) { - m_Vectors = new std::vector *>(m_NumberOfVectors); + m_Vectors = std::make_unique>>>(m_NumberOfVectors); } - // out with old, in with new - delete (*m_Vectors)[vectorIndex]; - - (*m_Vectors)[vectorIndex] = new vnl_vector(this->GetSystemOrder()); + (*m_Vectors)[vectorIndex] = std::make_unique>(this->GetSystemOrder()); (*m_Vectors)[vectorIndex]->fill(0.0); } @@ -97,8 +90,7 @@ LinearSystemWrapperDenseVNL::DestroyVector(unsigned int vectorIndex) { if (m_Vectors) { - delete (*m_Vectors)[vectorIndex]; - (*m_Vectors)[vectorIndex] = nullptr; + (*m_Vectors)[vectorIndex].reset(); } } @@ -108,13 +100,10 @@ LinearSystemWrapperDenseVNL::InitializeSolution(unsigned int solutionIndex) // allocate if necessary if (m_Solutions == nullptr) { - m_Solutions = new std::vector *>(m_NumberOfSolutions); + m_Solutions = std::make_unique>>>(m_NumberOfSolutions); } - // out with old, in with new - delete (*m_Solutions)[solutionIndex]; - - (*m_Solutions)[solutionIndex] = new vnl_vector(this->GetSystemOrder()); + (*m_Solutions)[solutionIndex] = std::make_unique>(this->GetSystemOrder()); (*m_Solutions)[solutionIndex]->fill(0.0); } @@ -138,8 +127,7 @@ LinearSystemWrapperDenseVNL::DestroySolution(unsigned int solutionIndex) { if (m_Solutions) { - delete (*m_Solutions)[solutionIndex]; - (*m_Solutions)[solutionIndex] = nullptr; + (*m_Solutions)[solutionIndex].reset(); } } @@ -189,39 +177,31 @@ LinearSystemWrapperDenseVNL::Solve() void LinearSystemWrapperDenseVNL::SwapMatrices(unsigned int MatrixIndex1, unsigned int MatrixIndex2) { - vnl_matrix * tmp = (*m_Matrices)[MatrixIndex1]; - (*m_Matrices)[MatrixIndex1] = (*m_Matrices)[MatrixIndex2]; - (*m_Matrices)[MatrixIndex2] = tmp; + std::swap((*m_Matrices)[MatrixIndex1], (*m_Matrices)[MatrixIndex2]); } void LinearSystemWrapperDenseVNL::SwapVectors(unsigned int VectorIndex1, unsigned int VectorIndex2) { - vnl_vector tmp = *(*m_Vectors)[VectorIndex1]; - *(*m_Vectors)[VectorIndex1] = *(*m_Vectors)[VectorIndex2]; - *(*m_Vectors)[VectorIndex2] = tmp; + std::swap((*m_Vectors)[VectorIndex1], (*m_Vectors)[VectorIndex2]); } void LinearSystemWrapperDenseVNL::SwapSolutions(unsigned int SolutionIndex1, unsigned int SolutionIndex2) { - vnl_vector * tmp = (*m_Solutions)[SolutionIndex1]; - (*m_Solutions)[SolutionIndex1] = (*m_Solutions)[SolutionIndex2]; - (*m_Solutions)[SolutionIndex2] = tmp; + std::swap((*m_Solutions)[SolutionIndex1], (*m_Solutions)[SolutionIndex2]); } void LinearSystemWrapperDenseVNL::CopySolution2Vector(unsigned int SolutionIndex, unsigned int VectorIndex) { - delete (*m_Vectors)[VectorIndex]; - (*m_Vectors)[VectorIndex] = new vnl_vector(*((*m_Solutions)[SolutionIndex])); + (*m_Vectors)[VectorIndex] = std::make_unique>(*((*m_Solutions)[SolutionIndex])); } void LinearSystemWrapperDenseVNL::CopyVector2Solution(unsigned int VectorIndex, unsigned int SolutionIndex) { - delete (*m_Solutions)[SolutionIndex]; - (*m_Solutions)[SolutionIndex] = new vnl_vector(*((*m_Vectors)[VectorIndex])); + (*m_Solutions)[SolutionIndex] = std::make_unique>(*((*m_Vectors)[VectorIndex])); } void @@ -266,24 +246,4 @@ LinearSystemWrapperDenseVNL::ScaleSolution(Float scale, unsigned int solutionInd (*(*m_Solutions)[solutionIndex]) = (*(*m_Solutions)[solutionIndex]) * scale; } -LinearSystemWrapperDenseVNL::~LinearSystemWrapperDenseVNL() -{ - for (unsigned int i = 0; i < m_NumberOfMatrices; ++i) - { - this->DestroyMatrix(i); - } - for (unsigned int i = 0; i < m_NumberOfVectors; ++i) - { - this->DestroyVector(i); - } - for (unsigned int i = 0; i < m_NumberOfSolutions; ++i) - { - this->DestroySolution(i); - } - - delete m_Matrices; - delete m_Vectors; - delete m_Solutions; -} - } // namespace itk::fem From f806e6b446a7d4b2e87ad136dd7e17e1b4b9b98d Mon Sep 17 00:00:00 2001 From: Hans Johnson Date: Mon, 16 Mar 2026 07:32:17 -0500 Subject: [PATCH 2/3] COMP: Replace raw new[] with std::vector in CellInterfaceTest - Replace PointIdentifier array allocation with std::vector - Use .data() method for passing to SetPointIds Co-Authored-By: Claude Opus 4.6 --- Modules/Core/Mesh/test/itkCellInterfaceTest.cxx | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/Modules/Core/Mesh/test/itkCellInterfaceTest.cxx b/Modules/Core/Mesh/test/itkCellInterfaceTest.cxx index 1dd3b555810..79d4bfb3ad6 100644 --- a/Modules/Core/Mesh/test/itkCellInterfaceTest.cxx +++ b/Modules/Core/Mesh/test/itkCellInterfaceTest.cxx @@ -17,7 +17,9 @@ *=========================================================================*/ #include +#include #include +#include #include "itkMesh.h" #include "itkPolyLineCell.h" @@ -88,15 +90,12 @@ TestCellInterface(const std::string_view name, TCell * aCell) std::cout << " SetPointIds" << std::endl; using PointIdentifier = typename TCell::PointIdentifier; + const unsigned int numberOfPointIds = cell->GetNumberOfPoints() * 2; - auto * pointIds = new PointIdentifier[cell->GetNumberOfPoints() * 2]; + std::vector pointIds(numberOfPointIds); + std::iota(pointIds.begin(), pointIds.end(), 0); - for (unsigned int i = 0; i < cell->GetNumberOfPoints() * 2; ++i) - { - pointIds[i] = i; - } - - cell->SetPointIds(pointIds); + cell->SetPointIds(pointIds.data()); if (cell->GetNumberOfPoints() > 0) { cell->SetPointId(0, 100); @@ -112,7 +111,7 @@ TestCellInterface(const std::string_view name, TCell * aCell) } std::cout << std::endl; - cell->SetPointIds(&pointIds[cell->GetNumberOfPoints()], &pointIds[cell->GetNumberOfPoints() * 2]); + cell->SetPointIds(pointIds.data() + cell->GetNumberOfPoints(), pointIds.data() + numberOfPointIds); std::cout << " Iterator test: PointIds for populated cell: "; typename TCell::PointIdIterator pxpointId = cell->PointIdsBegin(); typename TCell::PointIdIterator pxendId = cell->PointIdsEnd(); @@ -136,8 +135,6 @@ TestCellInterface(const std::string_view name, TCell * aCell) } std::cout << std::endl; - - delete[] pointIds; return EXIT_SUCCESS; } int From 6862133869d9d1bfe932c3f992d039df2f58959d Mon Sep 17 00:00:00 2001 From: Hans Johnson Date: Tue, 31 Mar 2026 09:50:52 -0500 Subject: [PATCH 3/3] COMP: Replace raw pointers with std::unique_ptr in VoxBoCUBImageIO Replace raw pointer return types and members with std::unique_ptr. Use std::make_unique for polymorphic factory creation. Destructor is defaulted since unique_ptr handles cleanup. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../Review/include/itkVoxBoCUBImageIO.h | 10 ++++--- .../Nonunit/Review/src/itkVoxBoCUBImageIO.cxx | 30 ++++++------------- 2 files changed, 15 insertions(+), 25 deletions(-) diff --git a/Modules/Nonunit/Review/include/itkVoxBoCUBImageIO.h b/Modules/Nonunit/Review/include/itkVoxBoCUBImageIO.h index 96134ae2f8a..6d94b8dd87c 100644 --- a/Modules/Nonunit/Review/include/itkVoxBoCUBImageIO.h +++ b/Modules/Nonunit/Review/include/itkVoxBoCUBImageIO.h @@ -22,6 +22,7 @@ #include #include #include +#include #include "itkImageIOBase.h" #include "itkSpatialOrientation.h" #include @@ -95,7 +96,7 @@ class VoxBoCUBImageIO : public ImageIOBase Write(const void * buffer) override; VoxBoCUBImageIO(); - ~VoxBoCUBImageIO() override; + ~VoxBoCUBImageIO() override = default; void PrintSelf(std::ostream & os, Indent indent) const override; @@ -103,13 +104,14 @@ class VoxBoCUBImageIO : public ImageIOBase bool CheckExtension(const char *, bool & isCompressed); - GenericCUBFileAdaptor * + std::unique_ptr CreateReader(const char * filename); - GenericCUBFileAdaptor * + std::unique_ptr CreateWriter(const char * filename); - GenericCUBFileAdaptor *m_Reader{ nullptr }, *m_Writer{ nullptr }; + std::unique_ptr m_Reader{}; + std::unique_ptr m_Writer{}; // Initialize the orientation map (from strings to ITK) void diff --git a/Modules/Nonunit/Review/src/itkVoxBoCUBImageIO.cxx b/Modules/Nonunit/Review/src/itkVoxBoCUBImageIO.cxx index 3f9e32dce66..8a1b24a907d 100644 --- a/Modules/Nonunit/Review/src/itkVoxBoCUBImageIO.cxx +++ b/Modules/Nonunit/Review/src/itkVoxBoCUBImageIO.cxx @@ -330,14 +330,7 @@ VoxBoCUBImageIO::VoxBoCUBImageIO() m_ByteOrder = IOByteOrderEnum::BigEndian; } -/** Destructor */ -VoxBoCUBImageIO::~VoxBoCUBImageIO() -{ - delete m_Reader; - delete m_Writer; -} - -GenericCUBFileAdaptor * +std::unique_ptr VoxBoCUBImageIO::CreateReader(const char * filename) { try @@ -347,11 +340,11 @@ VoxBoCUBImageIO::CreateReader(const char * filename) { if (compressed) { - return new CompressedCUBFileAdaptor(filename, "rb"); + return std::make_unique(filename, "rb"); } else { - return new DirectCUBFileAdaptor(filename, "rb"); + return std::make_unique(filename, "rb"); } } else @@ -365,7 +358,7 @@ VoxBoCUBImageIO::CreateReader(const char * filename) } } -GenericCUBFileAdaptor * +std::unique_ptr VoxBoCUBImageIO::CreateWriter(const char * filename) { try @@ -375,11 +368,11 @@ VoxBoCUBImageIO::CreateWriter(const char * filename) { if (compressed) { - return new CompressedCUBFileAdaptor(filename, "wb"); + return std::make_unique(filename, "wb"); } else { - return new DirectCUBFileAdaptor(filename, "wb"); + return std::make_unique(filename, "wb"); } } else @@ -397,7 +390,7 @@ bool VoxBoCUBImageIO::CanReadFile(const char * filename) { // First check if the file can be read - GenericCUBFileAdaptor * reader = CreateReader(filename); + auto reader = CreateReader(filename); if (reader == nullptr) { @@ -434,7 +427,6 @@ VoxBoCUBImageIO::CanReadFile(const char * filename) iscub = false; } - delete reader; return iscub; } @@ -468,10 +460,7 @@ VoxBoCUBImageIO::Read(void * buffer) void VoxBoCUBImageIO::ReadImageInformation() { - // Make sure there is no other reader - delete m_Reader; - - // Create a reader + // Create a reader (assignment releases any previous reader) m_Reader = CreateReader(m_FileName.c_str()); if (m_Reader == nullptr) { @@ -741,8 +730,7 @@ VoxBoCUBImageIO::Write(const void * buffer) m_Writer = CreateWriter(m_FileName.c_str()); WriteImageInformation(); m_Writer->WriteData(buffer, this->GetImageSizeInBytes()); - delete m_Writer; - m_Writer = nullptr; + m_Writer.reset(); } /** Print Self Method */