segfault in SHPCreateObject

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

segfault in SHPCreateObject

Ari Jolma
Any ideas why this segfaults:

my $fd = new ogr::FeatureDefn;
my $f = new ogr::Feature($fd);
my $g = new ogr::Geometry($ogr::wkbPolygon);
my $r = new ogr::Geometry($ogr::wkbLinearRing);
do this many times:        $r->AddPoint($lon,$lat);
$g->AddGeometry($r);
$g->CloseRings;
$f->SetGeometry($g);
$layer->CreateFeature($f);

layer should be ok, I probably need something more into those new's?

Ari

--
Prof. Ari Jolma
Kartografia ja Geoinformatiikka / Cartography and Geoinformatics
Teknillinen Korkeakoulu / Helsinki University of Technology
POBox 1200, 02015 TKK, Finland
Email: ari.jolma at tkk.fi URL: http://www.tkk.fi/~jolma

_______________________________________________
Gdal-dev mailing list
[hidden email]
http://lists.maptools.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|

Re: segfault in SHPCreateObject

Frank Warmerdam-2
On 9/12/05, Ari Jolma <[hidden email]> wrote:

> Any ideas why this segfaults:
>
> my $fd = new ogr::FeatureDefn;
> my $f = new ogr::Feature($fd);
> my $g = new ogr::Geometry($ogr::wkbPolygon);
> my $r = new ogr::Geometry($ogr::wkbLinearRing);
> do this many times:        $r->AddPoint($lon,$lat);
> $g->AddGeometry($r);
> $g->CloseRings;
> $f->SetGeometry($g);
> $layer->CreateFeature($f);
>
> layer should be ok, I probably need something more into those new's?

Ari,

When creating a feature you intend to write out to a layer with
CreateFeature(), you should use the featuredefn from that layer
for the feature.   It should be very rare that you need to create
your own freestanding OGRFeatureDefn objects.

Best regards,

--
---------------------------------------+--------------------------------------
I set the clouds in motion - turn up   | Frank Warmerdam, [hidden email]
light and sound - activate the windows | http://pobox.com/~warmerdam
and watch the world go round - Rush    | Geospatial Programmer for Rent

_______________________________________________
Gdal-dev mailing list
[hidden email]
http://lists.maptools.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|

Re: segfault in SHPCreateObject

Ari Jolma
In reply to this post by Ari Jolma
Back to this.. there were a couple of things which were sorted out in
IRC yesterday, there is the fact that it segfaulted, which is a policy
issue perhaps, the policy could be "one should not be able to produce a
seg fault by a scripting interface". Does that sound reasonable?

The issue here is that if a crippled polygon (into which no ring has
been put, i.e. the $g->AddGeometry($r); is missing from below -- as it
was because it silently failed) is given to SHPWriteOGRObject it seg
faults. This is because it uses getNumInteriorRings()+1 as the nRings,
thus implicitly assuming ExteriorRing exists. SHPWriteOGRObject could
also return "corrupt data" in this case.

Ari

Ari Jolma kirjoitti:

> Any ideas why this segfaults:
>
> my $fd = new ogr::FeatureDefn;
> my $f = new ogr::Feature($fd);
> my $g = new ogr::Geometry($ogr::wkbPolygon);
> my $r = new ogr::Geometry($ogr::wkbLinearRing);
> do this many times:        $r->AddPoint($lon,$lat);
> $g->AddGeometry($r);
> $g->CloseRings;
> $f->SetGeometry($g);
> $layer->CreateFeature($f);
>
> layer should be ok, I probably need something more into those new's?
>
> Ari
>


--
Prof. Ari Jolma
Kartografia ja Geoinformatiikka / Cartography and Geoinformatics
Teknillinen Korkeakoulu / Helsinki University of Technology
POBox 1200, 02015 TKK, Finland
Email: ari.jolma at tkk.fi URL: http://www.tkk.fi/~jolma

_______________________________________________
Gdal-dev mailing list
[hidden email]
http://lists.maptools.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|

Re: segfault in SHPCreateObject

Ari Jolma
In reply to this post by Frank Warmerdam-2
Frank Warmerdam wrote:

>When creating a feature you intend to write out to a layer with
>CreateFeature(), you should use the featuredefn from that layer
>for the feature.   It should be very rare that you need to create
>your own freestanding OGRFeatureDefn objects.
>  
>

