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

[Even Rouault]

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)

> 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.

Even