Skip to content

Fix #1667, CFE_SB_MsgHdrSize returns size_t #1674

Merged
astrogeco merged 2 commits intonasa:integration-candidatefrom
oliverhamburger:fix-1667-CFE-SB-MsgHdrSize-returns-size-t-but-still-attempts-to-return-CFE-status-code
Jul 20, 2021
Merged

Fix #1667, CFE_SB_MsgHdrSize returns size_t #1674
astrogeco merged 2 commits intonasa:integration-candidatefrom
oliverhamburger:fix-1667-CFE-SB-MsgHdrSize-returns-size-t-but-still-attempts-to-return-CFE-status-code

Conversation

@oliverhamburger
Copy link
Contributor

@oliverhamburger oliverhamburger commented Jul 19, 2021 •
edited
Loading

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
jphickey
jphickey requested changes Jul 19, 2021
View reviewed changes
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
@oliverhamburger oliverhamburger requested a review from jphickey July 20, 2021 13:53
jphickey
jphickey approved these changes Jul 20, 2021
View reviewed changes
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 astrogeco mentioned this pull request Jul 20, 2021
Merged
astrogeco added a commit to nasa/cFS that referenced this pull request Jul 20, 2021
@astrogeco
IC: 2021-07-20, cFE and osal PRs
5c712c4 
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 mentioned this pull request Jul 20, 2021
Merged
@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
@astrogeco @skliper
@jphickey @zanzaben @oliverhamburger
IC:2021-07-20, Update cfe and osal
b2f3605 
**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 <[email protected]>
Co-authored-by: Joseph Hickey <[email protected]>
Co-authored-by: Alex Campbell <[email protected]>
Co-authored-by: Oliver Hamburger <[email protected]>
@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
@oliverhamburger
Fix nasa#1667, CFE_SB_MsgHdrSize returns size_t (nasa#1674)
4c51828 
* 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

dmknutsen commented Oct 21, 2022

@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 commented Oct 21, 2022

@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 [email protected]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jphickey jphickey jphickey approved these changes

+1 more reviewer

@zanzaben zanzaben zanzaben approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CCB:Approved Indicates code review and approval by community CCB

Projects

None yet

Milestone

Caelum

Development

Successfully merging this pull request may close these issues.

6 participants

@oliverhamburger @dmknutsen @zanzaben @jphickey @skliper @astrogeco