Sorry for making multiple threads but my thunderbird tends to classify
gdal-dev as junk and I did not notice this before.

Ok, but that was not the source of the segfault I think. In this case
the layer is a brand new, just constructed. So the correct (?) way to
create a new layer  is to:

create layer
CreateFields into the layer
featuredefn = layer.GetLayerDefn
use this featuredefn for all features which are CreateFeature'd into the
layer

Ari

_______________________________________________
Gdal-dev mailing list
[hidden email]
http://lists.maptools.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|

Re: segfault in SHPCreateObject

Frank Warmerdam-2
On 9/13/05, Ari Jolma <[hidden email]> wrote:
> Sorry for making multiple threads but my thunderbird tends to classify
> gdal-dev as junk and I did not notice this before.

Ari,

/me expresses shock that gdal-dev would be considered junk!
 
> Ok, but that was not the source of the segfault I think. In this case
> the layer is a brand new, just constructed. So the correct (?) way to
> create a new layer  is to:
>
> create layer
> CreateFields into the layer
> featuredefn = layer.GetLayerDefn
> use this featuredefn for all features which are CreateFeature'd into the
> layer

This is the correct sequence.  

I do think that OGR ought to do some more run-time
checking that features use the right featuredefn.  
If you point out particular cases that don't currently I
will look into it.

Unfortunately, due to the OGR architecture where
methods such as CreateFeature() are directly overridden
by the format specific classes it is hard to check
pre-conditions without incorporating the code into all
the driver implementations.  I could put some extra
checking in the C cover functions which would help
applications using the C interface (and all the swig bindings).
Perhaps that would be a better angle.
 
Unfortunately that leaves us in the case where the C++
interface becomes uniquely dangerous.

Best regards,
--
---------------------------------------+--------------------------------------
I set the clouds in motion - turn up   | Frank Warmerdam, [hidden email]
light and sound - activate the windows | http://pobox.com/~warmerdam
and watch the world go round - Rush    | Geospatial Programmer for Rent

_______________________________________________
Gdal-dev mailing list
[hidden email]
http://lists.maptools.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|

Re: segfault in SHPCreateObject

Ari Jolma
Frank Warmerdam kirjoitti:

>I do think that OGR ought to do some more run-time
>checking ...
>it is hard to check
>pre-conditions without incorporating the code into all
>the driver implementations.  I could put some extra
>checking in the C cover functions which would help
>applications using the C interface (and all the swig bindings).
>Perhaps that would be a better angle.
>
>Unfortunately that leaves us in the case where the C++
>interface becomes uniquely dangerous.
>  
>

The question was whether "one should not be able to produce a seg fault
by a scripting interface" sounded reasonable. I guess that's of course
also up to the binding developers to put more shields. I'm very
interested in having bullet proof code in the sense that the (stupid)
end user hacking the script just shouldn't be able to blow up to whole
thing. That's because I'm giving the end user a CLI where to hack in his
stupid code, and they will do that since they are often students in my
case. Cryptic error messages are always better than segfaults. But as
said, there are many paths.

Ari

_______________________________________________
Gdal-dev mailing list
[hidden email]
http://lists.maptools.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|

Re: segfault in SHPCreateObject

Frank Warmerdam-2
On 9/13/05, Ari Jolma <[hidden email]> wrote:
> The question was whether "one should not be able to produce a seg fault
> by a scripting interface" sounded reasonable.

Ari,

I think it is over ambitious in the case of GDAL and OGR
to expect to achieve the above goal.  However, I would like
to try and make it somewhat hard to produce a segfault
from the scripting interface.

> I guess that's of course
> also up to the binding developers to put more shields. I'm very
> interested in having bullet proof code in the sense that the (stupid)
> end user hacking the script just shouldn't be able to blow up to whole
> thing. That's because I'm giving the end user a CLI where to hack in his
> stupid code, and they will do that since they are often students in my
> case. Cryptic error messages are always better than segfaults. But as
> said, there are many paths.

I understand the rationale, I just think it may be over ambitious.

Best regards,
--
---------------------------------------+--------------------------------------
I set the clouds in motion - turn up   | Frank Warmerdam, [hidden email]
light and sound - activate the windows | http://pobox.com/~warmerdam
and watch the world go round - Rush    | Geospatial Programmer for Rent

_______________________________________________
Gdal-dev mailing list
[hidden email]
http://lists.maptools.org/mailman/listinfo/gdal-dev