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 #1667, CFE_SB_MsgHdrSize returns size_t #1674

Merged
merged 2 commits into from
Jul 20, 2021
Merged

Fix #1667, CFE_SB_MsgHdrSize returns size_t #1674

merged 2 commits into from
Jul 20, 2021

Conversation

oliverhamburger
Copy link
Contributor

@oliverhamburger oliverhamburger commented Jul 19, 2021

Describe the contribution
Fixes Issue #1667 by changing the return type in CFE_SB_MsgHdrSize and CFE_SB_GetUserDataLength

Testing performed
Build and ran unit tests

Expected behavior changes
CFE_SB_MsgHdrSize and CFE_SB_GetUserDataLength now return 0 when *MsgPtr is NULL

System(s) tested on
Ubuntu 20.04

Additional context
Add any other context about the contribution here.

Contributor Info
Oliver Hamburger GSFC

@astrogeco astrogeco requested a review from jphickey July 19, 2021 17:28
@astrogeco astrogeco added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label Jul 19, 2021
Copy link
Contributor

@jphickey jphickey left a comment

Choose a reason for hiding this comment

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

Just a minor recommendation - when documenting return values, the \returns tag should be just a generic description, and the \retval tag should be used for documenting specific values.

So, I'd recommend something like:

 \returns Estimated number of bytes in the message header for the given message
 \retval 0 if an error occurs, such as if the MsgPtr argument is not valid or header type cannot be identified

(Also to be pedantic, it is MsgPtr that is checked for NULL, not *MsgPtr)

@skliper skliper added this to the cFS-Draco milestone Jul 20, 2021
Copy link
Contributor

@jphickey jphickey left a comment

Choose a reason for hiding this comment

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

Looks good. I would only recommend a "squash commit" now to consolidate the 3 commits into one on this branch. But otherwise looks good.

@astrogeco astrogeco changed the base branch from main to integration-candidate July 20, 2021 22:56
@astrogeco astrogeco merged commit 38d6515 into nasa:integration-candidate Jul 20, 2021
astrogeco added a commit to nasa/cFS that referenced this pull request Jul 20, 2021
nasa/cFE#1670, Update API doxygen list
nasa/cFE#1671, update documentation for CFE_ES_GetPoolBufInfo
nasa/cFE#1674, CFE_SB_MsgHdrSize returns size_t

nasa/osal#1106, Add independent OS_rename functional test parameter checks
@astrogeco astrogeco changed the title Fix #1667, CFE_SB_MsgHdrSize returns size_t but still attempts to ret… Fix #1667, CFE_SB_MsgHdrSize returns size_t Jul 21, 2021
astrogeco added a commit to nasa/cFS that referenced this pull request Jul 21, 2021
**Combines**

nasa/cFE#1665, v6.8.0-rc1+dev762
nasa/osal#1113, v5.1.0-rc1+dev573

**Includes**

nasa/cFE#1664, remove default .dat extension
nasa/cFE#1660, Change CI to use Test Log.
nasa/cFE#1670, Update API doxygen list
nasa/cFE#1671, update documentation for CFE_ES_GetPoolBufInfo
nasa/cFE#1674, CFE_SB_MsgHdrSize returns size_t
nasa/cFE#1668, improve SB coverage test
nasa/cFE#1694, correct function name in UT_BSP_Unlock

nasa/osal#1106, Add independent OS_rename functional test parameter checks

Co-authored-by: Jacob Hageman <skliper@users.noreply.github.com>
Co-authored-by: Joseph Hickey <jphickey@users.noreply.github.com>
Co-authored-by: Alex Campbell <zanzaben@users.noreply.github.com>
Co-authored-by: Oliver Hamburger <oliverhamburger@users.noreply.github.com>
@astrogeco astrogeco added CCB:Approved Indicates code review and approval by community CCB and removed CCB:Ready Ready for discussion at the Configuration Control Board (CCB) labels Jul 28, 2021
paulober pushed a commit to paulober/cFE that referenced this pull request Aug 1, 2021
* Fix nasa#1667, CFE_SB_MsgHdrSize returns size_t but still attempts to return CFE status code
@skliper skliper modified the milestones: cFS-Draco, 7.0.0 Sep 27, 2021
@dmknutsen
Copy link
Contributor

@oliverhamburger can you fill out a CLA? We are trying to submit an NTR for the Draco development cycle.
https://github.com/nasa/cFS/blob/main/CONTRIBUTING.md

@oliverhamburger
Copy link
Contributor Author

@oliverhamburger can you fill out a CLA? We are trying to submit an NTR for the Draco development cycle. https://github.com/nasa/cFS/blob/main/CONTRIBUTING.md

CLA sent to GSFC-SoftwareRelease@mail.nasa.gov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CCB:Approved Indicates code review and approval by community CCB
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants