[vtk-developers] Warning in vtkExtractSelectedIds

[David Gobbi]

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


On Fri, Dec 30, 2011 at 11:43 AM, Utkarsh Ayachit
<utkarsh.ayachit at kitware.com> wrote:
> I can take a look at it. I'm sure I know what it should be doing, even
> if not exactly how it's doing it.
>
> Utkarsh
>
> On Fri, Dec 30, 2011 at 1:24 PM, David Gobbi <david.gobbi at gmail.com> wrote:
>> The tricky thing is that the re-used index is not the only problem.
>> The outer loop is looping over all the points:
>>
>>  872         for (vtkIdType j = 0; j < input->GetNumberOfPoints(); j++)
>>  873           {
>>  874           input->GetPointCells(ptId, ptCells);
>>
>> So it seems that line 874 should use "j" instead of "ptId":
>>
>>  874           input->GetPointCells(j, ptCells);
>>
>> Unfortunately, I don't understand the code well enough...
>> any suggestions?
>>
>> I'm sure that multiple people have worked on this little block of code,
>> because some loops use "j++" and some use "++j"  :)
>>
>>  - David
>>
>>
>> On Fri, Dec 30, 2011 at 11:12 AM, Bill Lorensen <bill.lorensen at gmail.com> wrote:
>>> Bad, bad, really bad. Nice catch.
>>>
>>> On Fri, Dec 30, 2011 at 1:04 PM, David Gobbi <david.gobbi at gmail.com> wrote:
>>>> Hi Utkarsh,
>>>>
>>>> I think that I might have found a bug in that class.  At line 880,
>>>> the "j" is used as a for-loop index even though it was already
>>>> used in the enclosing for-loop:
>>>>
>>>> http://vtk.org/gitweb?p=VTK.git;a=blob;f=Graphics/vtkExtractSelectedIds.cxx;h=0d45ffc7#l870
>>>>
>>>> Can you take a look?
>>>>
>>>>  - David
>>>>
>>>>
>>>> On Fri, Dec 30, 2011 at 10:23 AM, Utkarsh Ayachit
>>>> <utkarsh.ayachit at kitware.com> wrote:
>>>>> That sounds great to me. Thanks.
>>>>>
>>>>> Utkarsh
>>>>>
>>>>> On Fri, Dec 30, 2011 at 11:13 AM, David Gobbi <david.gobbi at gmail.com> wrote:
>>>>>> On Fri, Dec 30, 2011 at 9:02 AM, David Doria <daviddoria at gmail.com> wrote:
>>>>>>> On Fri, Dec 30, 2011 at 10:26 AM, David Gobbi <david.gobbi at gmail.com> wrote:
>>>>>>>> The only way to avoid shadow warnings is to use a new name
>>>>>>>
>>>>>>> The name VTK_TT is used inside the macro, so I don't think there is an
>>>>>>> easy way to change it in the second call. Anyone have a "quick fix" we
>>>>>>> can apply now to fix the warning, and then we can put the proposed
>>>>>>> rewrite on the todo list?
>>>>>>
>>>>>> My proposed rewrite is fairly straightforward.  Since it was on my mind,
>>>>>> I've already gotten started on it, and will submit it to gerrit soon.
>>>>>>
>>>>>>  - David
>>>>>> _______________________________________________
>>>>>> Powered by www.kitware.com
>>>>>>
>>>>>> Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html
>>>>>>
>>>>>> Follow this link to subscribe/unsubscribe:
>>>>>> http://www.vtk.org/mailman/listinfo/vtk-developers
>>>>>>
>>>> _______________________________________________
>>>> Powered by www.kitware.com
>>>>
>>>> Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html
>>>>
>>>> Follow this link to subscribe/unsubscribe:
>>>> http://www.vtk.org/mailman/listinfo/vtk-developers
>>>>
>>>
>>>
>>>
>>> --
>>> Unpaid intern in BillsBasement at noware dot com