mapbox / earcut.hpp

Fast, header-only polygon triangulation
ISC License
858 stars 133 forks source link

Clang-Tidy static analysis warning: dereference of a null pointer #85

Closed 0xb8 closed 3 years ago

0xb8 commented 4 years ago

See warning message here: https://godbolt.org/z/dA9tMw (give it a few seconds, static analysis is somewhat slow).

I can silence the warning by adding assert(p && p->next) in earcut.hpp:235, and it doesn't trigger on my dataset, but I'm not familiar enough with the code to be certain there's no actual bug there.

In any case, thank you for this great library!

mrgreywater commented 4 years ago

I think the bug is that at earcut.hpp#L229 it should be: if (!end) end = start->next; instead of the current if (!end) end = start;

Otherwise the while loop in the same function continues to iterate elements outside the range of (start, end)

@mourner Could you take a look at it? The same thing is likely a bug at earcut.js#L66

mourner commented 4 years ago

@mrgreywater no, I think it's correct — if you don't provide the end argument, filterPoints is supposed to go through the whole ring instead of stopping immediately. You can also see how it breaks a bunch of tests.

We could add an assert — p->next can't really be null since all rings are circularly connected linked lists once they're created.