Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

ES missing branch in CFE_ES_RegisterCDSEx, overwrite success check doesn't make sense #1929

Closed
skliper opened this issue Sep 2, 2021 · 2 comments · Fixed by #2329
Closed

Comments

@skliper
Copy link
Contributor

skliper commented Sep 2, 2021

Is your feature request related to a problem? Please describe.
I suspect there were code changes that made the check redundant/pointless since RegUpdateStatus only gets set if Status == CFE_SUCCESS before this point, so there's no way for it to not be success and for Status to also not be success.

     443         [ +  + ]:         14 :     if (RegUpdateStatus != CFE_SUCCESS)
     444                 :            :     {
     445                 :          2 :         CFE_ES_WriteToSysLog("%s: Failed to update CDS Registry (Stat=0x%08X)\n", __func__,
     446                 :            :                              (unsigned int)RegUpdateStatus);
     447                 :            : 
     448                 :            :         /*
     449                 :            :          * Return failure only if this was the primary error,
     450                 :            :          * do not overwrite a preexisting error.
     451                 :            :          */
     452         [ +  - ]:          2 :         if (Status == CFE_SUCCESS)
     453                 :            :         {
     454                 :          2 :             Status = RegUpdateStatus;
     455                 :            :         }
     456                 :            :     }

here:

if (RegUpdateStatus != CFE_SUCCESS)
{
CFE_ES_WriteToSysLog("%s: Failed to update CDS Registry (Stat=0x%08X)\n", __func__,
(unsigned int)RegUpdateStatus);
/*
* Return failure only if this was the primary error,
* do not overwrite a preexisting error.
*/
if (Status == CFE_SUCCESS)
{
Status = RegUpdateStatus;
}
}

Describe the solution you'd like
Really collapses back down into just one status... no point for two.

Describe alternatives you've considered
None

Additional context
None

Requester Info
Jacob Hageman - NASA/GSFC

@thnkslprpt
Copy link
Contributor

I suspect there were code changes that made the check redundant...

@skliper

@avan989
Copy link
Contributor

avan989 commented Jun 12, 2023

code coverage is not possible.

As stated in the description, it is not possible to have RegUpdateStatus != CFE_SUCESS and Status != CFE_SUCCESS with the current implementation.

dzbaker added a commit that referenced this issue Nov 5, 2024
…atus-check-in-CFE_ES_RegisterCDSEx

Fix #1929, Remove redundant status check in CFE_ES_RegisterCDSEx()
@dzbaker dzbaker closed this as completed in 5236e56 Nov 5, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants