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 #67, remove use of bitfields in CF #103

Merged
merged 1 commit into from
Dec 9, 2021

Conversation

jphickey
Copy link
Contributor

@jphickey jphickey commented Dec 8, 2021

Bit field behavior is platform-specific, bits are not specified to be in any particular order. Furthermore, unions of bitfields are likely undefined behavior.

This removes the bitfields and replaces with normal fields.

Fixes #67

NOTE: The cost of doing this is slightly larger data size. I checked the size of the structure before and after to quantify this - the "flags" structure grew by 16 bytes on my dev system (x86-64, gcc 11.2).

The net result is that the memory footprint of the CF global grows by 1600 bytes (due to 100 transactions in default config). However, this is partially mitigated by a slight reduction in code size, of approximately 300 bytes. It probably runs faster too (although I did not quantify that - not as easy to test) but the 300 bytes of extra code were obviously being executed every time these flags were read/written, that adds up.

@jphickey jphickey added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label Dec 8, 2021
@jphickey
Copy link
Contributor Author

jphickey commented Dec 8, 2021

This also corrects what was a misnomer - the "all" flags structure was really just the common flags between rx+tx, definitely not all the flags. So it was renamed to "com".

Bit field behavior is platform-specific, bits are not
specified to be in any particular order. Furthermore,
unions of bitfields are likely undefined behavior.

This removes the bitfields and replaces with normal
fields.
@astrogeco astrogeco merged commit 1725cd0 into nasa:main Dec 9, 2021
@jphickey jphickey deleted the fix-67-bitfields branch December 9, 2021 16:39
@astrogeco astrogeco removed the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label Jan 5, 2022
@astrogeco
Copy link
Contributor

CCB:2022-01-05 APPROVED

  • Substantially reduced size of compiled code by removing the shifts and masks "under the hood"

@skliper skliper added this to the Draco milestone Mar 25, 2022
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.

CF should not use bitfields
3 participants