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

LC has duplicate conditions leading to untestable branches #17

Closed
skliper opened this issue Apr 22, 2022 · 0 comments · Fixed by #37
Closed

LC has duplicate conditions leading to untestable branches #17

skliper opened this issue Apr 22, 2022 · 0 comments · Fixed by #37
Assignees
Milestone

Comments

@skliper
Copy link
Contributor

skliper commented Apr 22, 2022

LC_ValidateWDT has duplicate conditions in its switch statement for both DataType and OperatorID. This leads to branches that cannot be covered by unit testing.

Imported from GSFCCFS-1731

EDIT:
Duplicate conditional in PUSH_RPN_DATA, since the limit is checked before the push and can never be exceeded

#define PUSH_RPN_DATA(x) ((StackPtr >= LC_MAX_RPN_EQU_SIZE) ? (IllegalRPN = true) : (RPNStack[StackPtr++] = x))

Setting Done to true when IllegalOperand is true makes the checks redundant

LC/fsw/src/lc_action.c

Lines 416 to 423 in bb91036

if ((EvalResult == LC_WATCH_ERROR) || (EvalResult == LC_WATCH_STALE))
{
IllegalOperand = true;
}
if (StackPtr == 0)
{
Done = true;
}

if ((Done == false) && (IllegalRPN == false) && (IllegalOperand == false))

while ((Done == false) && (IllegalRPN == false) && (IllegalOperand == false))

Recommended resolutions:
Remove duplicate condions in LC_ValidateWDT, replace PUSH_RPN_DATA with simple push, don't set Done when IllegalOperand.

@skliper skliper added this to the Draco milestone Jun 13, 2022
@skliper skliper self-assigned this Jun 13, 2022
astrogeco added a commit that referenced this issue Jun 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant