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

[N-Dekker]

Originally, a zero-length VariableLengthVector might still have its m_Data pointing to the heap, as it might have its memory allocated by new TValue[length], with length == 0.

A memory leak in VariableLengthVector(unsigned int length) was then introduced by pull request #5710 commit bb79801
"STYLE: Replace AllocateElements calls within VariableLengthVector" forgetting to delete memory that was allocated by new TValue[0]. As was reported by Mihail Isakov (@issakomi) at #5710 (comment), referring to the Valgrind results at https://open.cdash.org/builds/10917349/dynamic_analysis

With this commit, m_Data should always be null when the VariableLengthVector is empty and manages its own memory (m_LetArrayManageMemory is true).

[N-Dekker]

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. 🤷

[N-Dekker]

macOS-Build20656084594 test itkFFTConvolutionImageFilterStreamingTest fails, saying:

Wrote output image to /Users/runner/work/ITK/ITK/build/Testing/Temporary/ITKConvolution/itkFFTConvolutionImageFilterStreamingTestOutput.mha
Exception detected while reading /Users/runner/work/ITK/ITK/build/ExternalData/Modules/Filtering/Convolution/test/Baseline/itkConvolutionImageFilterStreamingTestOutput.mha :  Could not create IO object for reading file /Users/runner/work/ITK/ITK/build/Testing/Temporary/ITKConvolution/itkFFTConvolutionImageFilterStreamingTestOutput.mha
  ...
  You probably failed to set a file suffix, or set the suffix to an unsupported type.

I guess it's unrelated to this pull request 🤷

[blowekamp]

It looks like "itkFFTConvolutionImageFilterStreamingTestOutput.mha" is used for multiple outputs in the tests in the CMake file.

[blowekamp]

Addressing the duplicate output file race condition in #5714

[dzenanz]

LGTM