On 04/25/2012 04:31 PM, Even Rouault wrote:
Hi Ari,

Hum, I'm surprised that it has survived your testing ;-) : GetSpatialFilter()
might return NULL (in fact, in the general case, it will), so
GetSpatialFilter()->clone() should segfault.

I've not yet tested this one at all :) -- I only made sure it compiles.

I'll address the issues below later.

The autotests are in Python, I believe. Any volunteers for writing those?

Ari

hm, seems I failed to start a new thread since at least Thunderbird puts this into the old one.


CPLMalloc() cannot return NULL. Internally, it will abort if the memory
allocation failed. So you can just remove the if (!mapInput) // if (!mapMethod)
tests afterwards, or use VSIMalloc() instead. For consistency, memory allocated
with CPLMalloc()/VSIMalloc() should be freed with CPLFree()/VSIFree(), and not
free(). (There's an alternate memory allocator that does consistency checking
that can be enabled by defining #define DEBUG_VSIMALLOC in
port/cpl_vsisimple.cpp)

For the final implementation, the new methods that are the object of the RFC
will need Doxygen doc.

I don't see a strong reason to publish OGR_F_SetFieldsFromWithMap() in the C
API. This is mostly usefull for internal purposes for now.

About SetFieldsFrom(), it also changes SetFID() and SetStyleString(). This is a
bit counter-intuitive. It will be indeed interesting to make SetFeatureFrom()
call SetFeatureFrom() to avoid code duplication.

As far as the RFC is concerned, do you intend writing tests in the autotest
suite for the new methods ?

Best regards,

Even


_______________________________________________
gdal-dev mailing list
[email protected]
http://lists.osgeo.org/mailman/listinfo/gdal-dev

Reply via email to