osrm-backend
osrm-backend copied to clipboard
Better error handling for misaligned rasters
I recently inadvertently transposed nrows and ncols in my profile, which led to some unexpected (but subtly so) results. raster_source.hpp. My dataset has more rows than columns. There's an if statement in raster_source.hpp that skips rows past the stated last x:
if (x < xdim)
_data[(y * xdim) + x] = atoi(s.c_str());
Is there a reason why this skips data with too high an x value, rather than erroring? Also, there is error checking that the correct number of rows and columns are read, but this is done with BOOST_ASSERT; I'm guessing asserts are disabled in production since I didn't get an error. Since this is a user error not an OSRM error, does it make sense to have this throw an error and shut down building rather than using asserts? I can't imagine that the error checking is very costly performance-wise.
It does seem odd to code for a case that we then assert is not true.
https://github.com/Project-OSRM/osrm-backend/blob/3bb82ce1e2fd2299712b96176e651e6f7e999aae/include/extractor/raster_source.hpp#L66-L72
IMO it would make sense to have the assert first and then have a loop that operates on that assumption.
According to the original commit, this isn't something we can actually assert either, so either we keep the defensive coding, or ensure they do match (e.g. load the lua dimensions from the file).
PRs are welcome :slightly_smiling_face: