[gdal-dev] GDAL/OGR 1.11.0 Release Candidate 1 available for testing

[Even Rouault]

> 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