Skip to content

Commit

Permalink
BUG: Let MeshIOTestHelper AllocateBuffer return std::shared_ptr<void>
Browse files Browse the repository at this point in the history 
Let AllocateBuffer (from MeshIOTestHelper) return a `std::shared_ptr<void>`,
instead of producing a void pointer, to ensure that it is deleted correctly,
automatically.

Aims to fix Valgrind errors, saying:

> Mismatched free() / delete / delete []

As reported and discussed by Jon Haitz Legarreta Gorroño and Mihail Isakov at
pull request #3459
  • Loading branch information
@N-Dekker @dzenanz
N-Dekker authored and dzenanz committed Jun 7, 2022
1 parent 358f537 commit db8d797
Show file tree
Hide file tree
Showing 7 changed files with 132 additions and 173 deletions.
28 changes: 12 additions & 16 deletions Modules/IO/MeshBYU/test/itkBYUMeshIOTest.cxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,8 @@ itkBYUMeshIOTest(int argc, char * argv[])
testStatus = TestBaseClassMethodsMeshIO<itk::BYUMeshIO>(byuMeshIOBaseTest);

// Test reading exceptions
void * pointBuffer = nullptr;
ITK_TRY_EXPECT_EXCEPTION(byuMeshIO->ReadPoints(pointBuffer));

void * cellBuffer = nullptr;
ITK_TRY_EXPECT_EXCEPTION(byuMeshIO->ReadCells(cellBuffer));
ITK_TRY_EXPECT_EXCEPTION(byuMeshIO->ReadPoints(nullptr));
ITK_TRY_EXPECT_EXCEPTION(byuMeshIO->ReadCells(nullptr));

std::string inputFileName = argv[3];
ITK_TEST_EXPECT_TRUE(!byuMeshIO->CanReadFile(inputFileName.c_str()));
Expand Down Expand Up @@ -98,16 +95,18 @@ itkBYUMeshIOTest(int argc, char * argv[])
itk::SizeValueType pointBufferSize = 1000;
itk::SizeValueType cellBufferSize = 1000;

AllocateBuffer(&pointBuffer, byuMeshIO->GetPointComponentType(), pointBufferSize);
AllocateBuffer(&cellBuffer, byuMeshIO->GetCellComponentType(), cellBufferSize);
const std::shared_ptr<void> pointBuffer =
itk::MeshIOTestHelper::AllocateBuffer(byuMeshIO->GetPointComponentType(), pointBufferSize);
const std::shared_ptr<void> cellBuffer =
itk::MeshIOTestHelper::AllocateBuffer(byuMeshIO->GetCellComponentType(), cellBufferSize);

ITK_TRY_EXPECT_NO_EXCEPTION(byuMeshIO->ReadPoints(pointBuffer));
ITK_TRY_EXPECT_NO_EXCEPTION(byuMeshIO->ReadPoints(pointBuffer.get()));

void * pointDataBuffer = nullptr;
// Not used; empty method body; called for coverage purposes
byuMeshIO->ReadPointData(pointDataBuffer);

ITK_TRY_EXPECT_NO_EXCEPTION(byuMeshIO->ReadCells(cellBuffer));
ITK_TRY_EXPECT_NO_EXCEPTION(byuMeshIO->ReadCells(cellBuffer.get()));

void * cellDataBuffer = nullptr;
// Not used; empty method body; called for coverage purposes
Expand All @@ -116,8 +115,8 @@ itkBYUMeshIOTest(int argc, char * argv[])
// Test writing exceptions
std::string outputFileName = "";
byuMeshIO->SetFileName(outputFileName);
ITK_TRY_EXPECT_EXCEPTION(byuMeshIO->WritePoints(pointBuffer));
ITK_TRY_EXPECT_EXCEPTION(byuMeshIO->WriteCells(cellBuffer));
ITK_TRY_EXPECT_EXCEPTION(byuMeshIO->WritePoints(pointBuffer.get()));
ITK_TRY_EXPECT_EXCEPTION(byuMeshIO->WriteCells(cellBuffer.get()));
ITK_TRY_EXPECT_EXCEPTION(byuMeshIO->WriteMeshInformation());

outputFileName = argv[4];
Expand All @@ -128,12 +127,12 @@ itkBYUMeshIOTest(int argc, char * argv[])
byuMeshIO->SetFileName(outputFileName.c_str());

// Write the actual data
ITK_TRY_EXPECT_NO_EXCEPTION(byuMeshIO->WritePoints(pointBuffer));
ITK_TRY_EXPECT_NO_EXCEPTION(byuMeshIO->WritePoints(pointBuffer.get()));

// Not used; empty method body; called for coverage purposes
byuMeshIO->WritePointData(pointDataBuffer);

ITK_TRY_EXPECT_NO_EXCEPTION(byuMeshIO->WriteCells(cellBuffer));
ITK_TRY_EXPECT_NO_EXCEPTION(byuMeshIO->WriteCells(cellBuffer.get()));

// Not used; empty method body; called for coverage purposes
byuMeshIO->WriteCellData(cellDataBuffer);
Expand Down Expand Up @@ -174,9 +173,6 @@ itkBYUMeshIOTest(int argc, char * argv[])
readWriteByuMeshIO->GetNumberOfCellPixelComponents());


::operator delete(pointBuffer);
::operator delete(cellBuffer);

std::cout << "Test finished." << std::endl;
return testStatus;
}
60 changes: 30 additions & 30 deletions Modules/IO/MeshBase/include/itkMeshIOTestHelper.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -375,59 +375,59 @@ TestBaseClassMethodsMeshIO(typename TMeshIO::Pointer meshIO)
return EXIT_SUCCESS;
}

void
AllocateBuffer(void ** data, itk::IOComponentEnum componentType, itk::SizeValueType bufferSize)
namespace itk
{
namespace MeshIOTestHelper
{
void * buffer = nullptr;

template <typename T>
std::shared_ptr<void>
MakeSharedArray(const std::size_t bufferSize)
{
return std::shared_ptr<void>(new T[bufferSize], std::default_delete<T[]>());
}

inline std::shared_ptr<void>
AllocateBuffer(itk::IOComponentEnum componentType, itk::SizeValueType bufferSize)
{
switch (componentType)
{
case itk::IOComponentEnum::CHAR:
buffer = new char[bufferSize];
break;
return MakeSharedArray<char>(bufferSize);
case itk::IOComponentEnum::UCHAR:
buffer = new unsigned char[bufferSize];
break;
return MakeSharedArray<unsigned char>(bufferSize);
case itk::IOComponentEnum::USHORT:
buffer = new unsigned short[bufferSize];
break;
return MakeSharedArray<unsigned short>(bufferSize);
case itk::IOComponentEnum::SHORT:
buffer = new short[bufferSize];
break;
return MakeSharedArray<short>(bufferSize);
case itk::IOComponentEnum::UINT:
buffer = new unsigned int[bufferSize];
break;
return MakeSharedArray<unsigned int>(bufferSize);
case itk::IOComponentEnum::INT:
buffer = new unsigned int[bufferSize];
break;
return MakeSharedArray<int>(bufferSize);
case itk::IOComponentEnum::ULONG:
buffer = new unsigned long[bufferSize];
break;
return MakeSharedArray<unsigned long>(bufferSize);
case itk::IOComponentEnum::LONG:
buffer = new long[bufferSize];
break;
return MakeSharedArray<long>(bufferSize);
case itk::IOComponentEnum::LONGLONG:
buffer = new long long[bufferSize];
break;
return MakeSharedArray<long long>(bufferSize);
case itk::IOComponentEnum::ULONGLONG:
buffer = new unsigned long long[bufferSize];
break;
return MakeSharedArray<unsigned long long>(bufferSize);
case itk::IOComponentEnum::FLOAT:
buffer = new float[bufferSize];
break;
return MakeSharedArray<float>(bufferSize);
case itk::IOComponentEnum::DOUBLE:
buffer = new double[bufferSize];
break;
return MakeSharedArray<double>(bufferSize);
case itk::IOComponentEnum::LDOUBLE:
buffer = new long double[bufferSize];
break;
return MakeSharedArray<long double>(bufferSize);
case itk::IOComponentEnum::UNKNOWNCOMPONENTTYPE:
break;
default:
break;
}

*data = buffer;
return nullptr;
}

} // namespace MeshIOTestHelper
} // namespace itk

