Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Debug-printing geometries segfaults #365

Open
lnicola opened this issue Feb 5, 2023 · 7 comments
Open

Debug-printing geometries segfaults #365

lnicola opened this issue Feb 5, 2023 · 7 comments

Comments

@lnicola
Copy link
Member

lnicola commented Feb 5, 2023

Something like:

fn main() -> Result<()> {
    let args = std::env::args().collect::<Vec<_>>();
    let point_ds = Dataset::open(&args[1])?;
    let mut layer = point_ds.layer(0)?;
    for feature in layer.features() {
        dbg!(feature);
    }
    Ok(())
}

crashes with:

[src/main.rs:17] feature = Feature {
    _defn: Defn {
        c_defn: 0x0000562e86c81c80,
    },
    c_feature: 0x0000562e86c85d60,
    geometry: [
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', ~/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/gdal-0.14.0/src/vector/geometry.rs:177:38
stack backtrace:
   0: rust_begin_unwind
             at /rustc/efd27454ae94386e11997b1158f60d3a79deeb7d/library/std/src/panicking.rs:575:5
   1: core::panicking::panic_fmt
             at /rustc/efd27454ae94386e11997b1158f60d3a79deeb7d/library/core/src/panicking.rs:64:14
   2: core::panicking::panic
             at /rustc/efd27454ae94386e11997b1158f60d3a79deeb7d/library/core/src/panicking.rs:114:5
   3: gdal::vector::geometry::Geometry::wkt
   4: <&T as core::fmt::Debug>::fmt
@metasim
Copy link
Contributor

metasim commented Feb 5, 2023

@lnicola Was going to replicate on MacOS. Can you share the input file?

@lnicola
Copy link
Member Author

lnicola commented Feb 6, 2023

It happens with every input, like the sample from https://github.com/ngageoint/GeoPackage/blob/master/docs/examples/java/example.gpkg.

Tested with the crates.io version, though.

@metasim
Copy link
Contributor

metasim commented Feb 8, 2023

FWIW, here's what I get from master:

     Running `.../georust-gdal/target/debug/examples/segfault ./example.gpkg`
Warning 1: Database relies on the 'nga_contents_id' (http://ngageoint.github.io/GeoPackage/docs/extensions/contents-id.html) extension that should be implemented in order to read it safely, but is not currently. Some data may be missing while reading that database.
Warning 1: Database relies on the 'nga_feature_tile_link' (http://ngageoint.github.io/GeoPackage/docs/extensions/feature-tile-link.html) extension that should be implemented in order to read it safely, but is not currently. Some data may be missing while reading that database.
Warning 1: Layer point1 relies on the 'nga_geometry_index' (http://ngageoint.github.io/GeoPackage/docs/extensions/geometry-index.html) extension that should be implemented in order to read it safely, but is not currently. Some data may be missing while reading that layer.
[examples/segfault.rs:9] feature = Feature {
    _defn: Defn {
        c_defn: 0x000060000304afd0,
    },
    c_feature: 0x00006000030763f0,
    geometry: [
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', src/vector/geometry.rs:177:38

@metasim
Copy link
Contributor

metasim commented Feb 8, 2023

Not really the problem, but calling unwrap seems like a bad idea?

self.c_geometry_ref.borrow().unwrap()

@metasim
Copy link
Contributor

metasim commented Feb 15, 2023

@lnicola I wonder if this (foreign_types) or something like it might be worth considering. I think we need some principled semantics around these GDAL pointers, Rust has all the type machinery we need, we just need to find good examples of how to do this properly or rely on a library that does it for us.

@lnicola
Copy link
Member Author

lnicola commented Feb 15, 2023

Still haven't looked into it, but I think we handle layers in a better way, that could serve as inspiration.

@metasim
Copy link
Contributor

metasim commented May 16, 2023

I have foreign_types prototype here: master...metasim:gdal:prototype/foreign_types

Personally, once I converted a couple types, I really liked it (except the paucity of documentation), especially since it explicitly distinguishes owned vs. borrowed types. It's a very principled API. However, there would be several breaking changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants