georust / proj

Rust bindings for the latest stable release of PROJ
https://docs.rs/proj
Apache License 2.0
137 stars 45 forks source link

Handle null pointer when attempting to build an error string #74

Open urschrei opened 3 years ago

urschrei commented 3 years ago

It's possible that libproj will return a null pointer instead of an actual error string. In this case we now return a specific error indicating this.

Fixes #73

frewsxcv commented 3 years ago

The primary place that's affected by #73 is:

https://github.com/georust/proj/blob/81b233a54f2da49b5e5a5ae1aee855910df838ec/src/proj.rs#L133-L139

Specifically, the proj_errno_string function will return a null pointer if a 0 error code is passed to the function. And in such a situation, _string will attempt to dereference the null pointer which is undefined behavior, which we want to avoid.

So the question I'm wondering: do we have a guarantee that PROJ will always generate non-zero error codes?

urschrei commented 3 years ago

OK, I'll ask the authors to clarify the error code issue, and then let's see where we are.

frewsxcv commented 3 years ago

Awesome, thanks for doing that. Depending on the answer, I think there are some improvements I/we/they could make to the upstream PROJ documentation to clarify as it's currently ambiguous.

urschrei commented 3 years ago

The smallest error code that PROJ can (should) return is 1024, so anything smaller than that should be a panic.

urschrei commented 3 years ago

bors try

bors[bot] commented 3 years ago

try

Build failed:

urschrei commented 3 years ago

For some reason the proj@7 formula doesn't seem to play nicely with pkg-config, so gonna revert that change and hold off on merging this until the update to PROJ 8.1.0 has landed.

michaelkirk commented 3 years ago

For some reason the proj@7 formula doesn't seem to play nicely with pkg-config

Hmmm... I'm pretty sure I had the proj@7 working with the proj crate on my mac, but I'm also happy to wait until the whole pipeline is updated to proj8.

LMK if you'd like me to test anything locally on my mac.