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

Subtraction underflow crash calculating backwards lane count #102

Closed
dmfutcher opened this issue Oct 15, 2022 · 3 comments · Fixed by #103
Closed

Subtraction underflow crash calculating backwards lane count #102

dmfutcher opened this issue Oct 15, 2022 · 3 comments · Fixed by #103

Comments

@dmfutcher
Copy link
Contributor

dmfutcher commented Oct 15, 2022

Hi, I ran into a crash when trying to load Edinburgh, Scotland into A/B Street:
thread 'main' panicked at 'attempt to subtract with overflow', /home/david/.cargo/git/checkouts/osm2streets-fbbbdf753e406749/a55aef7/osm2streets/src/lanes/classic.rs:110:20

Did some debugging and saw here, n = 2 and num_driving_fwd=3, which doesn't make much sense, but was causing a crash.

I added a workaround to check for situations where that subtraction would underflow and default to 0 backward lanes, diff here: main...dmfutcher:osm2streets:fix-lanes-calculation-crash. This works and I successfully loaded Edinburgh, but I'm not sure if that's the "correct" way to fix it. I'd be happy to hack on this and submit a PR if you could give me some guidance on the correct way to handle this situation (I found this software looking for something to hack on for Hacktoberfest).

You can repro using this GeoJSON: https://gist.github.com/dmfutcher/2b906f1b2b953a62ea1da8b361ab2fed

@dabreegster
Copy link
Contributor

dabreegster commented Oct 15, 2022 via email

@dmfutcher
Copy link
Contributor Author

PR submitted. I found and fixed the issue in the OSM data (way 413519168).

dabreegster added a commit to a-b-street/abstreet that referenced this issue Oct 15, 2022
@dabreegster
Copy link
Contributor

Edinburgh looks alright after this fix! It's a huge area and slow to import / use, of course. The missing ocean is a-b-street/abstreet#32. If you hit more problems or get interested in working on anything, let me know -- thanks for trying stuff out and sending a fix!

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 a pull request may close this issue.

2 participants