[x ] I reviewed the CF README.md file to see if the feature is in the major future work.
[x ] I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.
Is your feature request related to a problem? Please describe.
Update code to fix 2 issues identified by IV&V below:
Issue #1:
The code in question is in cf_cfdp_s.c lines 747 and 752.
CF_CFDP_S_SendEof(t) on line 747 does not return the error code CF_SEND_PDU_ERROR (checked by the else if on line 752).
CF_CFDP_S_SendEof() merely returns the result of CF_CFDP_SendEof(), and in that function, it only returns status codes CFE_SUCCESS and CF_SEND_PDU_NO_BUF_AVAIL_ERROR.
If the code is checking for CF_SEND_PDU_ERROR on line 752, but function CF_CFDP_SendEof() isn’t returning that code, then is there potentially a missing check somewhere? Perhaps there was an intent to implement/return that error code within CF_CFDP_SendEof() but it didn’t happen.
There is also an issue with the code coverage. Gcov is reporting 100%, and that is probably because the unit test file cf_cfdp_s_tests.c is written to test both error codes CF_SEND_PDU_NO_BUF_AVAIL_ERROR and CF_SEND_PDU_ERROR, using a stubbed version of CF_CFDP_SendEof(). Even though CF_CFDP_SendEof() does not actually return CF_SEND_PDU_ERROR.
Issue #2:
The code in question is in cf_cfdp_s.c lines 196-201.
CF_CFDP_SendFd() on line 196 ONLY returns CFE_SUCCESS. There is a code comment (cf_cfdp.c line 371) that says “/ this should check if any encoding error occurred /”, however there is no subsequent check. This missing check would’ve returned the CF_SEND_PDU_ERROR code that is checked on line 201.
Line 197 checks for CF_SEND_PDU_NO_BUF_AVAIL_ERROR. As we know from the first point above, CF_CFDP_SendFd() ONLY returns CFE_SUCCESS. But actually this check is redundant.
Explanation: The error code CF_SEND_PDU_NO_BUF_AVAIL_ERROR is typically set when a call to CF_CFDP_ConstructPduHeader() fails [references: cf_cfdp.c line 327-334, 426-433, 484-488, and 517-524]. Within CF_CFDP_S_SendFileData(), at the beginning, there is already a call to CF_CFDP_ConstructPduHeader() and a check for its failure on line 122. Therefore, there is no need to check for CF_SEND_PDU_NO_BUF_AVAIL_ERROR again within the same function.
Code coverage - similar comment above about the unit testing both error codes CF_SEND_PDU_NO_BUF_AVAIL_ERROR and CF_SEND_PDU_ERROR, using a stubbed version of CF_CFDP_SendFd().
Describe the solution you'd like
At one point the CF 'send' routines could return CF_SEND_PDU_ERROR. This is no longer the case and the code should be cleaned up to remove it. The second issue is valid as well, both unnecessary checks should be removed - or additional checks within CF_CFDP_SendFd should be added.
Checklist (Please check before submitting)
Is your feature request related to a problem? Please describe. Update code to fix 2 issues identified by IV&V below:
Issue #1: The code in question is in cf_cfdp_s.c lines 747 and 752.
CF_CFDP_S_SendEof(t) on line 747 does not return the error code CF_SEND_PDU_ERROR (checked by the else if on line 752). CF_CFDP_S_SendEof() merely returns the result of CF_CFDP_SendEof(), and in that function, it only returns status codes CFE_SUCCESS and CF_SEND_PDU_NO_BUF_AVAIL_ERROR.
If the code is checking for CF_SEND_PDU_ERROR on line 752, but function CF_CFDP_SendEof() isn’t returning that code, then is there potentially a missing check somewhere? Perhaps there was an intent to implement/return that error code within CF_CFDP_SendEof() but it didn’t happen.
There is also an issue with the code coverage. Gcov is reporting 100%, and that is probably because the unit test file cf_cfdp_s_tests.c is written to test both error codes CF_SEND_PDU_NO_BUF_AVAIL_ERROR and CF_SEND_PDU_ERROR, using a stubbed version of CF_CFDP_SendEof(). Even though CF_CFDP_SendEof() does not actually return CF_SEND_PDU_ERROR.
Issue #2:
The code in question is in cf_cfdp_s.c lines 196-201.
CF_CFDP_SendFd() on line 196 ONLY returns CFE_SUCCESS. There is a code comment (cf_cfdp.c line 371) that says “/ this should check if any encoding error occurred /”, however there is no subsequent check. This missing check would’ve returned the CF_SEND_PDU_ERROR code that is checked on line 201. Line 197 checks for CF_SEND_PDU_NO_BUF_AVAIL_ERROR. As we know from the first point above, CF_CFDP_SendFd() ONLY returns CFE_SUCCESS. But actually this check is redundant.
Explanation: The error code CF_SEND_PDU_NO_BUF_AVAIL_ERROR is typically set when a call to CF_CFDP_ConstructPduHeader() fails [references: cf_cfdp.c line 327-334, 426-433, 484-488, and 517-524]. Within CF_CFDP_S_SendFileData(), at the beginning, there is already a call to CF_CFDP_ConstructPduHeader() and a check for its failure on line 122. Therefore, there is no need to check for CF_SEND_PDU_NO_BUF_AVAIL_ERROR again within the same function.
Code coverage - similar comment above about the unit testing both error codes CF_SEND_PDU_NO_BUF_AVAIL_ERROR and CF_SEND_PDU_ERROR, using a stubbed version of CF_CFDP_SendFd().
Describe the solution you'd like At one point the CF 'send' routines could return CF_SEND_PDU_ERROR. This is no longer the case and the code should be cleaned up to remove it. The second issue is valid as well, both unnecessary checks should be removed - or additional checks within CF_CFDP_SendFd should be added.
Requester Info Dan Knutsen NASA Goddard