#endif
38 changes: 15 additions & 23 deletions Modules/IO/MeshFreeSurfer/test/itkFreeSurferMeshIOTest.cxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,33 +79,31 @@ itkFreeSurferMeshIOTestHelper(typename TMeshIO::Pointer fsMeshIO,

itk::SizeValueType cellBufferSize = 2000;

void * pointBuffer = nullptr;
AllocateBuffer(&pointBuffer, fsMeshIO->GetPointComponentType(), pointBufferSize);
const std::shared_ptr<void> pointBuffer =
itk::MeshIOTestHelper::AllocateBuffer(fsMeshIO->GetPointComponentType(), pointBufferSize);
const std::shared_ptr<void> pointDataBuffer =
itk::MeshIOTestHelper::AllocateBuffer(fsMeshIO->GetPointPixelComponentType(), pointDataBufferSize);
const std::shared_ptr<void> cellBuffer =
itk::MeshIOTestHelper::AllocateBuffer(fsMeshIO->GetCellComponentType(), cellBufferSize);

void * pointDataBuffer = nullptr;
AllocateBuffer(&pointDataBuffer, fsMeshIO->GetPointPixelComponentType(), pointDataBufferSize);
ITK_TRY_EXPECT_NO_EXCEPTION(fsMeshIO->ReadPoints(pointBuffer.get()));
ITK_TRY_EXPECT_NO_EXCEPTION(fsMeshIO->ReadPointData(pointDataBuffer.get()));

void * cellBuffer = nullptr;
AllocateBuffer(&cellBuffer, fsMeshIO->GetCellComponentType(), cellBufferSize);

ITK_TRY_EXPECT_NO_EXCEPTION(fsMeshIO->ReadPoints(pointBuffer));
ITK_TRY_EXPECT_NO_EXCEPTION(fsMeshIO->ReadPointData(pointDataBuffer));

ITK_TRY_EXPECT_NO_EXCEPTION(fsMeshIO->ReadCells(cellBuffer));
ITK_TRY_EXPECT_NO_EXCEPTION(fsMeshIO->ReadCells(cellBuffer.get()));

void * cellDataBuffer = nullptr;
// Not used; empty method body; called for coverage purposes
fsMeshIO->ReadCellData(cellDataBuffer);

// Test writing exceptions
fsMeshIO->SetFileName("");
ITK_TRY_EXPECT_EXCEPTION(fsMeshIO->WritePoints(pointBuffer));
ITK_TRY_EXPECT_EXCEPTION(fsMeshIO->WritePoints(pointBuffer.get()));
if (dynamic_cast<itk::FreeSurferBinaryMeshIO *>(fsMeshIO.GetPointer()))
{
ITK_TRY_EXPECT_EXCEPTION(fsMeshIO->WritePointData(pointDataBuffer));
ITK_TRY_EXPECT_EXCEPTION(fsMeshIO->WritePointData(pointDataBuffer.get()));
}

ITK_TRY_EXPECT_EXCEPTION(fsMeshIO->WriteCells(cellBuffer));
ITK_TRY_EXPECT_EXCEPTION(fsMeshIO->WriteCells(cellBuffer.get()));
ITK_TRY_EXPECT_EXCEPTION(fsMeshIO->WriteMeshInformation());

ITK_TEST_EXPECT_TRUE(!fsMeshIO->CanWriteFile(notAFsOutputFileName));
Expand All @@ -114,10 +112,10 @@ itkFreeSurferMeshIOTestHelper(typename TMeshIO::Pointer fsMeshIO,
fsMeshIO->SetFileName(outputFileName);

// Write the actual data
ITK_TRY_EXPECT_NO_EXCEPTION(fsMeshIO->WritePoints(pointBuffer));
ITK_TRY_EXPECT_NO_EXCEPTION(fsMeshIO->WritePointData(pointDataBuffer));
ITK_TRY_EXPECT_NO_EXCEPTION(fsMeshIO->WritePoints(pointBuffer.get()));
ITK_TRY_EXPECT_NO_EXCEPTION(fsMeshIO->WritePointData(pointDataBuffer.get()));

ITK_TRY_EXPECT_NO_EXCEPTION(fsMeshIO->WriteCells(cellBuffer));
ITK_TRY_EXPECT_NO_EXCEPTION(fsMeshIO->WriteCells(cellBuffer.get()));

// Not used; empty method body; called for coverage purposes
fsMeshIO->WriteCellData(cellDataBuffer);
Expand Down Expand Up @@ -148,12 +146,6 @@ itkFreeSurferMeshIOTestHelper(typename TMeshIO::Pointer fsMeshIO,
ITK_TEST_EXPECT_EQUAL(fsMeshIO->GetNumberOfCellPixelComponents(),
readWritefsMeshIO->GetNumberOfCellPixelComponents());


::operator delete(pointBuffer);
::operator delete(pointDataBuffer);
::operator delete(cellBuffer);
::operator delete(cellDataBuffer);

return testStatus;
}

Expand Down
48 changes: 20 additions & 28 deletions Modules/IO/MeshGifti/test/itkGiftiMeshIOTest.cxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,10 @@ itkGiftiMeshIOTest(int argc, char * argv[])
giftiMeshIO->SetFileName(inputFileName);
ITK_TRY_EXPECT_EXCEPTION(giftiMeshIO->ReadMeshInformation());

void * pointBuffer = nullptr;
void * pointDataBuffer = nullptr;
ITK_TRY_EXPECT_EXCEPTION(giftiMeshIO->ReadPoints(pointBuffer));
ITK_TRY_EXPECT_EXCEPTION(giftiMeshIO->ReadPointData(pointDataBuffer));

void * cellBuffer = nullptr;
void * cellDataBuffer = nullptr;
ITK_TRY_EXPECT_EXCEPTION(giftiMeshIO->ReadCells(cellBuffer));
ITK_TRY_EXPECT_EXCEPTION(giftiMeshIO->ReadCellData(cellDataBuffer));
ITK_TRY_EXPECT_EXCEPTION(giftiMeshIO->ReadPoints(nullptr));
ITK_TRY_EXPECT_EXCEPTION(giftiMeshIO->ReadPointData(nullptr));
ITK_TRY_EXPECT_EXCEPTION(giftiMeshIO->ReadCells(nullptr));
ITK_TRY_EXPECT_EXCEPTION(giftiMeshIO->ReadCellData(nullptr));

// Until the mesh information is read, the label color and name tables should be empty
ITK_TEST_SET_GET_NULL_VALUE(giftiMeshIO->GetLabelColorTable());
Expand Down Expand Up @@ -146,17 +141,20 @@ itkGiftiMeshIOTest(int argc, char * argv[])
itk::SizeValueType cellBufferSize = 1000000;
itk::SizeValueType cellDataBufferSize = 1000000;

AllocateBuffer(&pointBuffer, giftiMeshIO->GetPointComponentType(), pointBufferSize);
AllocateBuffer(&pointDataBuffer, giftiMeshIO->GetPointPixelComponentType(), pointDataBufferSize);

AllocateBuffer(&cellBuffer, giftiMeshIO->GetCellComponentType(), cellBufferSize);
AllocateBuffer(&cellDataBuffer, giftiMeshIO->GetCellPixelComponentType(), cellDataBufferSize);
const std::shared_ptr<void> pointBuffer =
itk::MeshIOTestHelper::AllocateBuffer(giftiMeshIO->GetPointComponentType(), pointBufferSize);
const std::shared_ptr<void> pointDataBuffer =
itk::MeshIOTestHelper::AllocateBuffer(giftiMeshIO->GetPointPixelComponentType(), pointDataBufferSize);
const std::shared_ptr<void> cellBuffer =
itk::MeshIOTestHelper::AllocateBuffer(giftiMeshIO->GetCellComponentType(), cellBufferSize);
const std::shared_ptr<void> cellDataBuffer =
itk::MeshIOTestHelper::AllocateBuffer(giftiMeshIO->GetCellPixelComponentType(), cellDataBufferSize);

ITK_TRY_EXPECT_NO_EXCEPTION(giftiMeshIO->ReadPoints(pointBuffer));
ITK_TRY_EXPECT_NO_EXCEPTION(giftiMeshIO->ReadPointData(pointDataBuffer));
ITK_TRY_EXPECT_NO_EXCEPTION(giftiMeshIO->ReadPoints(pointBuffer.get()));
ITK_TRY_EXPECT_NO_EXCEPTION(giftiMeshIO->ReadPointData(pointDataBuffer.get()));

ITK_TRY_EXPECT_NO_EXCEPTION(giftiMeshIO->ReadCells(cellBuffer));
ITK_TRY_EXPECT_NO_EXCEPTION(giftiMeshIO->ReadCellData(cellDataBuffer));
ITK_TRY_EXPECT_NO_EXCEPTION(giftiMeshIO->ReadCells(cellBuffer.get()));
ITK_TRY_EXPECT_NO_EXCEPTION(giftiMeshIO->ReadCellData(cellDataBuffer.get()));

auto writeUpdatePointData = static_cast<bool>(std::stoi(argv[10]));
giftiMeshIO->SetUpdatePointData(writeUpdatePointData);
Expand Down Expand Up @@ -192,11 +190,11 @@ itkGiftiMeshIOTest(int argc, char * argv[])
ITK_TEST_EXPECT_TRUE(giftiMeshIO->CanWriteFile(outputFileName.c_str()));

// Write the actual data
// ITK_TRY_EXPECT_NO_EXCEPTION(giftiMeshIO->WritePoints(pointBuffer));
// ITK_TRY_EXPECT_NO_EXCEPTION(giftiMeshIO->WritePointData(pointDataBuffer));
// ITK_TRY_EXPECT_NO_EXCEPTION(giftiMeshIO->WritePoints(pointBuffer.get()));
// ITK_TRY_EXPECT_NO_EXCEPTION(giftiMeshIO->WritePointData(pointDataBuffer.get()));

// ITK_TRY_EXPECT_NO_EXCEPTION(giftiMeshIO->WriteCells(cellBuffer));
// ITK_TRY_EXPECT_NO_EXCEPTION(giftiMeshIO->WriteCellData(cellDataBuffer));
// ITK_TRY_EXPECT_NO_EXCEPTION(giftiMeshIO->WriteCells(cellBuffer.get()));
// ITK_TRY_EXPECT_NO_EXCEPTION(giftiMeshIO->WriteCellData(cellDataBuffer.get()));

ITK_TRY_EXPECT_NO_EXCEPTION(giftiMeshIO->WriteMeshInformation());

Expand Down Expand Up @@ -246,12 +244,6 @@ itkGiftiMeshIOTest(int argc, char * argv[])
ITK_TEST_EXPECT_EQUAL(giftiMeshIO->GetNumberOfCellPixelComponents(),
readWriteGiftiMeshIO->GetNumberOfCellPixelComponents());


::operator delete(pointBuffer);
::operator delete(pointDataBuffer);
::operator delete(cellBuffer);
::operator delete(cellDataBuffer);

std::cout << "Test finished." << std::endl;
return testStatus;
}
42 changes: 17 additions & 25 deletions Modules/IO/MeshOBJ/test/itkOBJMeshIOTest.cxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,8 @@ itkOBJMeshIOTest(int argc, char * argv[])
objMeshIO->SetFileName(inputFileName);
ITK_TRY_EXPECT_EXCEPTION(objMeshIO->ReadMeshInformation());

void * pointBuffer = nullptr;
ITK_TRY_EXPECT_EXCEPTION(objMeshIO->ReadPoints(pointBuffer));

void * cellBuffer = nullptr;
ITK_TRY_EXPECT_EXCEPTION(objMeshIO->ReadCells(cellBuffer));
ITK_TRY_EXPECT_EXCEPTION(objMeshIO->ReadPoints(nullptr));
ITK_TRY_EXPECT_EXCEPTION(objMeshIO->ReadCells(nullptr));

inputFileName = argv[3];
ITK_TEST_EXPECT_TRUE(!objMeshIO->CanReadFile(inputFileName.c_str()));
Expand Down Expand Up @@ -102,18 +99,18 @@ itkOBJMeshIOTest(int argc, char * argv[])
itk::SizeValueType pointBufferSize = 100000;
itk::SizeValueType pointDataBufferSize = 100000;

itk::SizeValueType cellBufferSize = 100000;
AllocateBuffer(&pointBuffer, objMeshIO->GetPointComponentType(), pointBufferSize);

void * pointDataBuffer = nullptr;
AllocateBuffer(&pointDataBuffer, objMeshIO->GetPointPixelComponentType(), pointDataBufferSize);

AllocateBuffer(&cellBuffer, objMeshIO->GetCellComponentType(), cellBufferSize);
itk::SizeValueType cellBufferSize = 100000;
const std::shared_ptr<void> pointBuffer =
itk::MeshIOTestHelper::AllocateBuffer(objMeshIO->GetPointComponentType(), pointBufferSize);
const std::shared_ptr<void> pointDataBuffer =
itk::MeshIOTestHelper::AllocateBuffer(objMeshIO->GetPointPixelComponentType(), pointDataBufferSize);
const std::shared_ptr<void> cellBuffer =
itk::MeshIOTestHelper::AllocateBuffer(objMeshIO->GetCellComponentType(), cellBufferSize);

ITK_TRY_EXPECT_NO_EXCEPTION(objMeshIO->ReadPoints(pointBuffer));
ITK_TRY_EXPECT_NO_EXCEPTION(objMeshIO->ReadPointData(pointDataBuffer));
ITK_TRY_EXPECT_NO_EXCEPTION(objMeshIO->ReadPoints(pointBuffer.get()));
ITK_TRY_EXPECT_NO_EXCEPTION(objMeshIO->ReadPointData(pointDataBuffer.get()));

ITK_TRY_EXPECT_NO_EXCEPTION(objMeshIO->ReadCells(cellBuffer));
ITK_TRY_EXPECT_NO_EXCEPTION(objMeshIO->ReadCells(cellBuffer.get()));

void * cellDataBuffer = nullptr;
// Not used; empty method body; called for coverage purposes
Expand All @@ -122,8 +119,8 @@ itkOBJMeshIOTest(int argc, char * argv[])
// Test writing exceptions
std::string outputFileName = "";
objMeshIO->SetFileName(outputFileName);
ITK_TRY_EXPECT_EXCEPTION(objMeshIO->WritePoints(pointBuffer));
ITK_TRY_EXPECT_EXCEPTION(objMeshIO->WriteCells(cellBuffer));
ITK_TRY_EXPECT_EXCEPTION(objMeshIO->WritePoints(pointBuffer.get()));
ITK_TRY_EXPECT_EXCEPTION(objMeshIO->WriteCells(cellBuffer.get()));
ITK_TRY_EXPECT_EXCEPTION(objMeshIO->WriteMeshInformation());

outputFileName = argv[4];
Expand All @@ -134,12 +131,12 @@ itkOBJMeshIOTest(int argc, char * argv[])
objMeshIO->SetFileName(outputFileName);

// Write the actual data
ITK_TRY_EXPECT_NO_EXCEPTION(objMeshIO->WritePoints(pointBuffer));
ITK_TRY_EXPECT_NO_EXCEPTION(objMeshIO->WritePoints(pointBuffer.get()));

// Not used; empty method body; called for coverage purposes
objMeshIO->WritePointData(pointDataBuffer);
objMeshIO->WritePointData(pointDataBuffer.get());

ITK_TRY_EXPECT_NO_EXCEPTION(objMeshIO->WriteCells(cellBuffer));
ITK_TRY_EXPECT_NO_EXCEPTION(objMeshIO->WriteCells(cellBuffer.get()));

// Not used; empty method body; called for coverage purposes
objMeshIO->WriteCellData(cellDataBuffer);
Expand Down Expand Up @@ -179,11 +176,6 @@ itkOBJMeshIOTest(int argc, char * argv[])
ITK_TEST_EXPECT_EQUAL(objMeshIO->GetNumberOfCellPixelComponents(),
readWriteByuMeshIO->GetNumberOfCellPixelComponents());


::operator delete(pointBuffer);
::operator delete(pointDataBuffer);
::operator delete(cellBuffer);

std::cout << "Test finished." << std::endl;
return testStatus;
}
Loading

0 comments on commit db8d797

Please sign in to comment.