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

Useless assignments/redundant checks - static analysis warning #1186

Closed
skliper opened this issue Feb 26, 2021 · 1 comment · Fixed by #1236 or #1258
Closed

Useless assignments/redundant checks - static analysis warning #1186

skliper opened this issue Feb 26, 2021 · 1 comment · Fixed by #1236 or #1258
Milestone

Comments

@skliper
Copy link
Contributor

skliper commented Feb 26, 2021

Is your feature request related to a problem? Please describe.
Useless assignments since FileOpened is initialized to false (or already checked for false):

FileOpened = false;

FileOpened = false;

Initialized to zero, then set to zero:

Already memset to 0:

CFE_EVS_Global.EVS_TlmPkt.Payload.LogFullFlag = false;

Already checked for NumBlockSizes . CFE_PLATFORM_ES_POOL_MAX_BUCKETS

if (NumBlockSizes == 0 || NumBlockSizes > CFE_PLATFORM_ES_POOL_MAX_BUCKETS)

Already CFE_SUCCESS:

Status = CFE_SUCCESS;

Describe the solution you'd like
Remove.

Describe alternatives you've considered
None, useless assignments in the name of future-proofing is a slippery slope. To some (like me) these useless assignments make me thing the implementer didn't fully understand the implemented logic, was sloppy/careless, or added useless logic "just in case".

Additional context
wait for #972

Requester Info
Jacob Hageman - NASA/GSFC

@skliper skliper added this to the 7.0.0 milestone Feb 26, 2021
@jphickey
Copy link
Contributor

I concur these are redundant - not even future-proofing, really - just redundant.

Also a pet peeve of mine:
instead of testing if (FileOpened == true) and if (FileOpened == false) we should be testing if (FileOpened) and if (!FileOpened), respectively.

This is (was?) more of a real risk when using uint8 as a stand-in for bool ... but with the C99 semantics of the real bool type it should be equivalent ... but it's still ugly (IMO).

@skliper skliper changed the title Useless assignments - static analysis warning Useless assignments/redundant checks - static analysis warning Mar 3, 2021
skliper added a commit to skliper/cFE that referenced this issue Mar 17, 2021
skliper added a commit to skliper/cFE that referenced this issue Mar 17, 2021
skliper added a commit to skliper/cFE that referenced this issue Mar 23, 2021
astrogeco added a commit that referenced this issue Mar 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants