This repository was archived by the owner on Aug 8, 2023. It is now read-only.
Repository navigation
Fix segfault resulting from an invalid geometry. - #16409
Merged
Merged
Conversation
pozdnyakov
approved these changes
Apr 17, 2020
pozdnyakov
left a comment
Contributor
There was a problem hiding this comment.
Thanks for catching this! LGTM % nit
alexshalamov
approved these changes
Apr 17, 2020
Contributor
|
I kicked the CI bots. 🤖 |
Contributor
|
@lseelenbinder thanks for catching this! @tmpsantos @pozdnyakov where can @lseelenbinder add the test? Are there other invalid geometries that we should skip? |
Contributor
|
@lseelenbinder I merged before you could add a test for this PR. Do you have one? |
Contributor
Author
|
I don't, @tmpsantos. I wasn't sure where such a test should go in the testing apparatus. :) |
Contributor
|
@lseelenbinder it could be a unit test using gtest. They are inside |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Launch Checklist
needs changeloglabel if a changelog is needed (remove label when added)The current implementation of
SymbolLayout::addFeaturecan segfault on an invalid LineString (containing aLinewith no members). This skips adding invalid geometries (i.e., LineStrings) when encountered.This is a trivial bug to verify based on reading the code. I'm happy to add a test case, if someone can point me to the right place.