Fix #2588, Split up CFE_TBL_Load and remove early returns #2589

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.

Sign up for GitHub

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

Jump to bottom

Open

thnkslprpt wants to merge 1 commit into nasa:main from thnkslprpt:fix-2588-split-up-CFE_TBL_Load

Contributor

thnkslprpt commented Aug 9, 2024

Checklist

I reviewed the Contributing Guide.
I signed and emailed the appropriate Contributor License Agreement to GSFC-SoftwareRelease@mail.nasa.gov and copied cfs-program@lists.nasa.gov.

Describe the contribution

Fixes Split up CFE_TBL_Load() and remove early returns #2588
- Splits the functionality of CFE_TBL_Load() into 4 helper functions
- Removes all except the first 2 early returns (by introducing a new boolean status guard StillProcessing) - allowing us to move CFE_TBL_TxnFinish() to the end of the function
- CFE_TBL_Load() reduced from 223 -> 123 lines (and cyclomatic complexity roughly halved)

Testing performed
GitHub CI actions all passing successfully (incl. Build + Run, Unit/Functional Tests etc.) + some basic sanity checking with a cFS suite run, loading of a table etc.

Expected behavior changes
This PR minimises complexity of CFE_TBL_Load() thereby easing future maintenance/testing/refactoring. Logic is largely unchanged here, other than the points mentioned above.

System(s) tested on
Debian GNU/Linux 12 (bookworm)
Current main branch of cFS bundle.

Contributor Info
Avi Weiss @thnkslprpt


          Fix nasa#2588, Split up CFE_TBL_Load and remove early returns

ce52b70

github-advanced-security bot found potential problems

View reviewed changes

modules/tbl/fsw/src/cfe_tbl_internal.c

    
               *

               *-----------------------------------------------------------------*/

              CFE_TBL_LoadBuff_t *CFE_TBL_GetSelectedBuffer(CFE_TBL_RegistryRec_t *     RegRecPtr,

              CFE_TBL_LoadBuff_t *CFE_TBL_GetSelectedBuffer(CFE_TBL_RegistryRec_t      *RegRecPtr,

Check notice

Code scanning / CodeQL

Long function without assertion

All functions of more than 10 lines should have at least one assertion.

modules/tbl/fsw/src/cfe_tbl_internal.c

    
               * See description in header file for argument/return detail

               *

               *-----------------------------------------------------------------*/

              CFE_Status_t CFE_TBL_LoadDumpOnlyTable(CFE_TBL_RegistryRec_t *RegRecPtr, const char *AppName, const void *SrcDataPtr)

Check notice

Code scanning / CodeQL

Long function without assertion

All functions of more than 10 lines should have at least one assertion.

modules/tbl/fsw/src/cfe_tbl_internal.c

    
               * See description in header file for argument/return detail

               *

               *-----------------------------------------------------------------*/

              CFE_Status_t CFE_TBL_LoadWorkingBuffer(const CFE_TBL_SrcEnum_t SrcType, const char *AppName,

Check notice

Code scanning / CodeQL

Long function without assertion

All functions of more than 10 lines should have at least one assertion.

modules/tbl/fsw/src/cfe_tbl_internal.c

    
               *

               *-----------------------------------------------------------------*/

              CFE_Status_t CFE_TBL_ValidateWorkingBufferContents(CFE_TBL_RegistryRec_t *RegRecPtr,

Check notice

Code scanning / CodeQL

Long function without assertion

All functions of more than 10 lines should have at least one assertion.

modules/tbl/fsw/src/cfe_tbl_internal.c

    
              CFE_Status_t CFE_TBL_ValidateWorkingBufferContents(CFE_TBL_RegistryRec_t *RegRecPtr,

                                                                 CFE_TBL_LoadBuff_t *WorkingBufferPtr, const char *AppName)

              {

                  CFE_Status_t Status = (RegRecPtr->ValidationFuncPtr)(WorkingBufferPtr->BufferPtr);

Check notice

Code scanning / CodeQL

Use of non-constant function pointer

This call does not go through a const function pointer.

modules/tbl/fsw/src/cfe_tbl_internal.c

    
               * See description in header file for argument/return detail

               *

               *-----------------------------------------------------------------*/

              CFE_Status_t CFE_TBL_PerformUpdate(CFE_TBL_RegistryRec_t *RegRecPtr, const CFE_TBL_Handle_t TblHandle,

Check notice

Code scanning / CodeQL

Long function without assertion

All functions of more than 10 lines should have at least one assertion.

github-advanced-security bot found potential problems

View reviewed changes

modules/tbl/fsw/src/cfe_tbl_internal.c

    
                  {

                      /* The Application is allowed to call Load once when the address  */

                      /* of the dump only table is being defined by the application.    */

                      RegRecPtr->Buffers[0].BufferPtr = (void *)SrcDataPtr;

Check warning

Code scanning / CodeQL

Local variable address stored in non-local memory

A stack address which arrived via a [parameter](1) may be assigned to a non-local variable.

Contributor

jphickey commented Nov 14, 2025

The TBL Load function is totally reworked in the upcoming release (not on github yet unfortunately) so this is likely overridden by that. Probably will not be able to merge this one (or at least will need to re-evaluate after we drop the new code)

Contributor Author

thnkslprpt commented Nov 15, 2025

The TBL Load function is totally reworked in the upcoming release (not on github yet unfortunately) so this is likely overridden by that. Probably will not be able to merge this one (or at least will need to re-evaluate after we drop the new code)

No worries - I'll check once that's uploaded if there's anything useful left from here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet