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 #1708, Align padding explicitly in cfe_tbl_msg.h #2278

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

Conversation

thnkslprpt
Copy link
Contributor

@thnkslprpt thnkslprpt commented Apr 3, 2023

Checklist

Describe the contribution

  • Fixes Implicit padding in CFE_TBL_HousekeepingTlm_Payload_t #1708
    • The 2-byte implicit padding in CFE_TBL_HousekeepingTlm_Payload_t as noted in the issue has been made explicit, and my testing showed 2-byte implicit padding on the end of CFE_TBL_TblRegPacket_Payload_t as well, which has also been rectified and made explicit.

Note: If this is merged, it will require updates to cfe-tbl-hk-tlm.txt in the Ground System tool to ensure clean and correct interpretation of the data members.

Testing performed
GitHub CI actions all passing successfully.
Local testing confirms all cFS bundle tests passing and coverage unaffected.

Expected behavior changes
Compilers trying to align to 32-bits will not longer add implicit padding in these 2 locations.

System(s) tested on
Debian GNU/Linux 11 (bullseye)
Current main branch of cFS bundle.

Contributor Info
Avi Weiss @thnkslprpt

@thnkslprpt thnkslprpt force-pushed the fix-1708-align-padding-explicitly-in-cfe-tbl-msg branch from 9e43957 to 945c7df Compare May 5, 2023 04:36
@dzbaker dzbaker requested a review from dmknutsen June 8, 2023 18:34
@thnkslprpt
Copy link
Contributor Author

thnkslprpt commented Jun 10, 2023

There are a few other cases of implicit padding in the msg structs in the other cFE modules.

See quick-fixes here which make all compiler-added padding explicit in the msg files: main...thnkslprpt:cFE:fix-padding-issue-in-msgstruct-files
(There was also CFE_SB_RouteCmd_Payload_t which seems to have an unnecessary spare byte there...)

Should I add those on to this PR or just open a new issue?

@thnkslprpt thnkslprpt force-pushed the fix-1708-align-padding-explicitly-in-cfe-tbl-msg branch 2 times, most recently from 23b0bda to 089c185 Compare October 26, 2023 13:41
@thnkslprpt thnkslprpt force-pushed the fix-1708-align-padding-explicitly-in-cfe-tbl-msg branch from 089c185 to 3a198e2 Compare October 26, 2023 13:45
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.

Implicit padding in CFE_TBL_HousekeepingTlm_Payload_t
1 participant