Fix #2546, add handle list operation routines#2548

Merged

dzbaker merged 1 commit intonasa:mainfrom

jphickey:fix-2546-foreach-accessdesc

Apr 16, 2024

Contributor

jphickey commented Apr 5, 2024 •

edited

Loading

Checklist (Please check before submitting)

I reviewed the Contributing Guide.
I signed and emailed the appropriate Contributor License Agreement to [email protected] and copied [email protected].

Describe the contribution
Adds a "HandleLink" structure to encapsulate the linked list functionality and add generic routines to initialize and test/check this structure, along with list insert/remove functions for access descriptors that use this link struct interally.

This also cleans up documentation of the table resource ID functions defined in cfe_tbl_resource.h

Fixes #2546

Testing performed
Build and run all tests

Expected behavior changes
None (code clean up)

System(s) tested on
Debian

Additional context
Improves maintainability of code by moving linked list ops into common routines

Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.

github-advanced-security bot found potential problems

View reviewed changes

modules/tbl/fsw/src/cfe_tbl_internal.h Fixed Show fixed Hide fixed

modules/tbl/fsw/src/cfe_tbl_internal.c

+                          break;
+                      }
+                      Func(AccDescPtr, Arg);

Check notice

Code scanning / CodeQL-coding-standard

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
+               *
+               *-----------------------------------------------------------------*/
+              void CFE_TBL_HandleListInsertLink(CFE_TBL_RegistryRec_t *RegRecPtr, CFE_TBL_AccessDescriptor_t *AccessDescPtr)

Check notice

Code scanning / CodeQL-coding-standard

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
+               *
+               *-----------------------------------------------------------------*/
+              void CFE_TBL_HandleListRemoveLink(CFE_TBL_RegistryRec_t *RegRecPtr, CFE_TBL_AccessDescriptor_t *AccessDescPtr)

Check notice

Code scanning / CodeQL-coding-standard

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
+               *
+               *-----------------------------------------------------------------*/
+              void CFE_TBL_ForeachAccessDescriptor(CFE_TBL_RegistryRec_t *RegRecPtr, CFE_TBL_AccessDescFunc_t Func, void *Arg)

Check notice

Code scanning / CodeQL-coding-standard

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
+               *
+               *-----------------------------------------------------------------*/
+              void CFE_TBL_HandleListGetSafeLink(CFE_TBL_RegistryRec_t *RegRecPtr, CFE_TBL_AccessDescriptor_t *AccDescPtr,

Check notice

Code scanning / CodeQL-coding-standard

Long function without assertion

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

modules/tbl/fsw/src/cfe_tbl_api.c

+                  CFE_TBL_TxnState_t *Txn = Arg;
+                  /* Only notify *OTHER* applications that the contents have changed */
+                  if (!CFE_RESOURCEID_TEST_EQUAL(AccessDescPtr->AppId, CFE_TBL_TxnAppId(Txn)))

Check warning

Code scanning / CodeQL-coding-standard

Side effect in a Boolean expression

This Boolean expression is not side-effect free.

modules/tbl/fsw/src/cfe_tbl_transaction.c

    
                  /* If this was the last Access Descriptor for this table, we can free the memory buffers as well */

                  if (RegRecPtr->HeadOfAccessList == CFE_TBL_END_OF_LIST)

                  if (!CFE_TBL_HandleLinkIsAttached(&RegRecPtr->AccessList))

Check warning

Code scanning / CodeQL-coding-standard

Side effect in a Boolean expression

This Boolean expression is not side-effect free.

modules/tbl/fsw/src/cfe_tbl_transaction.c

Comment on lines +236 to +237

		if (AccDescPtr->UsedFlag && AccDescPtr->RegIndex == CFE_TBL_TxnRegId(Txn) &&
		CFE_RESOURCEID_TEST_EQUAL(AccDescPtr->AppId, CFE_TBL_TxnAppId(Txn)))

Check warning

Code scanning / CodeQL-coding-standard

Side effect in a Boolean expression

This Boolean expression is not side-effect free.

Contributor

skliper commented Apr 5, 2024

Any chance the linked list support could be an exposed API? I know CF has a linked list and nice if any other app wouldn't have to roll their own.

jphickey force-pushed the fix-2546-foreach-accessdesc branch from 465925d to a7ea4a3 Compare

April 5, 2024 17:35

Contributor Author

jphickey commented Apr 5, 2024

Any chance the linked list support could be an exposed API? I know CF has a linked list and nice if any other app wouldn't have to roll their own.

Good suggestion. Although this does isolate the prev/next references into a linkage struct, the linkage here is still via Table Handle. So in other words - not a totally generic list function.

I think CFE should have a module for general purpose data structs, like trees and lists....


          Fix nasa#2546, add handle list operation routines

b13c79f

Adds a "HandleLink" structure to encapsulate the linked list
functionality and add generic routines to initialize and test/check
this structure, along with list insert/remove functions for
access descriptors that use this link struct interally.

This also cleans up documentation of the table resource ID functions
defined in cfe_tbl_resource.h

jphickey force-pushed the fix-2546-foreach-accessdesc branch from a7ea4a3 to b13c79f Compare

April 5, 2024 19:15

github-advanced-security bot found potential problems

View reviewed changes

modules/tbl/fsw/src/cfe_tbl_internal.h

+               * \param AccDescPtr Pointer to the current access descriptor
+               * \param Arg Opaque argument from caller (passed through)
+               */
+              typedef void (* const CFE_TBL_AccessDescFunc_t)(CFE_TBL_AccessDescriptor_t *AccDescPtr, void *Arg);

Check notice

Code scanning / CodeQL-coding-standard

Hidden pointer indirection

The typedef CFE_TBL_AccessDescFunc_t hides pointer indirection.

jphickey added the CCB:Ready label

dzbaker added CCB:Approved and removed CCB:Ready labels

dzbaker added a commit to nasa/cFS that referenced this pull request


          IC: equuleus-rc1+dev11

425875c

*Combines:*

cFE equuleus-rc1+dev131
to_lab equuleus-rc1+dev52

**Includes:**

*cFE*
- nasa/cFE#2548

*to_lab*
- nasa/to_lab#188

Co-authored by: Avi Weiss <[email protected]>
Co-authored by: Joseph Hickey <[email protected]>

This was referenced Apr 16, 2024

cFE Integration candidate: Equuleus-rc1+dev11 #2549

Merged

IC: equuleus-rc1+dev11 nasa/cFS#757

Merged

dzbaker merged commit 012d62a into nasa:main

dzbaker added a commit to nasa/cFS that referenced this pull request


          IC: equuleus-rc1+dev11

ac4da60

*Combines:*

cFE equuleus-rc1+dev131
to_lab equuleus-rc1+dev52

**Includes:**

*cFE*
- nasa/cFE#2548

*to_lab*
- nasa/to_lab#188

Co-authored by: Avi Weiss <[email protected]>
Co-authored by: Joseph Hickey <[email protected]>

jphickey deleted the fix-2546-foreach-accessdesc branch

May 29, 2024 14:20

dzbaker added this to the v7.0.0 milestone

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

Labels