[gdal-dev] Who really owns the OGRFeatureDefn returned by OGRLayer::GetLayerDefn()

[Vincent Mora]

On 28/02/2014 10:54, Even Rouault wrote:
> Hi Vincent,
>
>> Hi,
>>
>> OGRLayer::GetLayerDefn() doc says "The returned OGRFeatureDefn is owned by
>> the OGRLayer,
>> and should not be modified or freed by the application."
> Yes, a OGRFeatureDefn created by a layer should not be modified outside of the
> driver, otherwise chaos will happen.
>
>> But in alg/contour.cpp, in OGRContourWriter, the return value ofGetLayerDefn
>> is used to
>> create a feature that apparently takes ownership of the returned
>> OGRFeatureDefn. When this
>> feature is freed at the end ofOGRContourWriter, theOGRFeatureDefn is freed.
>>
>> Am I missing something ?
> OGRContourWriter is correct. If the OGRFeatureDefn is freed at the end of
> OGRContourWriter, this is very likely because the ref count of the feature defn
> passed to OGRContourWriter  was 0, which is incorrect. Creating a feature
> increases the ref count of the feature defn that is used to create it.
> Destroying a feature decreases the ref count, and if 0 is reached, it destroys
> the feature defn.
>
>> For info, my implementation ofGetLayerDefn is:
>>
>> OGRFeatureDefn * OGRWAsPLayer::GetLayerDefn()
>> {
>>       return &oLayerDefn;
>> }
>>
>> withoLayerDefn a member ofOGRWAsPLayer:
>>
>> classOGRWAsPLayer: public OGRLayer
>> {
>>      ...
>>      OGRFeatureDefn        oLayerDefn;
>>      ...
>> };
> This is incorrect. Your OGRWAsPLayer should have a OGRFeatureDefn* pointer, and
> do the following in its constructor :
>
> poLayerDefn = new OGRFeatureDefn();
> poLayerDefn->Reference();
>
> and in its destructor
>
> poLayerDefn->Release();
>
> (look at
> http://trac.osgeo.org/gdal/browser/trunk/gdal/ogr/ogrsf_frmts/pgdump/ogrpgdumplayer.cpp)
Thanks for that.

>> Note that the xls driver creates a new "dummy"OGRFeatureDefn  in its
>> implementation
>> ofGetLayerDefn.
> Why do you call it "dummy" ? This is perfectly valid. It is just a lazy/differed
> instanciation of the layer defn (for performance reasons). But you likely don't
> need that.
Sorry, I must have had a 'return new OGRFeatureDefn("dummy");' in my 
code and mistakenly took that code for xls driver code (which I took as 
a starting point before you mentionned pgdump). "Dummy" was referring to 
that line of code (which was my code :-( ).
> Even
>