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

*: preliminary fixes for -Wcast-qual #17936

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

eqvinox
Copy link
Contributor

@eqvinox eqvinox commented Jan 27, 2025

⚠️ Viewer discretion is advised before reading the changes in this PR. No liability is assumed for nightmares and/or other psychological damage incurred from ingesting the cursed compiler incantations in this PR. ⚠️

This doesn't get anywhere close to being clean on -Wcast-qual, but it gets the warnings down from a few hundred thousand (due to warnings in headers being shown on every single file using that header) to less than a thousand.

I initially thought this isn't worth it, but albeit the lib/compiler.h magic is now notably longer it might actually be a bit easier to understand, so even just for that it might be worth it. (If you want to read it, I highly recommend reading the after-changes version rather than the diff.)

(also contains 2 tangentially-related C++ fixes since I had to fix those to be able to try with a C++ compiler)

`register` is archaic C, and rejected by C++ compilers.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
I have no idea why, but the C++ preprocessor doesn't like the GCC
`, ## __VA_ARGS__` magic for eliding the comma in case there are no
macro arguments...

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Not for the faint of heart, but makes `-Wcast-qual` not a total lost
cause.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Initial stab at reducing some `-Wcast-qual` warnings.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
This time with `unconst()`, which is for actual casts where const needs
to go away.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
This warning is entirely unhelpful for this generated code.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
@eqvinox
Copy link
Contributor Author

eqvinox commented Jan 27, 2025

checkpatch.pl warnings are, er, completely useless on this 🤣

The @frrbot suggestion to add that single space character is in disagreement with clang-format (version 19) on my laptop

Copy link
Contributor

@mjstapp mjstapp left a comment

Choose a reason for hiding this comment

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

Looked good to me - but I did have a question...

lib/checksum.h Show resolved Hide resolved
Not sure what is going on here, Gentoo's 8.4 and 7.5 are both fine so
I'm gonna call it Debian's mess-up.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
@donaldsharp
Copy link
Member

Waiting for freeze period to be over

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.

3 participants