Segmentation fault

[ldemaz]

Hi, this looks like a very promising approach that I hope to use for cleaning up user generated polygons. I have tested it now with some of my data, and I get a segmentation fault.

$ wget https://dl.dropbox.com/u/31890709/Poly_test.zip
$ unzip Poly_test.zip

$ pprepair -i Polygon_tester.shp -o Poly_fix.shp -fix
[snip]
Repair successful (0 s). All polygons are now valid.
Triangulation already repaired!
Repaired triangulation:
Holes: 0 triangles (0.00000000 %)
Ok: 739 triangles (100.00000000 %)
Overlaps: 0 triangles (0.00000000 %)
Reconstructing polygons (geometry)...
Polygons reconstructed (0 s).
Exporting polygons...
Writing file...
Segmentation fault

Do I have to use some of the other options first on this dataset to fix it, or is there something else going on here? Will appreciate any insight on this you can provide.

Many thanks for making this available!

[dmcritchie]

I did some debugging for the researcher who reported this problem, and came up with this patch that fixes the problem. However, I can't be sure that this is the right fix, since I had to make certain assumptions about the design.

What I found while debugging with the above dataset was that 15 polygon objects were created within the program (though the running commentary generated by the program says 13 polygons). What is then done in the exportPolygons() method (which writes out the results) is to loop through all the vertices for each polygon to add a point to the output data for each one; and having done so, one last point is added, which appears to be referencing the first vertex again. Perhaps this is to close the polygon.

In any event, of the 15 polygons, the first had no vertices after cleanup. So when the code tried to reference the first vertex of the polygon (in the last step above), it crashed with a segfault since no vertices existed for that polygon. Apparently the code was not designed for a polygon object with no vertices. Perhaps the vertices were all removed during cleanup.

My change was simply to skip the last step if there were no vertices in the polygon. Hopefully that was the right fix. The researcher now reports that the results using the above dataset look good.

Here is the patch for my fix. I had to inline it since GitHub issues don't support attachments:

--- pprepair.orig/IOWorker.cpp  2013-01-31 17:12:27.935176416 -0500
+++ pprepair/IOWorker.cpp   2013-01-31 17:17:42.605176467 -0500
@@ -1261,11 +1261,16 @@
    for (std::vector<std::pair<PolygonHandle *, Polygon> >::iterator currentPolygon = outputPolygons.begin(); currentPolygon != outputPolygons.end(); ++currentPolygon) {
        OGRPolygon polygon;
        OGRLinearRing outerRing;
+       bool vertexPresent = false;
        for (Ring::Vertex_iterator currentVertex = currentPolygon->second.outer_boundary().vertices_begin();
             currentVertex != currentPolygon->second.outer_boundary().vertices_end();
             ++currentVertex) {
+           vertexPresent = true;
            outerRing.addPoint(CGAL::to_double(currentVertex->x()), CGAL::to_double(currentVertex->y()));
-       } outerRing.addPoint(CGAL::to_double(currentPolygon->second.outer_boundary().vertex(0).x()), CGAL::to_double(currentPolygon->second.outer_boundary().vertex(0).y()));
+       }
+       if (vertexPresent) {
+           outerRing.addPoint(CGAL::to_double(currentPolygon->second.outer_boundary().vertex(0).x()), CGAL::to_double(currentPolygon->second.outer_boundary().vertex(0).y()));
+       }
        polygon.addRing(&outerRing);
        for (Polygon::Hole_const_iterator currentRing = currentPolygon->second.holes_begin(); currentRing != currentPolygon->second.holes_end(); ++currentRing) {
            OGRLinearRing innerRing;

Dennis

[kenohori]

Thanks for the report and the bug fix!
I've been quite busy lately, so I haven't been able to spend time on this project, but I'll take a look at this by the end of the week.

All the best,
Ken

[kenohori]

Ok, this has been solved in the newest version. Thanks again!