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 #735, add comment if null terminated or not. #1168

Conversation

zanzaben
Copy link
Contributor

@zanzaben zanzaben commented Feb 16, 2021

Describe the contribution
Fixes #735
added comments for if lengths included null terminals or not.

Testing performed
Build and run unit test

Expected behavior changes
No impact to behavior

System(s) tested on
Ubuntu 20.04

Contributor Info - All information REQUIRED for consideration of pull request
Alex Campbell GSFC

@zanzaben zanzaben added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label Feb 16, 2021
@@ -47,6 +47,7 @@
** messages sent. If the pkt length field indicates the message is larger
** than this define, SB sends an event and rejects the send.
**
** This length does not need to include an extra character for NULL termination.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd remove this one to avoid confusion, the CFE_MISSION_SB_MAX_SB_MSG_SIZE isn't used for your classic "string".

@zanzaben zanzaben force-pushed the fix735_document_null_terminators branch from ec65900 to 81e907f Compare February 17, 2021 17:04
@astrogeco
Copy link
Contributor

astrogeco commented Feb 17, 2021

CCB 2021-02-17 Follow on with architectural discussion: "do we want follow the same pattern everywhere?"

  • current rule: anything inside a message DOES NOT have to be null terminated because we can't control them
  • open new issues based on findings from this

@astrogeco astrogeco removed the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label Feb 17, 2021
@astrogeco astrogeco changed the base branch from main to integration-candidate February 24, 2021 03:12
@astrogeco astrogeco merged commit ab21c87 into nasa:integration-candidate Feb 24, 2021
astrogeco added a commit to nasa/cFS that referenced this pull request Feb 26, 2021
@skliper skliper added this to the 7.0.0 milestone Sep 24, 2021
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.

document configs to indicate that MAX string sizes do not include null-terminator
3 participants