[gdal-dev] RFC 39

[Even Rouault]

Hi Ari,

> I've uploaded a new patch, which hopefully corrects the issues Even
> brought up. Below are comments.

I've reviewed quickly and the new version seems to address my comments well.

>
> I've not added the Append or Buffer methods, which basically work on
> just one layer. I believe such functionality is simple enough to create
> using feature iteration.
>
> Ari
>
> the patch:
> http://trac.osgeo.org/gdal/attachment/wiki/rfc39_ogr_layer_algebra/rfc39.patch
>

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.

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