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 #903, Add CFE_SB_GetUserData padding check #905

Merged

Conversation

skliper
Copy link
Contributor

@skliper skliper commented Sep 23, 2020

Describe the contribution
Fix #903 - this adds checks to see that CFE_SB_GetUserData works with all payload data types.

Illustrates the issue in #488 - As expected it fails for 64 bit payload in a telemetry packet because of implicit padding added between the header (12 bytes) and the 64bit payload (requires 64bit alignment.. so pads to 16 bytes).

Cmds are only OK because the default header is 8 bytes.

Also note - I expect these tests would also throw cast-align errors for an alignment sensitive build, since there's a cast from a structure with 64bit alignment to 32bit alignment.

Testing performed
Build and run unit tests... currently failing due to implicit padding.

Expected behavior changes
None, except shows the current issue with our header definitions.

System(s) tested on

  • Hardware: cFS Dev Server
  • OS: Ubuntu 18.04
  • Versions: main bundle + this commit

Additional context
Depends on nasa/osal#605 (address equal assert)

Third party code
None

Contributor Info - All information REQUIRED for consideration of pull request
Jacob Hageman - NASA/GSFC

@skliper skliper added this to the 7.0.0 milestone Sep 23, 2020
@skliper skliper added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label Sep 23, 2020
@skliper
Copy link
Contributor Author

skliper commented Sep 23, 2020

This breaks the build (will fail unit tests), don't merge until #488 is fixed.

@skliper skliper added CCB:Ignore Pull Request can be ignored. Will be re-examined at by next CCB. and removed CCB:Ready Ready for discussion at the Configuration Control Board (CCB) labels Sep 23, 2020
@skliper skliper force-pushed the fix903-getuserdata-padcheck branch from 4eba3c0 to 856c88a Compare January 7, 2021 23:32
@skliper skliper added CCB:Ready Ready for discussion at the Configuration Control Board (CCB) and removed CCB:Ignore Pull Request can be ignored. Will be re-examined at by next CCB. labels Jan 7, 2021
@skliper
Copy link
Contributor Author

skliper commented Jan 7, 2021

Depends on #1077 to pass

@astrogeco
Copy link
Contributor

CCB:2021-01-13 APPROVED

@astrogeco astrogeco added CCB:2021-01-13 and removed CCB:Ready Ready for discussion at the Configuration Control Board (CCB) labels Jan 13, 2021
@astrogeco astrogeco changed the base branch from main to integration-candidate January 25, 2021 02:27
@astrogeco astrogeco merged commit 6ca043d into nasa:integration-candidate Jan 25, 2021
astrogeco added a commit to nasa/cFS that referenced this pull request Jan 25, 2021
@skliper skliper deleted the fix903-getuserdata-padcheck branch February 1, 2021 22:05
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.

Update CFE_SB_GetUserData unit test to catch padding differences between header/payload
3 participants