[Gdal-dev] Re: A list of things in the wrappers and geom type in ogr

[Kevin Ruland]

Hi all,

I've been away for a couple of days and am a little bit behind.

 From what I can see in the mail there are three different questions 
going about.

1)  Ari pointed out a serious flaw in ogr.i SetGeometry.  None of the 
functions should have an explicit throw in them.  There are of course 
ways to wrap a "throw" but we don't really want to go there.  Instead 
this method should be fixed and we should just use the OGRErr typemap.  
I've committed the fix

2)  Passing nulls into methods.  The reason this is allowed by swig (in 
general) is because some c/c++ functions allow null pointers and in fact 
might do something different.  I think we should very likely code checks 
for these conditions.  There are two ways to appraoch it, we can either 
code the tests in every method, or we can code typecheck typemaps which 
can do this for us.

3)  There has been very little time spent looking at the memory 
management in the osr library.  There could be some troubles in there.

kevin


Charlie Savage wrote:

> That's very interesting.  Hadn't noticed that before.
>
> I think the new code in cpl_exceptions.i will take care of these 
> situations, so the throw would be redundant.  I think all we have to 
> do is this:
>
>   void SetFeature(OGRFeatureShadow *feature) {
>     OGR_L_SetFeature(self, feature);
>   }
>
> Then in the wrapper function we check for the error and take the 
> appropriate action.
>
> Kevin, what do you think?
>
>> For example SetProjection in Dataset.i, it takes (char *prj) 
>> parameter. If I give it uninitialized (undef) $prj in Perl, I get a 
>> segfault. There are two possibilities: 1) I write
>>
>> %typemap(perl5,in) (char const *prj)
>> {
>>  /* %typemap(freearg) (char const *prj) */
>>  if ( !$1 ) {
>>    croak("null argument is not allowed");
>>    SWIG_fail;
>>  }
>> }
>>
>> or we write in Dataset.i
>>
>> CPLErr SetProjection( char const *prj ) {
>>    if (!prj) return SOME_ERROR_CODE;
>>    return GDALSetProjection( self, prj );
>>  }
>
>
> I'll check in Ruby.  If this occurs in all languages, I'd vote choice 
> #2 (update Dataset.i).
>
>> Ah, one more thing to this list: in osr.i there is
>>
>> ~OSRSpatialReferenceShadow() {
>>    if (OSRDereference( self ) == 0 ) {
>>      OSRDestroySpatialReference( self );
>>    }
>>  }
>>
>> It seems to me that OSRDereference is currently stopping the 
>> OSRSpatialReferenceShadow being properly disposed of (I saw this in 
>> debugging, there are also some (Kevin's?) comments near this in 
>> osr.i). At least datasource.createlayer makes a copy of the osr, I'm 
>> not sure of other methods.
>
>
> Not sure, I'm not as familiar with OSR.   However, I've started to 
> test the OSR wrappers in Ruby, so I'll add it to the todo list to check.
>
> Charlie
>
> _______________________________________________
> Gdal-dev mailing list
> Gdal-dev at lists.maptools.org
> http://lists.maptools.org/mailman/listinfo/gdal-dev