[Gdal-dev] Re: GetGeometryRef in scripts

[Charlie Savage]

> I see that r1.35 of ogr.i on 8/22 I checked in a change which added 
> %newobject to both Layer.GetFeature and Layer.GetNextFeature.  Recall 
> that %newobject tells the script that it owns the returned pointer and 
> will call the destructor (which is OGR_F_Destroy()).

Yes, this is correct.  These features are new and thus must be owned by 
the dynamic language.

> Could this be the source of the original angst?  It probably isn't 
> because chained calls still wont work because you'll end up with a 
> dangling reference.  However, it probably does fix my percieved gc 
> problem between layer and feature.

Not quite sure what you mean - but you could be running into the 
FeatureDefn ref count issue.  That causes all sorts of strange errors 
when it happens.


> Unless Charlie can tell me otherwise, I don't think there is a potential 
> gc problem between Datasource and Layer anymore...This is of course dependent on the assumption
> that the datasource reference is maintained during the lifetime of the 
> layer. 

Yes, agreed.

> I think it would be useful for my sanity if you could walk through ogr.i 
> and look at the various methods which create or consume pointers.  Any 
> function which returns a pointer owned by the script needs to have 
> %newobject.  Any function which assumes ownership of a pointer needs to 
> have a DISOWN typemap.  I see that Layer has things like SetFeature and 
> CreateFeature, I suspect they should be using DISOWN, but hey what do I 
> know.

I've gone through the OGR api fairly closely (i.e., traced each method 
with a debugger) and think what we have now is correct.  By that, I mean 
we have %newobject where we should, we have %disown where we should, and 
we have identified methods that return child objects that the dynamic 
language must let go off before their parents are freed.  However, it 
would be a good idea for someone to double check.

As far as setfeature and createfeature, they both clone the features 
that they work on.  So no, they should not use disown.

Charlie