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

Partial #331, scrub return values #980

Merged
merged 1 commit into from
May 11, 2021

Conversation

jphickey
Copy link
Contributor

@jphickey jphickey commented May 4, 2021

Describe the contribution
Confirm consistency between the return values generated by each OSAL API and the doxygen documentation. Each explicitly-returned status code should have a corresponding "retval" entry in the documentation.

Also confirms that each explicitly-returned status code has a matching case in the coverage tests that specifically checks for
that return value. Some were actually missing, where as some were just being reported incorrectly.

Finally this also corrects one argument name mismatch in QueueCreate where it was prototyped as "data_size" but implemented as "max_size".

Related to #331 (but more work to do, so should not close yet)

Testing performed
Build and sanity check CFE, run all unit tests, build osalguide documentation and confirmed no warnings

Expected behavior changes
None, test and documentation updates only.

System(s) tested on
Ubuntu 20.04

Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.

Confirm consistency between the return values generated by each
OSAL API and the doxygen documentation.  Each explicitly-returned
status code should have a corresponding "retval" entry in the
documentation.

Also confirms that each explicitly-returned status code has a
matching case in the coverage tests that specifically checks for
that return value.  Some were actually missing, where as some
were just being reported incorrectly.

Finally this also corrects one argument name mismatch in QueueCreate
where it was prototyped as "data_size" but implemented as "max_size".
@jphickey jphickey added the CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) label May 4, 2021
@jphickey
Copy link
Contributor Author

jphickey commented May 4, 2021

Note this currently just focuses on the coverage test aspect - i.e. that every return value directly generated by an API is both documented and tested for in the coverage test.

@jphickey jphickey changed the title Fix #331, scrub return values Partial #331, scrub return values May 5, 2021
@astrogeco
Copy link
Contributor

astrogeco commented May 5, 2021

CCB:2021-05-05 APPROVED

  • Scrubs return codes
  • Replace "Fix" with "Partial" or "Related"
  • Share return-code script with @ejtimmon
  • Open cFE ticket to do the same thing

@astrogeco astrogeco added the CCB:Approved Indicates code review and approval by community CCB label May 5, 2021
@skliper
Copy link
Contributor

skliper commented May 7, 2021

Needs a follow-on to add all of our standard input checking (even for pass through return codes) along with the black box tests. Probably would benefit from documenting the list - NULL pointer checks (if enabled), filename/path validity checks, etc.

@astrogeco astrogeco changed the base branch from main to integration-candidate May 11, 2021 01:54
@astrogeco astrogeco added IC:2021-05-11 and removed CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels May 11, 2021
@astrogeco
Copy link
Contributor

Needs a follow-on to add all of our standard input checking (even for pass through return codes) along with the black box tests. Probably would benefit from documenting the list - NULL pointer checks (if enabled), filename/path validity checks, etc.

Should we open a new issue for these?

Also, did we open the issue in cFE to do the same scrub?

CCB:2021-05-05, Open cFE ticket to do the same thing

@skliper
Copy link
Contributor

skliper commented May 11, 2021

Needs a follow-on to add all of our standard input checking (even for pass through return codes) along with the black box tests. Probably would benefit from documenting the list - NULL pointer checks (if enabled), filename/path validity checks, etc.

Should we open a new issue for these?

Also, did we open the issue in cFE to do the same scrub?

CCB:2021-05-05, Open cFE ticket to do the same thing

#331 is still open to complete the follow-on in OSAL. Note the tests are still being added in cFE, and have associated open issues. We just need to confirm they check the necessary return codes (follow the same patter as OSAL).

@astrogeco astrogeco merged commit 84b7abb into nasa:integration-candidate May 11, 2021
astrogeco added a commit to nasa/cFS that referenced this pull request May 11, 2021
nasa/cFE#1489, removes --quiet option so files checked go to stdout

nasa/osal#997, Enable cppcheck results output

nasa/osal#980, Scrub return values
astrogeco added a commit to nasa/cFS that referenced this pull request May 12, 2021
nasa/cFE#1492, cFE v6.8.0-rc1+dev575
nasa/osal#996, osal v5.1.0-rc1+dev434

nasa/cFE#1487, Remove broken travis-ci script
nasa/cFE#1463, generated coverage stubs for CFE core
nasa/cFE#1463, Move CFE_FS_RunBackgroundFileDump to internal API
nasa/cFE#1451, OSAL config file simplification
nasa/cFE#1489, removes --quiet option so files checked go to stdout

nasa/osal#978, configuration guide updates
nasa/osal#974, improve documentation of UtAssert API calls
nasa/osal#977, update OS_TaskCreate doc
nasa/osal#997, Enable cppcheck results output
nasa/osal#980, Scrub return values
nasa/osal#992, add local mutex to BSP console
nasa/osal#993, do not require nonblock mode
astrogeco added a commit to nasa/cFS that referenced this pull request May 12, 2021
nasa/cFE#1492, cFE v6.8.0-rc1+dev575
nasa/osal#996, osal v5.1.0-rc1+dev434

nasa/cFE#1487, Remove broken travis-ci script
nasa/cFE#1463, generated coverage stubs for CFE core
nasa/cFE#1463, Move CFE_FS_RunBackgroundFileDump to internal API
nasa/cFE#1451, OSAL config file simplification
nasa/cFE#1489, removes --quiet option so files checked go to stdout

nasa/osal#978, configuration guide updates
nasa/osal#974, improve documentation of UtAssert API calls
nasa/osal#977, update OS_TaskCreate doc
nasa/osal#997, Enable cppcheck results output
nasa/osal#980, Scrub return values
nasa/osal#992, add local mutex to BSP console
nasa/osal#993, do not require nonblock mode

Co-authored-by: Jacob Hageman <[email protected]>
Co-authored-by: Joseph Hickey <[email protected]>
@astrogeco astrogeco linked an issue May 12, 2021 that may be closed by this pull request
@jphickey jphickey deleted the fix-331-retcodes branch May 14, 2021 13:55
@skliper skliper added this to the 6.0.0 milestone Sep 24, 2021
jphickey pushed a commit to jphickey/osal that referenced this pull request Aug 10, 2022
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.

Assert return codes specified in API in functional tests
3 participants