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

[Backport] Use static_cast instead of C-style casts for non-fundamental types #2185

Merged

Conversation

furszy
Copy link

@furszy furszy commented Feb 4, 2021

Coming straight from bitcoin#10498 with minimum adaptations.

A C-style cast is equivalent to try casting in the following order:

1. const_cast(...)
2. static_cast(...)
3. const_cast(static_cast(...))
4. reinterpret_cast(...)
5. const_cast(reinterpret_cast(...))

By using static_cast<T>(...) explicitly we avoid the possibility
of an unintentional and dangerous reinterpret_cast. Furthermore
static_cast<T>(...) allows for easier grepping of casts.
@furszy furszy self-assigned this Feb 4, 2021
@random-zebra random-zebra added this to the 5.1.0 milestone Feb 5, 2021
@random-zebra
Copy link

Looking good.
Could backport also bitcoin#12416 here (basically reverting the change in qt/pivx.cpp and qt/winshutdownmonitor.cpp) to fix the win issue.

@furszy
Copy link
Author

furszy commented Feb 7, 2021

done @random-zebra, added windows shutdown fix.

Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK ab6f1b6

Copy link
Collaborator

@Fuzzbawls Fuzzbawls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK ab6f1b6

@random-zebra random-zebra merged commit 9232a36 into PIVX-Project:master Feb 13, 2021
@furszy furszy deleted the 2020_backport_static_cast_fixes branch November 29, 2022 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants