I think the best approach is to perform more error checking in the reader. If you catch an invalid file while it is being loaded, it is easier to report what the problem is to the user. If you catch it later in the pipeline, it is much harder to do anything about it. For example, the fix that David suggested, is going to cause GetTuple() to keep values from a previous call (or worse junk from uninitialized memory) UNLESS the developer wipes the contents of tuple between every call. This is much harder to debug than a simple error at read time or even the current behavior of crashing. Of course, you could add a return value or exception but it is still not much help to the end user.<br>
<br>-berk<br><br><div class="gmail_quote">On Sat, Feb 13, 2010 at 6:42 PM, pat marion <span dir="ltr"><<a href="mailto:pat.marion@kitware.com">pat.marion@kitware.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Methods like GetTuple should be fast, and therefore not perform bounds checking for each call. For methods requiring speed, usually it's the caller's responsibility to do the bounds checking. Maybe the proper fix would be somewhere higher in the stack like <br>
<div>vtkPolygon::ComputeNormal or PVGeometryFilter::ExecuteCellNormals?</div><br>Pat<br><br><div class="gmail_quote"><div><div></div><div class="h5">On Sat, Feb 13, 2010 at 6:18 PM, David Doria <span dir="ltr"><<a href="mailto:daviddoria%2Bvtk@gmail.com" target="_blank">daviddoria+vtk@gmail.com</a>></span> wrote:<br>
</div></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div><div></div><div class="h5"><div>I noticed this problem a while back:</div>
<a href="http://public.kitware.com/Bug/view.php?id=9814" target="_blank">http://public.kitware.com/Bug/view.php?id=9814</a><div>
<br></div><div>The demo file I added to the bug contains a triangle cell but 0 points. Clearly this is a bad situation. It doesn't crash the XMLPolyDataReader, but if you load the file in Paraview, the call stack shows:</div>
<div><br></div><div>vtkDataArrayTemplate<float>::GetTuple (line 613)</div><div>vtkPoints::GetPoint</div><div>vtkPolygon::ComputeNormal</div><div>PVGeometryFilter::ExecuteCellNormals</div><div><br></div><div>The problem occurs on line 610 of vtkDataArrayTemplate.txx - 't' is NULL, so then on line 613 it segfaults.</div>
<div><br></div><div>Is this type of thing where we would want to allow a crash (the assumption being that only good data files are loaded)? Or could we change:</div><div><br></div><div><div>template <class T></div>
<div>
void vtkDataArrayTemplate<T>::GetTuple(vtkIdType i, double* tuple)</div><div>{</div><div> T* t = this->Array + this->NumberOfComponents*i;</div><div> for(int j=0; j < this->NumberOfComponents; ++j)</div>
<div> {</div><div> tuple[j] = static_cast<double>(t[j]);</div><div> }</div><div>}</div><div><br></div><div>to something like:</div><div><br></div><div><div>template <class T></div><div>void vtkDataArrayTemplate<T>::GetTuple(vtkIdType i, double* tuple)</div>
<div>{</div><div> T* t = this->Array + this->NumberOfComponents*i;</div><div><br></div><div> if(!t)</div><div> {</div><div> return;</div><div> }</div><div><br></div><div> for(int j=0; j < this->NumberOfComponents; ++j)</div>
<div> {</div><div> tuple[j] = static_cast<double>(t[j]);</div><div> }</div><div>}</div></div></div><div><br clear="all">Thanks,<br><br>David<br>
</div>
<br></div></div>_______________________________________________<br>
Powered by <a href="http://www.kitware.com" target="_blank">www.kitware.com</a><br>
<br>
Visit other Kitware open-source projects at <a href="http://www.kitware.com/opensource/opensource.html" target="_blank">http://www.kitware.com/opensource/opensource.html</a><br>
<br>
Follow this link to subscribe/unsubscribe:<br>
<a href="http://www.vtk.org/mailman/listinfo/vtk-developers" target="_blank">http://www.vtk.org/mailman/listinfo/vtk-developers</a><br>
<br>
<br></blockquote></div><br>
<br>_______________________________________________<br>
Powered by <a href="http://www.kitware.com" target="_blank">www.kitware.com</a><br>
<br>
Visit other Kitware open-source projects at <a href="http://www.kitware.com/opensource/opensource.html" target="_blank">http://www.kitware.com/opensource/opensource.html</a><br>
<br>
Follow this link to subscribe/unsubscribe:<br>
<a href="http://www.vtk.org/mailman/listinfo/vtk-developers" target="_blank">http://www.vtk.org/mailman/listinfo/vtk-developers</a><br>
<br>
<br></blockquote></div><br>