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.

Sign up for GitHub

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

Jump to bottom

Fix #888, better return codes from OS_SymbolTableDump_Impl #915

Merged
merged 1 commit into from
Mar 29, 2021
Merged

Fix #888, better return codes from OS_SymbolTableDump_Impl #915

merged 1 commit into from
Mar 29, 2021

Conversation

jphickey
Copy link
Contributor

Describe the contribution
Improve the error codes from this function.

  • Introduces a new error OS_ERR_OUTPUT_TOO_LARGE if the size limit was insufficient (instead of OS_SUCCESS).
  • Return OS_ERROR if an empty file was written - this likely indicates some fundamental issue with the VxWorks symbol table.
  • Return OS_ERR_NAME_TOO_LONG if one of the symbol names was too long, (instead of generic OS_ERROR).

Improve unit test to check for/verify these responses.

Fixes #888

Testing performed
Build and sanity check, run all unit tests, confirm coverage of new cases.

Expected behavior changes
Better/more descriptive return codes if OS_SymbolTableDump_Impl does not do what is expected.

System(s) tested on
Ubuntu 20.04

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

@jphickey jphickey added the CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) label Mar 17, 2021
@jphickey
Fix nasa#888, better return codes from OS_SymbolTableDump_Impl
c9327ee 
Improve the error codes from this function.

- Introduces a new error "OS_ERR_OUTPUT_TOO_LARGE" if the size
limit was insufficient (instead of OS_SUCCESS).
- Return OS_ERROR if an empty file was written - this likely
indicates some fundamental issue with the VxWorks symbol table.
- Return OS_ERR_NAME_TOO_LONG if one of the symbol names was
too long, (instead of generic OS_ERROR).

Improve unit test to check for/verify these responses.
@jphickey
Copy link
Contributor Author

Also note - I did go back and check OSAL 4.2.1a source code - this version also returned "OS_SUCCESS" if/when the size limit was reached/exceeded. But I think the reason is because it was not trivial to return a value through the symEach() callback, so it simply didn't. So I think it was implemented this way merely to be "bug-for-bug" compatible with the historical behavior.

Otherwise, I can't think of a good reason to return success if the file was actually truncated.....

@astrogeco
Copy link
Contributor

astrogeco commented Mar 24, 2021 •
edited
Loading

CCB:2021-03-24 APPROVED

  • Return codes were vague
  • Silently truncated files but returned a "success" code
  • Adds new error code "output too large"
  • Checks some more paths in unit tests

@astrogeco astrogeco changed the base branch from main to integration-candidate March 29, 2021 22:40
@astrogeco astrogeco added IC:2021-03-30 and removed CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels Mar 29, 2021
@astrogeco astrogeco mentioned this pull request Mar 29, 2021
Merged
@astrogeco astrogeco merged commit 318afe6 into nasa:integration-candidate Mar 29, 2021
astrogeco added a commit to nasa/cFS that referenced this pull request Mar 31, 2021
@astrogeco
IC:2021-03-30, Add remaining PRs and FastTrakcs
9b842c1 
nasa/cFE#1260 - Fix #1259, adds --branch-coverage option to genhtml line in lcov Makefile rule

nasa/osal#915 - Fix #888, better return codes from OS_SymbolTableDump_Impl
nasa/osal#926 - Fix #853, remove OS_TaskRegister
nasa/osal#935 - Fix #934, Remove unused SCRIPT_MODE flag
nasa/osal#930 - Fix #929, use test function for osal_id_t

nasa/PSP#276 - Fix #275, Adds a UT stub for CFE_PSP_GetKernelTextSegmentInfo

nasa/ci_lab#84 - Fix #73, use zero copy API
nasa/sample_app#143 - Fix #142, scrub header guards
astrogeco added a commit to nasa/cFS that referenced this pull request Apr 1, 2021
@astrogeco
cFS bundle IC:2021-03-30, Includes:
9335754 
nasa/cFE#1237 - Fix #1180, Removes impossible conditions
nasa/cFE#1240 - Fix #1002, Remove deprecated elements
nasa/cFE#1253 - Fix #1252, Squash potentially uninitialized variable warnings
nasa/cFE#1241 - Fix #1185, Squash possible uninitialized variable false alarms
nasa/cFE#1247 - Fix #1246, Typo in CFE_TBL_Validate AppName
nasa/cFE#1234 - Fix #1192, Initialize TotalMsgSize in CFE_SB_GetUserDataLength
nasa/cFE#1245 - Fix #1187, Increment CreatePipeErrorCounter for all create pipe errors
nasa/cFE#1236 - Fix #1186, Remove useless assignments/checks
nasa/cFE#1237 - Fix #1180, Removes impossible conditions
nasa/cFE#1240 - Fix #1002, Remove deprecated elements
nasa/cFE#1253 - Fix #1252, Squash potentially uninitialized variable warnings
nasa/cFE#1241 - Fix #1185, Squash possible uninitialized variable false alarms
nasa/cFE#1247 - Fix #1246, Typo in CFE_TBL_Validate AppName
nasa/cFE#1234 - Fix #1192, Initialize TotalMsgSize in CFE_SB_GetUserDataLength
nasa/cFE#1245 - Fix #1187, Increment CreatePipeErrorCounter for all create pipe errors
nasa/cFE#1236 - Fix #1186, Remove useless assignments/checks
nasa/cFE#1262 - Fix #1239, scrub include header guards
nasa/cFE#1256 - Fix #1194, check for NULL in SlotUsed helpers
nasa/cFE#1263 - Fix #1261, removed redundant checks for CFE_SUCCESS
nasa/cFE#1250 - Fix #1215, remove task registration calls
nasa/cFE#1242 - Fix #1223, shorten TestRunner function name
nasa/cFE#1242 - Fix #1264, Convert functional test startup script example to use "simple" filenames
nasa/cFE#1229 - Fix #1164, use FS file name parser for commands
nasa/cFE#1257 - Fix #1155, clean up zero copy API
nasa/cFE#1254 - Fix #1181, global variable cleanup
nasa/cFE#1255 - Fix #1206, report PSP version in ES HK TLM
nasa/cFE#1271 - Fix #1270, limit check in pool validation
nasa/cFE#1268 - Fix #1267, add null pointer check
nasa/cFE#1268 - Fix #1269, replace CFE_ES_ERR_BUFFER return for invalid null pointer arguments
nasa/cFE#1260 - Fix #1259, adds --branch-coverage option to genhtml line in lcov Makefile rule

nasa/osal#878 - Fix #843, remove BIG/LITTLE bit order macros
nasa/osal#918 - Fix #846, Minor clean up and clarification in comments/namin
nasa/osal#923 - Fix #831, Finish os-impl-bsd-socket.c coverage testing
nasa/osal#915 - Fix #888, better return codes from OS_SymbolTableDump_Impl
nasa/osal#926 - Fix #853, remove OS_TaskRegister
nasa/osal#935 - Fix #934, Remove unused SCRIPT_MODE flag
nasa/osal#930 - Fix #929, use test function for osal_id_t

nasa/PSP#276 - Fix #275, Adds a UT stub for CFE_PSP_GetKernelTextSegmentInfo
nasa/PSP#278 - Fix #277, add version file

nasa/ci_lab#83 - Fix #82, remove app registration
nasa/ci_lab#84 - Fix #73, use zero copy API

nasa/sample_app#139 - Fix #138, remove app registration
nasa/sample_app#143 - Fix #142, scrub header guards

nasa/sch_lab#75 - Fix #74, remove app registration

nasa/to_lab#96 - Fix #95, remove app registration
@jphickey jphickey deleted the fix-888-symEach-errs branch April 28, 2021 18:58
@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
@skliper
Fix nasa#915, Check for file existence in CFE_ES_RestartApp
79c4210
jphickey pushed a commit to jphickey/osal that referenced this pull request Aug 10, 2022
@astrogeco
Merge pull request nasa#1128 from skliper/fix915-restart_file_check
d9f3fa4 
Fix nasa#915, Check for file existence in CFE_ES_RestartApp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Caelum
Development

Successfully merging this pull request may close these issues.

Report symEach errors in OS_SymbolTableDump_Impl
3 participants
@jphickey @astrogeco @skliper