Skip to content

BUG: Make VariableLengthVector::m_Data null when its length is zero #5713

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.

Sign up for GitHub

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

Merged
dzenanz merged 1 commit into from
Jan 5, 2026
Merged

BUG: Make VariableLengthVector::m_Data null when its length is zero #5713

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 6 additions & 3 deletions Modules/Core/Common/include/itkVariableLengthVector.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,10 +270,13 @@ class ITK_TEMPLATE_EXPORT VariableLengthVector
void
operator()(unsigned int newSize, unsigned int oldSize, TValue2 * oldBuffer, TValue2 * newBuffer) const
{
itkAssertInDebugAndIgnoreInReleaseMacro(newBuffer);
const size_t nb = std::min(newSize, oldSize);
itkAssertInDebugAndIgnoreInReleaseMacro(nb == 0 || (nb > 0 && oldBuffer != nullptr));
std::copy_n(oldBuffer, nb, newBuffer);
if (nb > 0)
{
itkAssertInDebugAndIgnoreInReleaseMacro(newBuffer);
itkAssertInDebugAndIgnoreInReleaseMacro(oldBuffer);
std::copy_n(oldBuffer, nb, newBuffer);
}
}
};

Expand Down
44 changes: 22 additions & 22 deletions Modules/Core/Common/include/itkVariableLengthVector.hxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,15 @@ namespace itk

template <typename TValue>
VariableLengthVector<TValue>::VariableLengthVector(unsigned int length)
: m_Data(new TValue[length])
: m_Data(length == 0 ? nullptr : new TValue[length])
, m_NumElements(length)
{
// postcondition(s)
itkAssertInDebugAndIgnoreInReleaseMacro(m_Data != nullptr);
}
{}

template <typename TValue>
VariableLengthVector<TValue>::VariableLengthVector(unsigned int length, const TValue & value)
: VariableLengthVector(length)
{
std::fill_n(m_Data, length, value);
Fill(value);
}

template <typename TValue>
Expand All @@ -67,10 +64,6 @@ VariableLengthVector<TValue>::VariableLengthVector(const VariableLengthVector<TV
itkAssertInDebugAndIgnoreInReleaseMacro(v.m_Data != nullptr);
std::copy_n(&v.m_Data[0], m_NumElements, &this->m_Data[0]);
}
else
{
m_Data = nullptr;
}
}

template <typename TValue>
Expand Down Expand Up @@ -133,8 +126,6 @@ VariableLengthVector<TValue>::VariableLengthVector(
rhs)
: VariableLengthVector(rhs.Size())
{
// VariableLengthVector(length) post-condition
itkAssertInDebugAndIgnoreInReleaseMacro(m_Data != nullptr);
for (ElementIdentifier i = 0; i < m_NumElements; ++i)
{
this->m_Data[i] = static_cast<TValue>(rhs[i]);
Expand Down Expand Up @@ -191,11 +182,14 @@ VariableLengthVector<TValue>::Reserve(ElementIdentifier size)
}
else
{
m_Data = new TValue[size];
// At this point, m_Data is null.
if (size > 0)
{
m_Data = new TValue[size];
}
m_NumElements = size;
m_LetArrayManageMemory = true;
}
itkAssertInDebugAndIgnoreInReleaseMacro(m_Data != nullptr);
}

#ifndef ITK_FUTURE_LEGACY_REMOVE
Expand Down Expand Up @@ -274,10 +268,15 @@ VariableLengthVector<TValue>::SetSize(unsigned int sz, TReallocatePolicy realloc

if (reallocatePolicy(sz, m_NumElements) || !m_LetArrayManageMemory)
{
auto temp = make_unique_for_overwrite<TValue[]>(sz); // may throw
itkAssertInDebugAndIgnoreInReleaseMacro(temp);
itkAssertInDebugAndIgnoreInReleaseMacro(m_NumElements == 0 || (m_NumElements > 0 && m_Data != nullptr));
keepValues(sz, m_NumElements, m_Data, temp.get());
std::unique_ptr<TValue[]> temp;

if (sz > 0)
{
temp = make_unique_for_overwrite<TValue[]>(sz); // may throw
itkAssertInDebugAndIgnoreInReleaseMacro(temp);
itkAssertInDebugAndIgnoreInReleaseMacro(m_NumElements == 0 || (m_NumElements > 0 && m_Data != nullptr));
keepValues(sz, m_NumElements, m_Data, temp.get());
}
// commit changes
if (m_LetArrayManageMemory)
{
Expand All @@ -293,10 +292,11 @@ template <typename TValue>
void
VariableLengthVector<TValue>::Fill(const TValue & v)
{
itkAssertInDebugAndIgnoreInReleaseMacro(m_NumElements == 0 || (m_NumElements > 0 && m_Data != nullptr));
// VC++ version of std::fill_n() expects the output iterator to be valid
// instead of expecting the range [OutIt, OutIt+n) to be valid.
Comment on lines -297 to -298
Copy link
Contributor Author

@N-Dekker N-Dekker Jan 2, 2026

Choose a reason for hiding this comment

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

Removed this comment, as it does not apply to currently supported versions of VC++ anymore, as far as I can see.

David Cole (@dlrdave) mentioned in https://review.source.kitware.com/%23/c/20307/, Oct 24, 2015:

You cannot call std::fill_n with a nullptr for Debug VS 2013.

The implementation of fill_n looks like this:

template<class _OutIt,
  class _Diff,
  class _Ty> inline
  _OutIt fill_n(_OutIt _Dest, _Diff _Count, const _Ty& _Val)
  {  // copy _Val _Count times through [_Dest, ...)
  _DEBUG_POINTER(_Dest);
  return (_Fill_n(_Dest, _Count, _Val,
    _Is_checked(_Dest)));
  }

The _DEBUG_POINTER line is called unconditionally, and will throw this assert if the pointer is nullptr.

However, this appears obsolete now, as VS 2019 and VS2022 no longer have those _DEBUG_POINTER macro calls. I don't see them in https://github.com/microsoft/STL/ either.

Honestly I don't know if the C++ standard now officially allows passing a null pointer as iterator to fill_n, or any other STL algorithm. 🤷

std::fill(&this->m_Data[0], &this->m_Data[m_NumElements], v);
if (m_NumElements > 0)
{
itkAssertInDebugAndIgnoreInReleaseMacro(m_Data);
std::fill_n(m_Data, m_NumElements, v);
}
}

template <typename TValue>
Expand Down
Loading