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

Inconsistent location comparison #935

Closed
kkarbowiak opened this issue Jun 22, 2023 · 2 comments
Closed

Inconsistent location comparison #935

kkarbowiak opened this issue Jun 22, 2023 · 2 comments

Comments

@kkarbowiak
Copy link
Contributor

This is related to libvroom.

If I have a location created using an index

auto lhs = vroom::Location(1);

and a location created using coordinates

auto rhs = vroom::Location(vroom::Coordinates{10, 11});

I would expect the below to pass

assert(!(lhs == rhs));

but this is not always the case. The comparison code checks presence of index only on left hand side object and assumes the right hand side object to have index as well. This reads to reading from an uninitialised member variable, which is undefined behaviour.

If I flip the arguments

assert(!(rhs == lhs));

the bug is bypassed and result is correct.

@kkarbowiak kkarbowiak mentioned this issue Jun 22, 2023
2 tasks
@jcoupey jcoupey added this to the v1.14.0 milestone Jun 22, 2023
@jcoupey
Copy link
Collaborator

jcoupey commented Jun 22, 2023

Good catch, I guess this went unnoticed because when parsing json directly we enforce either never or always defining custom index values.

The whole index/coord logic (and hence operator==) for locations is a bit awkward but this is the best I could come up with at the time to handle all possible scenarios. I add to check back to recall why we even need Location::operator==. 😅

@jcoupey
Copy link
Collaborator

jcoupey commented Jun 22, 2023

Fixed in #936.

@jcoupey jcoupey closed this as completed Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants