[vtk-developers] announce: vtk 6.2.0 release candidate 1 is ready

[Sean McBride]

So I untangled this...

In the Insert...() methods of the various vtk*Array classes there are calls to ResizeAndExtend() within which there are code paths like:

  if (newSize <= 0)
    {
    this->Initialize();
    return 0;
    }
  if ( (newArray = new unsigned char[(newSize+7)/8]) == NULL )
    {
    vtkErrorMacro(<< "Cannot allocate memory\n");
    return 0;
    }

So 'return 0' looked (looks!) like an error-only path to me, and I added a vtkErrorMacro to the first 'if'.  (That's now reverted.)  But 'newSize' of 0 is not an error path, it's an acceptable size to resize to.

Q1) What's the return value of ResizeAndExtend() mean?  It's not well documented.
Q2) Should resize to 0 be returning null?

Initialize() sets the 'Array' ivar to null.  But the callers of ResizeAndExtend() don't check the return value and then have code paths that dereference 'Array'.  Hence the analyzer warning.  Alas, the Insert...() methods don't have any return value to notify _their_ callers of failure.  So there's no beautiful fix that I see.

This is most reasonable thing I could come up with:
<http://review.source.kitware.com/19406>

Sean



On Wed, 25 Feb 2015 10:42:40 -0700, David Gobbi said:

>It looks like the added vtkErrorMacro() will be called whenever Squeeze()
>is called on an empty array, which is a common occurrence and should
>definitely not generate an error.
>
>I think the patch should be reverted (though I was the reviewer... oops!).
>
> - David
>
>
>On Wed, Feb 25, 2015 at 9:59 AM, David E DeMarle <dave.demarle at kitware.com>
>wrote:
>
>> Keep us informed. If it needs fixing we're still making fixes and taking
>> patches based off anywhere upstream of the release branch point and tested
>> in master.
>>
>> We'll make a decision on turning the release branch into either rc2 or
>> final on Monday some time.
>>
>> On Wed, Feb 25, 2015 at 11:52 AM, Sean McBride <sean at rogue-research.com>
>> wrote:
>>>
>>>
>>> No serious issues found.
>>>
>>> Maybe found a mistake I made though:
>>>
>>> <http://review.source.kitware.com/#/c/18331/1//COMMIT_MSG>
>>>
>>> I think some of the vtkErrorMacros I added should maybe not be there, as
>>> I see them running my app where I don't believe I saw them before.  Then
>>> again, they are maybe flagging a real issue that was not flagged before.
>>> Haven't had time to dig...