fix an assertion that would fire erroneously fire due to int overflow #7257
Conversation
|
@woodpeck I have the same problem - would be good to merge. I see that one test fails - is it normal? |
| edges.insert(edges.end(), new_edges.begin(), new_end); | ||
| auto edges_size = edges.size(); | ||
| auto new_edges_size = std::distance(new_edges.begin(), new_end); | ||
| BOOST_ASSERT(static_cast<int>(edges_size) >= new_edges_size); |
There was a problem hiding this comment.
I think here is a better fix here: We want int64_t which will be equivalent to std::ptrdiff_t (the return type of std::distance). The casting to int was probably there originally because the return type of std::distance is signed.
|
Thanks for your contribution! If you have a minute to rebase and fix, I can get this until next week. Otherwise let me know, happy to just cherry-pick into a fresh PR. |
|
TBH I've mentally swapped out OSRM at present so the context switch would take me a while, if you have just looked at this then your memory is fresher than mine & if you could just salvage this into a new PR that would be much appreciated! |
TheMarex
force-pushed
the
fix_assertion
branch
from
136a2bf to
28fa3bc
Compare
3 participants
I have recently had
osrm-contractabort on a full-planet import with v6.0.0 with the problem described in issue #5440. The modification of the assertion described in that issue solves the problem but it appears it has never made it into the code proper. So here's a PR that does that; after the modification, theosrm-contractstep runs normally and produces a functioning routing graph.