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

Fix #1332, Resolve compiler warnings re. signedness comparisons #2247

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

thnkslprpt
Copy link
Contributor

@thnkslprpt thnkslprpt commented Feb 20, 2023

Checklist

Describe the contribution

  • Partially fixes Resolve signed/unsigned comparison warnings #1332 - only flight code was amended. Warnings for test code still exist.
    • MinSystemState represents CFE_ES_SystemState values. Seems safe (and future-proof) to cast to a simple int given the possible range of these values.
    • ElapsedTime casted to int64_t instead of int/int32 to guarantee no conceivable chance of overflow even with (very) large msec time values.
    • BlockSize used to represent the size of various structures - fine to cast to standard int.
    • StringLength type changed to size_t (more correct given it's usage, and avoids 2 signedness comparison warnings).
    • RecordSize represents number of bytes - maybe safest with int64_t.
    • TblSizeInBytes represents number of bytes (safest with int64_t)

Testing performed
GitHub CI actions (incl. Build + Run, Unit Tests etc.) all passing successfully.
Tested locally with -Wsign-compare flag enabled - no warnings issued for cFE.

Expected behavior changes
No change to behavior.

Contributor Info
Avi Weiss @thnkslprpt

@thnkslprpt thnkslprpt marked this pull request as ready for review February 21, 2023 02:58
@thnkslprpt thnkslprpt force-pushed the fix-1332-squash-signedness-warnings branch from 7b4c47c to 235eade Compare April 19, 2024 21:31
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.

Resolve signed/unsigned comparison warnings
1 participant