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

Fix #926: Fixed crash in GEOSInterpolate() when used with empty LineString. #126

Merged
merged 1 commit into from
Sep 24, 2018
Merged

Fix #926: Fixed crash in GEOSInterpolate() when used with empty LineString. #126

merged 1 commit into from
Sep 24, 2018

Conversation

sir-sigurd
Copy link
Contributor

@dbaston
Copy link
Member

dbaston commented Sep 21, 2018

@sir-sigurd semantics of empty geometries are always tricky. Why did you choose to have GEOSInterpolate return an empty point instead of NULL in this case? Is this what it does for other invalid cases such as non-linear inputs?

@sir-sigurd
Copy link
Contributor Author

semantics of empty geometries are always tricky

I agree with it.

Is this what it does for other invalid cases such as non-linear inputs?

GEOSInterpolate returns NULL for non-linear inputs, but I don't think that empty LineString is invalid input for GEOSInterpolate. I think it should work similar to GEOSGetCentroid which returns empty point for empty input geometries, also if I'm not missing something GEOSGetCentroid(ls) == GEOSInterpolateNormalized(ls, 0.5) for any ls with two points and I think it should also be true for empty ls.

@dbaston
Copy link
Member

dbaston commented Sep 22, 2018

Sounds logical to me. Any objections, @strk or others?

@strk
Copy link
Member

strk commented Sep 23, 2018

I agree on the empty semantic

@strk strk merged commit 18505af into libgeos:master Sep 24, 2018
@sir-sigurd sir-sigurd deleted the empty-crash-interpolate branch September 24, 2018 02:49
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

Successfully merging this pull request may close these issues.

None yet

3 participants