[gdal-dev] Question on OGR handle types

[Even Rouault]

Answers below

P.S. : Don't forget to use 'Answer to all' when answering to a post so that 
the list is CC'ed

----- Original Message ----- 
From: "Stefan Moebius" <stefan.moebius at actix.com>
To: "Even Rouault" <even.rouault at mines-paris.org>
Sent: Friday, August 07, 2009 11:45 AM
Subject: Re: [gdal-dev] Question on OGR handle types


> Even,
>
> Thanks for your reply.
>
> No we aren't actually afraid to use casts, we were just wondering what the 
> point is in changing the GDAL sources to improve type safety, just to tell 
> users to use not-really-type safe casts?

That depends on the point of view. The change to turn all "type void* foo" 
into something more specific enabled us to discover bugs in code using the C 
API in a few places inside GDAL itself as all C handles were equivalent 
before.

>
> > For the C++ part, we could argue there's a lack of a static void
> > OGRCoordinateTransformation::Destroy(OGRCoordinateTransformation*) 
> > method
> > to avoid using the delete operator, which can be an issue on Windows.
>
> Yes, that's exactly what we do argue ;-)
> (The pure-C way is not an option in our case.)
>
> Would it make sense for us to come up with a patch for adding this 
> function?

Patch welcome. Please post it as an attachement to a ticket in GDAL Trac. It 
should be done against svn trunk code ideally.

>
> Cheers,
> Stefan
>
> Even Rouault wrote:
>> Stefan,
>>
>> Why are you so much afraid about using cast ? ;-) This is probably the
>> easiest solution for you :
>> OGRCoordinateTransformation* poCT =
>> OGRCreateCoordinateTransformation(poSRS1, poSRS2);
>> OCTDestroyCoordinateTransformation( (OGRCoordinateTransformationH) poCT);
>>
>> No worry to have. It's exactly the kind of casts done in GDAL/OGR C API
>> wrappers to call the C++ API.
>> For example :
>>
>> OGRCoordinateTransformationH CPL_STDCALL OCTNewCoordinateTransformation(
>>  OGRSpatialReferenceH hSourceSRS, OGRSpatialReferenceH hTargetSRS )
>> {
>>  return (OGRCoordinateTransformationH)
>>    OGRCreateCoordinateTransformation(   (OGRSpatialReference *) 
>> hSourceSRS,
>> (OGRSpatialReference *) hTargetSRS );
>> }
>>
>> void CPL_STDCALL OCTDestroyCoordinateTransformation(
>> OGRCoordinateTransformationH hCT )
>> {
>>  delete (OGRCoordinateTransformation *) hCT;
>> }
>>
>> Otherwise, as you've noted, the stricter typing is only enforced when 
>> DEBUG
>> is defined.
>> So your code should compile without any change if you don't define DEBUG.
>>
>> The fundamental issue in fact is that you're mixing use of C++ API and C
>> API.
>> A pure C solution would be to use the pair
>> OCTNewCoordinateTransformation()/OCTDestroyCoordinateTransformation().
>> For the C++ part, we could argue there's a lack of a static void
>> OGRCoordinateTransformation::Destroy(OGRCoordinateTransformation*) method
>> to avoid using the delete operator, which can be an issue on Windows.
>>
>> ----- Original Message ----- From: "Stefan Moebius" 
>> <stefan.moebius at actix.com>
>> To: <gdal-dev at lists.osgeo.org>
>> Sent: Wednesday, August 05, 2009 5:59 PM
>> Subject: [gdal-dev] Question on OGR handle types
>>
>>
>>> Hi,
>>>
>>> In May 2008, a couple of changes where made to improve type checking.
>>>
>>> See http://lists.osgeo.org/pipermail/gdal-dev/2008-May/017059.html
>>> http://trac.osgeo.org/gdal/changeset/14435
>>> http://trac.osgeo.org/gdal/changeset/14441
>>>
>>> Our code is using the function
>>>
>>> OGRCoordinateTransformation* OGRCreateCoordinateTransformation
>>> (OGRSpatialReference * poSource,
>>> OGRSpatialReference * poTarget)
>>>
>>> and with the upgrade from 1.5 to 1.6, we now cannot properly destroy the 
>>> OGRCoordinateTransformation anymore (at least in debug mode).
>>>
>>> The documentation of that create function says:
>>>
>>>> Create transformation object.
>>>>
>>>> This is the same as the C function OCTNewCoordinateTransformation().
>>>>
>>>> Input spatial reference system objects are assigned by copy (calling 
>>>> clone() method) and no ownership transfer occurs.
>>>>
>>>> The delete operator, or OCTDestroyCoordinateTransformation() should be 
>>>> used to destroy transformation objects.
>>>
>>> As we are linking the GDAL dynamically, using delete is not really an 
>>> option as we cannot predict what heap was used to create the object. The 
>>> destroy function, however, requires a parameter of type 
>>> OGRCoordinateTransformationH, which we cannot cleanly cast to anymore 
>>> for that change mentioned at the beginning.
>>>
>>> What are we missing here? How is this supposed to be used?
>>>
>>> Any help is appreciated.
>>>
>>> Regards,
>>> Stefan
>
> -- 
> Stefan Möbius
> Development Manager
>
> Actix GmbH
> Altmarkt 10
> 01067 Dresden
> Germany
>
> T +49 351 40429 17
> www.actix.com
>
>