> Code reviews would be another way to prevent accidental > segfaults, but we might not have enough people with enough time for that, > and some people might not want it. I personally love when someone reviews > my code.
Code reviews are of course good for quality. I think any GDAL committer should welcome any remark coming from a reviewer. But, the practical problem is indeed to find the reviewer with the time and experience to do the review (which might translate into extra costs). So practically, we cannot realistically impose systematic peer code review. I should mention that we have a "free" (as in beer) code reviewer... The Coverity static analyzer : https://scan.coverity.com/projects/749 . Any GDAL committer can request access to the details (the analysis is done on a weekly basis). Complementary with human reviewing, but not completely equivalent. But in the segfault case, it would have found the issue since there was a if( poGeomIn == NULL )test AFTER the dereferencing of the pointer, and this is something for which it has a heuristics. But if the if( poGeomIn == NULL ) didn't exist, there would be no information for the analyzer to figure out if calling SetSpatialFilter() with a NULL pointer is valid or not, so in doubt, it will shut up to avoid false positives. Practical exercice: there's a 7000 lines long patch for the SQLAnywhere driver waiting for review in Trac... And a lot of other tickets with smaller patches. My experience is that most patches coming from casual contributors needs to be at least amended to some degree. And they rarely come with the associated regression test. -- Geospatial professional services http://even.rouault.free.fr/services.html _______________________________________________ gdal-dev mailing list [email protected] http://lists.osgeo.org/mailman/listinfo/gdal-dev
