-
Notifications
You must be signed in to change notification settings - Fork 70
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
Fix unsafe globals in Interval and Region3 #396
Fix unsafe globals in Interval and Region3 #396
Conversation
Signed-off-by: Michel Hidalgo <[email protected]>
Signed-off-by: Michel Hidalgo <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #396 +/- ##
=======================================
Coverage 99.73% 99.73%
=======================================
Files 69 69
Lines 6353 6353
=======================================
Hits 6336 6336
Misses 17 17
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but I'd like to make sure @scpeters approves too before merging
it seems right to me, but I think @azeey understands this issue better than me. I think the right way to test this is with asan using gazebosim/gz-cmake#210, but I haven't had a chance to do so. I think it's fine to merge without my approval. Once we get CI with sanitizers, we'll know for sure :) |
Sounds reasonable, we can revisit when the sanitizers are running in CI, if needed |
Signed-off-by: Michel Hidalgo <[email protected]>
Signed-off-by: Michel Hidalgo <[email protected]>
🦟 Bug fix
Relates to #269.
Summary
This PR fixes a potential global initialization order issue introduced in #388 and #390 by making all
Interval
andRegion3
construction formsconstexpr
.Checklist
codecheck
passed (See contributing)Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining
Signed-off-by
messages.