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 #145, Clean up CF return codes #364

Merged
merged 2 commits into from
Mar 13, 2023

Conversation

thnkslprpt
Copy link
Contributor

Checklist

Describe the contribution
Fixes #145

  • most int/int32 return types were converted to CFE_Status_t (cases of specifically unsigned or intentionally long/short types (e.g. uint64, uint8) were not changed)
  • successful returns now almost all represented by CFE_SUCCESS rather than 0 (zero)
  • unsuccessful returns now almost all represented by CF_ERROR macro, or specific error macros where appropriate for repeat cases
  • new error return macros all negative (unlike previously where some were positive enum values)
  • CF_Timer_Expired() changed to bool return type
  • converted CF_SendRet_t enum error return values to macros (all negative)
    • deleted unused CF_SendRet_FAILURE enum value
  • converted CF_RxEofRet_t enum error return values to macros (all negative)
    • deleted unused CF_RxEofRet_INVALID enum value

Minor changes:

  • updated this test message: should say (failed), not (success):
  • couple of typos that were noticed along the way (e.g. deode instead of decode)
  • added documentation for @retval CF_SEND_PDU_NO_BUF_AVAIL_ERROR (previously CF_SendRet_NO_MSG) for CF_CFDP_SendNak() (was simply missing)
  • @retval CF_SEND_PDU_ERROR (previously CF_SendRet_ERROR) was not actually implemented/used and was removed from the prototype descriptions of:
    • CF_CFDP_SendFin()
    • CF_CFDP_SendAck()
    • CF_CFDP_SendMd()
    • CF_CFDP_SendEof()
  • removed /* error return path */ comments that are no longer relevant since the goto's were removed

Testing performed
GitHub CI actions (incl. Build + Run, Unit Tests etc.) all passing successfully.

Expected behavior changes
Behavior essentially unchanged.
Removing positive error return values eases future maintainability.
Using a defined set of error return macros improves code clarity and makes CF more consistent with cFE and the other cFS apps.
Synchronizing the return types to CFE_Status_t simplifies the code, and makes it more type-safe.

Could consider adding to this PR, or in the future, defining unique error return macros for each unique return type.

Contributor Info
Avi Weiss @thnkslprpt

Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

CodeQL-coding-standard found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

@thnkslprpt thnkslprpt marked this pull request as ready for review February 21, 2023 02:59
@dmknutsen dmknutsen self-requested a review March 2, 2023 19:36
@dzbaker
Copy link
Contributor

dzbaker commented Mar 9, 2023

Hey @thnkslprpt, would you be able to resolve the conflicts? Thanks!

@thnkslprpt thnkslprpt force-pushed the fix-145-clean-up-cf-return-codes branch 10 times, most recently from cbea274 to 9a0ba05 Compare March 12, 2023 02:15
@thnkslprpt thnkslprpt force-pushed the fix-145-clean-up-cf-return-codes branch from 9a0ba05 to 237dd4f Compare March 12, 2023 02:18
@dzbaker dzbaker merged commit 3c2889f into nasa:main Mar 13, 2023
@thnkslprpt thnkslprpt deleted the fix-145-clean-up-cf-return-codes branch March 13, 2023 20:28
@dmknutsen dmknutsen added this to the Equuleus milestone May 26, 2023
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.

Mixture of different return types, and many use "int"
4 participants