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

treewide: make type casts explicit #170

Draft
wants to merge 28 commits into
base: main
Choose a base branch
from

Conversation

JKRhb
Copy link
Contributor

@JKRhb JKRhb commented Sep 28, 2022

As another follow-up to #126, when compiling with Visual Studio, there are a lot of warnings regarding a potential data loss due to implicit casts with a potential lossy conversion.

This PR makes the type casts in question explicit, getting rid of potential warnings.

@JKRhb JKRhb marked this pull request as ready for review September 28, 2022 14:20
@obgm
Copy link
Contributor

obgm commented Sep 30, 2022

Thanks for working on this. The warnings usually are there for a reason, and shutting them up with type casts is not always the best solution. One example is the return type of dtls_ccm_encrypt_message() which seems to be wrong (the result of adding two size_t values obviously should be size_t; The explicit typecast therefore should result in another warning...).
For this (and some other places), I recommend to double-check if there are no cleaner ways to address the compiler warnings.

@JKRhb
Copy link
Contributor Author

JKRhb commented Sep 30, 2022

Thank you for your feedback, @obgm! I will revisit the casts and try to come up with a cleaner solution :)

@JKRhb
Copy link
Contributor Author

JKRhb commented Nov 18, 2022

Just a quick update: I now managed to get rid of most of the type casts while keeping stricter compiler settings (adding the -Wconversion and -Wno-sign-conversion flags). However, this required changing some of the return types from int to ssize_t in order to be able to cover -1 as a return value. Is using ssize_t fine in general (since it is not a C99, but a POSIX type)? Or would a different type (such as intmax_t) be preferable here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants