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 #2260, Convert CFE_TBL_INFO_TABLE_LOCKED into a negative error code #2261

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

thnkslprpt
Copy link
Contributor

@thnkslprpt thnkslprpt commented Mar 23, 2023

Checklist

Describe the contribution

Testing performed
GitHub CI actions (incl. Code Coverage Analysis, Functional Test etc.) all passing successfully.

CFE_TBL_Update() (which calls CFE_TBL_UpdateInternal()) tests for (Status < 0) which is now triggered by CFE_TBL_INFO_TABLE_LOCKED as well, and the branch for a non-NULL RegRecPtr was not previously covered. With these changes, that branch is now executed by the existing tests. This results in an increase in coverage of 2 lines and 1 branch:

Prior to the changes:
  lines......: 98.1% (13074 of 13326 lines)
  functions..: 97.0% (1041 of 1073 functions)
  branches...: 97.1% (5870 of 6047 branches)
  
With the changes:
  lines......: 98.1% (13076 of 13326 lines)
  functions..: 97.0% (1041 of 1073 functions)
  branches...: 97.1% (5871 of 6047 branches)

Expected behavior changes
Behavior of the function is the same. Only the return code in the case of trying to update a locked table is changing from positive to negative.

The only use of CFE_TBL_Update() from other apps where the return value is checked (e.g. in HK) use == CFE_SUCCESS, so will not be affected by this change. Other (non-public) users who were checking the return of CFE_TBL_Update() and testing less precisely for return >= 0 (or similar equivalents) will see a change, but it seems unlikely that they would consider a non-update as success anyway.

System(s) tested on
Intel(R) Celeron(R) N4100 CPU @ 1.10GHz x86_64
Debian GNU/Linux 11 (bullseye)
Current main branch of cFS.

Contributor Info
Avi Weiss @thnkslprpt

@thnkslprpt
Copy link
Contributor Author

Note: will need to update the coverage minimums in code-coverage.yml when this is merged. To what numbers will depend on when this is merged in relation to other PRs that also improve coverage.

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.

CFE_TBL_INFO_TABLE_LOCKED should be a negative error code
2 participants