[vtk-developers] Warning in vtkExtractSelectedIds

[David Gobbi]

On Fri, Dec 30, 2011 at 2:04 PM, David Doria <daviddoria at gmail.com> wrote:
> On Fri, Dec 30, 2011 at 3:44 PM, David Gobbi <david.gobbi at gmail.com> wrote:
>> The more that I look at it, the more convinced I am that the outer loop
>> is not even supposed to be there.
>>
>>  860         for (vtkIdType j = 0; j < input->GetNumberOfPoints(); j++)
>>
>> http://vtk.org/gitweb?p=VTK.git;a=blob;f=Graphics/vtkExtractSelectedIds.cxx;hb=ecbbf7f7#l860
>>
>> Take a look at the commit where this line was added:
>> http://vtk.org/gitweb?p=VTK.git;a=commitdiff;h=7030e24d#patch5
>>
>> You have to scroll down some to see the line, but when this line was
>> first added it was missing curly braces and it really looks as if the
>> whole line is just a big typo.
>>
>>  - David
>
> Once the patch is ready, we should probably also add a test:
>
> ~/src/VTK/Graphics/Testing/Cxx$ grep -ri "ExtractSelectedIds" .
> ~/src/VTK/Graphics/Testing/Cxx$
>
> I just wrote this demo:
> http://www.vtk.org/Wiki/VTK/Examples/Cxx/PolyData/ExtractSelectedIds
>
> It could be used as a starting point for a simple test.

According to the dashboard, the class already has good coverage,
because it is used by vtkExtractSelection.  Of course more tests
are always good.

This is one of those odd bugs that testing might never find.  The
block of code that it was in had full test coverage. It seems that its
only side-effect was to eat up a few extra CPU cycles, though it had
the potential of causing an infinite loop.

That's why, even with automated testing, it is always a good idea
to have real people look over the code from time to time.

 - David