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 #54, Update initializations causing Cppcheck failure #56

Conversation

thnkslprpt
Copy link
Contributor

@thnkslprpt thnkslprpt commented Nov 7, 2022

Checklist

Describe the contribution
Fixes #54
Note: all are local variables only.
In order of the errors reported in the issue report:

mm_dump.c
In the MM_PeekCmd() function:
bool Valid = true;: Valid is assigned a value (on line 62) before any and all of its references, so it can be safely changed from an initialization to a declaration-only at the top of the function.

In the MM_DumpMemToFileCmd() function:
int32 OS_Status = OS_SUCCESS;: OS_Status is assigned a value (on line 240) before any and all of its references (and all are covered logically by this assignment on line 240).

In the MM_FillDumpInEventBuffer() function:
int32 PSP_Status;: This is a false positive. I've added a cppcheck-suppress comment on the preceding line as part of this PR.

mm_load.c
In the MM_LoadMemWIDCmd() function:
uint32 ComputedCRC = 0;: ComputedCRC is only used on line 320 and it is assigned a value on the immediately preceding line of code.

In the MM_LoadMemFromFileCmd() function:
int32 OS_Status = OS_SUCCESS;: OS_Status is assigned a value on line 388 (and again on line 529) which cover logically all of its uses in the function.

mm_mem16.c
In the MM_LoadMem16FromFile() function:
uint32 i = 0;: i is assigned a value in the init-statement of the for loop on line 82, so the initialization at the top of the function is redundant.
int32 ReadLength = 0;: ReadLength is assigned a value in the if statement on line 72, and is only used once immediately after that inside that if block.

In the MM_DumpMem16ToFile() function:
int32 OS_Status = OS_ERROR;: OS_Status is assigned a value in the if statement on line 178, and is only used in that if/else block which is all covered by that local assignment on line 178. This makes the initialization at the top of the function redundant.
uint32 i = 0;: i is assigned a value in the init-statement of the for loop on line 155, so the initialization at the top of the function is redundant.

In the MM_FillMem16() function:
uint32 i = 0;: i is assigned a value in the init-statement of the for loop on line 252, so the initialization at the top of the function is redundant.
uint32 NewBytesRemaining = 0;: NewBytesRemaining is only used inside the if block (starting on line 234) and it is assigned a value there (on line 236) before any and all of its references.

mm_mem32.c
In the MM_LoadMem32FromFile() function:
uint32 i = 0;: i is assigned a value in the init-statement of the for loop on line 82, so the initialization at the top of the function is redundant.
int32 ReadLength = 0;: ReadLength is assigned a value in the if statement on line 72, and is only used once immediately after that inside that if block.

In the MM_DumpMem32ToFile() function:
int32 OS_Status = OS_ERROR;: OS_Status is assigned a value in the if statement on line 179, and is only used in that if/else block which is fully covered by that local assignment on line 179. This makes the initialization at the top of the function redundant.
uint32 i = 0;: i is assigned a value in the init-statement of the for loop on line 155, so the initialization at the top of the function is redundant.

In the MM_FillMem32() function:
uint32 i = 0;: i is assigned a value in the init-statement of the for loop on line 253, so the initialization at the top of the function is redundant.
uint32 NewBytesRemaining = 0;: NewBytesRemaining is only used inside the if block (starting on line 235) and it is assigned a value there (on line 237) before any and all of its references

mm_mem8.c
In the MM_LoadMem8FromFile() function:
uint32 i = 0;: i is assigned a value in the init-statement of the for loop on line 82, so the initialization at the top of the function is redundant.
int32 ReadLength = 0;: ReadLength is assigned a value in the if statement on line 72, and is only used once immediately after that inside that if block.

In the MM_DumpMem8ToFile() function:
int32 OS_Status = OS_ERROR;: OS_Status is assigned a value in the if statement on line 178, and is only used in that if/else block which is fully covered by that local assignment on line 178. This makes the initialization at the top of the function redundant.
uint32 i = 0;: i is assigned a value in the init-statement of the for loop on line 155, so the initialization at the top of the function is redundant.

In the MM_FillMem8() function:
uint32 i = 0;: i is assigned a value in the init-statement of the for loop on line 241, so the initialization at the top of the function is redundant.

mm_utils.c
int32 OS_Status = OS_SUCCESS;: OS_Status is assigned a value on line 528 and is only used once on the next line.

Testing performed
GitHub CI actions (incl. Build + Run, Unit Tests etc.) all passing successfully if separate issue #55 is suppressed
image
The log from the successful build (with the GCC suppressions that can't be included in this PR) can be viewed here:
https://github.com/thnkslprpt/MM/actions/runs/3407355310/jobs/5667038143

Expected behavior changes
No impact on code behavior.
Cppcheck now passes without error again.

Contributor Info
Avi @thnkslprpt

@dzbaker dzbaker merged commit b824250 into nasa:main Jan 12, 2023
@thnkslprpt thnkslprpt deleted the fix-54-update-initializations-causing-cppcheck-failure branch January 12, 2023 21:58
@dmknutsen dmknutsen added this to the Draco milestone Jan 18, 2023
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.

New Cppcheck errors: '[unreadVariable]' and '[unusedVariable]'
4 participants