Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Deref(T *), to ease dereferencing a pointer safely #4278

Merged
merged 2 commits into from
Nov 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/itk_dict.txt
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ Cz
DDataFrameobject
Dz
Dasarathy
Deref
Dij
Dinstein
DownsamplerType
Expand Down
59 changes: 59 additions & 0 deletions Modules/Core/Common/include/itkDeref.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/*=========================================================================
*
* Copyright NumFOCUS
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0.txt
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*=========================================================================*/
#ifndef itkDeref_h
#define itkDeref_h

#include "itkMacro.h"
#include <string>
#include <typeinfo>

namespace itk
{

/** \class DerefError
* Exception thrown when trying to dereference a null pointer.
* \ingroup ITKCommon
*/
class DerefError : public ExceptionObject
{
public:
// Inherit the constructors from its base class.
using ExceptionObject::ExceptionObject;

/** Runtime information support. */
itkTypeMacro(DerefError, ExceptionObject);
};


/** Dereferences the specified pointer, when the pointer is not null. Throws a `DerefError` exception when the pointer
* is null. Aims to avoid undefined behavior from accidentally dereferencing a null pointer.
*/
template <typename T>
T &
Deref(T * const ptr)
{
if (ptr == nullptr)
{
itkSpecializedMessageExceptionMacro(
DerefError, "The pointer passed to `itk::Deref(T*)` is null! T's typeid name: `" << typeid(T).name() << '`');
}
return *ptr;
}
} // namespace itk

#endif
1 change: 1 addition & 0 deletions Modules/Core/Common/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1706,6 +1706,7 @@ set(ITKCommonGTests
itkBuildInformationGTest.cxx
itkConnectedImageNeighborhoodShapeGTest.cxx
itkConstantBoundaryImageNeighborhoodPixelAccessPolicyGTest.cxx
itkDerefGTest.cxx
itkExceptionObjectGTest.cxx
itkFixedArrayGTest.cxx
itkImageNeighborhoodOffsetsGTest.cxx
Expand Down
48 changes: 48 additions & 0 deletions Modules/Core/Common/test/itkDerefGTest.cxx
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*=========================================================================
*
* Copyright NumFOCUS
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0.txt
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*=========================================================================*/

// First include the header file to be tested:
#include "itkDeref.h"
#include <gtest/gtest.h>
#include "itkObject.h"


// Tests that `Deref(ptr)` throws a `DerefError` exception when its argument is null.
TEST(Deref, ThrowsExceptionWhenArgumentIsNull)
{
const auto check = [](const auto & ptr) { EXPECT_THROW(itk::Deref(ptr), itk::DerefError); };

check(static_cast<int *>(nullptr));
check(static_cast<const int *>(nullptr));
check(static_cast<itk::Object *>(nullptr));
}


// Tests that `Deref(ptr)` returns the same reference as `*ptr`, when `ptr` is not null.
TEST(Deref, ReturnsReferenceWhenArgumentIsNotNull)
{
const auto check = [](const auto & ptr) {
const auto & ref = itk::Deref(ptr);
EXPECT_EQ(&ref, &*ptr);
};

constexpr int i{};
check(&i);
check(std::make_unique<int>().get());
check(itk::Object::New().get());
}
16 changes: 7 additions & 9 deletions Modules/IO/MeshVTK/test/itkVTKPolyDataMeshIOGTest.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
// First include the header file to be tested:
#include "itkMeshFileReader.h"

#include "itkDeref.h"
#include "itkMesh.h"
#include "itkMeshFileWriter.h"
#include "itkVTKPolyDataMeshIO.h"
Expand Down Expand Up @@ -53,9 +54,8 @@ Expect_lossless_writing_and_reading_of_points(const std::string &
{
const auto inputMesh = TMesh::New();

const auto inputPoints = inputMesh->GetPoints();
ASSERT_NE(inputPoints, nullptr);
inputPoints->assign(points.cbegin(), points.cend());
auto & inputPoints = itk::Deref(inputMesh->GetPoints());
inputPoints.assign(points.cbegin(), points.cend());

const auto writer = itk::MeshFileWriter<TMesh>::New();
if (writeAsBinary)
Expand All @@ -76,18 +76,16 @@ Expect_lossless_writing_and_reading_of_points(const std::string &
reader->SetMeshIO(itk::VTKPolyDataMeshIO::New());
reader->Update();

const auto outputMesh = reader->GetOutput();
ASSERT_NE(outputMesh, nullptr);
const auto outputPoints = outputMesh->GetPoints();
ASSERT_NE(outputPoints, nullptr);
const auto & outputMesh = itk::Deref(reader->GetOutput());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good demonstration of how this is supposed to be used. I would like to see some more instances of use, better demonstrating a need for a new function and exception class.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @dzenanz

I use elastix::Deref extensively within elastix, e.g. (source):

elastix::Deref(MersenneTwisterRandomVariateGenerator::GetInstance()).SetSeed(...);

and (source)

MultiThreaderBase & multiThreader = elastix::Deref(this->ProcessObject::GetMultiThreader());
multiThreader.SetSingleMethod(...);
multiThreader.SingleMethodExecute();

Once itk::Deref is available, I would like to replace those elastix::Deref calls with itk::Deref. I feel that this function isn't really elastix specific. It could be useful for other ITK users as well. Does that answer your question?

const auto & outputPoints = itk::Deref(outputMesh.GetPoints());

const size_t expectedNumberOfPoints = points.size();

EXPECT_EQ(outputPoints->size(), expectedNumberOfPoints);
EXPECT_EQ(outputPoints.size(), expectedNumberOfPoints);

auto pointIterator = points.cbegin();

for (const auto & outputPoint : *outputPoints)
for (const auto & outputPoint : outputPoints)
{
const auto & expectedPoint = *pointIterator;

Expand Down