Fix #1338, check status of call to CFE_ES_CDS_CachePreload#1373
Fix #1338, check status of call to CFE_ES_CDS_CachePreload#1373astrogeco merged 1 commit intonasa:integration-candidatefrom
Conversation
jphickey
added
the
CCB:Ready
skliper
left a comment
skliper
left a comment
There was a problem hiding this comment.
If it can't fail, just comment as such and ignore the return. It's ignored in the one other calling location:
I don't follow: cFE/modules/es/fsw/src/cfe_es_cds.h Line 252 in e80aae9 |
The function cannot fail in this context
jphickey
force-pushed
the
fix-1338-check-status
branch
from
624d586 to
85bfba2
Compare
|
Updated to just ignore the return of CFE_ES_CDS_CachePreload here. We still need to init the local status to CFE_SUCCESS, though (it had been doing that too). Anyway, by "void function" I was referring to the parent, i.e. CFE_ES_ClearCDS() - there are no args to this function that coverage test can change, to get it to call CFE_ES_CDS_CachePreload any differently. It always calls it with the same args, and they are always good/within range so there is no way to get the failure path from this function. |
|
Would it make sense to use sizeof the appropriate Cache->Data union member (instead of the underlying type)? Then it really could never get out of sync? EDIT - this refers to the other case where it's used, since I think it's already done the "better" way here. EDIT1 - maybe (void) and add the comment this is explicitly ignored because it can't fail? |
|
CCB:2021-04-21 APPROVED with changes
|
astrogeco
added
the
CCB:Approved
astrogeco
added
IC:2021-04-27
and removed
CCB:Ready
nasa/cFE#1379, memory pool pointer type nasa/cFE#1289, ES child task functional test nasa/cFE#1289, typo in macro name nasa/cFE#1286, Remove broken BUILDDIR reference nasa/cFE#1305, remove option for "osal_compatible" nasa/cFE#1374, CFE_SUCCESS constant type nasa/cFE#1316, Remove Unused Error Codes nasa/cFE#1370, better warning about malformed startup line nasa/cFE#1373, check status of call to `CFE_ES_CDS_CachePreload` nasa/cFE#1384, update documentation for `CFE_ES_DeleteCDS`
Combines: nasa/cFE#1431 nasa/osal#975 nasa/sample_lib#61 Includes: nasa/cFE#1379, memory pool pointer type nasa/cFE#1289, ES child task functional test nasa/cFE#1289, typo in macro name nasa/cFE#1286, Remove broken BUILDDIR reference nasa/cFE#1305, remove option for "osal_compatible" nasa/cFE#1374, CFE_SUCCESS constant type nasa/cFE#1316, Remove Unused Error Codes nasa/cFE#1370, better warning about malformed startup line nasa/cFE#1373, check status of call to `CFE_ES_CDS_CachePreload` nasa/cFE#1384, update documentation for `CFE_ES_DeleteCDS` nasa/cFE#1385, exception logic when app/task is not found nasa/cFE#1372, error if alignment size not a power of two nasa/cFE#1368, remove unneeded CFE_ES_SYSLOG_APPEND macro nasa/cFE#1382, improve documentation for resourceID patterns nasa/cFE#1371, assert `CFE_RESOURCEID_MAX` is a bitmask nasa/osal#972, update documentation for read/write nasa/osal#966, add "handler" feature to utassert stub API nasa/osal#953, Adds local makefile and bundle/local unit test actions with coverage verification nasa/osal#971, socket accept using incorrect record nasa/osal#959, move async console option nasa/sample_lib#60, replace direct ref to ArgPtr with macro
3 participants
Describe the contribution
Confirm that the call returned CFE_SUCCESS before continuing.
Fixes #1338
Testing performed
Build and sanity check CFE, run all unit tests
Expected behavior changes
None
System(s) tested on
Ubuntu 20.04
Additional context
Note that the error/failure condition here is effectively dead code from the beginning. Not even coverage test can exercise it, because its a void function, and the args are largely fixed/constant values. So this now shows up as untested lines in the coverage report.
Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.