Fix #2550, Use resource ID for dump control index

Use a resourceID value for access into the dump control
structure array.  This allows for consistent lookup, matching,
and free/in-use determination, as well as improved resilience
to race conditions and stale data.

modules/resourceid/fsw/inc/cfe_core_resourceid_basevalues.h

@@ -73,6 +73,7 @@ enum
 
  /* TBL managed resources */
  CFE_RESOURCEID_TBL_VALRESULTID_BASE_OFFSET = OS_OBJECT_TYPE_USER + 8,
+ CFE_RESOURCEID_TBL_DUMPCTRLID_BASE_OFFSET = OS_OBJECT_TYPE_USER + 9,
 
 };
 
@@ -99,6 +100,7 @@ enum
 
  /* TBL managed resources */
  CFE_TBL_VALRESULTID_BASE = CFE_RESOURCEID_MAKE_BASE(CFE_RESOURCEID_TBL_VALRESULTID_BASE_OFFSET),
+ CFE_TBL_DUMPCTRLID_BASE = CFE_RESOURCEID_MAKE_BASE(CFE_RESOURCEID_TBL_DUMPCTRLID_BASE_OFFSET),
 
 };
 

modules/tbl/fsw/src/cfe_tbl_api.c

@@ -1096,44 +1096,42 @@ CFE_Status_t CFE_TBL_DumpToBuffer(CFE_TBL_Handle_t TblHandle)
  int32 Status;
  CFE_TBL_RegistryRec_t *RegRecPtr = NULL;
  CFE_TBL_DumpControl_t *DumpCtrlPtr = NULL;
- CFE_ES_AppId_t   ThisAppId;
+ CFE_TBL_LoadBuff_t * ActiveBufPtr;
 
  Status = CFE_TBL_TxnStartFromHandle(&Txn, TblHandle, CFE_TBL_TxnContext_ACCESSOR_APP);
 
  if (Status == CFE_SUCCESS)
  {
  Status = CFE_TBL_TxnGetTableStatus(&Txn);
 
- RegRecPtr = CFE_TBL_TxnRegRec(&Txn);
-
- CFE_TBL_TxnFinish(&Txn);
- }
- else
- {
- ThisAppId = CFE_TBL_TxnAppId(&Txn);
-
- CFE_ES_WriteToSysLog("%s: App(%lu) does not have access to Tbl Handle=%d\n", __func__,
- CFE_RESOURCEID_TO_ULONG(ThisAppId), (int)TblHandle);
- }
+ /* Make sure the table has been requested to be dumped */
+ if (Status == CFE_TBL_INFO_DUMP_PENDING)
+ {
+ RegRecPtr = CFE_TBL_TxnRegRec(&Txn);
+ DumpCtrlPtr = CFE_TBL_LocateDumpCtrlByID(RegRecPtr->DumpControlId);
+ ActiveBufPtr = CFE_TBL_GetActiveBuffer(RegRecPtr);
 
- /* Make sure the table has been requested to be dumped */
- if (Status == CFE_TBL_INFO_DUMP_PENDING)
- {
- DumpCtrlPtr = &CFE_TBL_Global.DumpControlBlocks[RegRecPtr->DumpControlIndex];
+ /* Copy the contents of the active buffer to the assigned dump buffer */
+ memcpy(DumpCtrlPtr->DumpBufferPtr->BufferPtr, ActiveBufPtr->BufferPtr, DumpCtrlPtr->Size);
 
- /* Copy the contents of the active buffer to the assigned dump buffer */
- memcpy(DumpCtrlPtr->DumpBufferPtr->BufferPtr, RegRecPtr->Buffers[0].BufferPtr, DumpCtrlPtr->Size);
+  /* Save the current time so that the header in the dump file can have the correct time */
+  DumpCtrlPtr->DumpBufferPtr->FileTime = CFE_TIME_GetTime();
 
- /* Save the current time so that the header in the dump file can have the correct time */
- DumpCtrlPtr->DumpBufferPtr->FileTime = CFE_TIME_GetTime();
+  /* Disassociate the dump request from the table */
+  RegRecPtr->DumpControlId = CFE_TBL_NO_DUMP_PENDING;
 
- /* Disassociate the dump request from the table */
- RegRecPtr->DumpControlIndex = CFE_TBL_NO_DUMP_PENDING;
+  /* Notify the Table Services Application that the dump buffer is ready to be written to a file */
+  DumpCtrlPtr->State = CFE_TBL_DUMP_PERFORMED;
 
- /* Notify the Table Services Application that the dump buffer is ready to be written to a file */
- DumpCtrlPtr->State = CFE_TBL_DUMP_PERFORMED;
+  Status = CFE_SUCCESS;
+ }
 
- Status = CFE_SUCCESS;
+ CFE_TBL_TxnFinish(&Txn);
+ }
+ else
+ {
+ CFE_ES_WriteToSysLog("%s: App(%lu) does not have access to Tbl Handle=%lu\n", __func__,
+ CFE_TBL_TxnAppIdAsULong(&Txn), CFE_TBL_TxnHandleAsULong(&Txn));
  }
 
  return Status;

modules/tbl/fsw/src/cfe_tbl_internal.c

@@ -92,8 +92,6 @@ int32 CFE_TBL_EarlyInit(void)
  /* Initialize the Dump-Only Table Dump Control Blocks nonzero values */
  for (i = 0; i < CFE_PLATFORM_TBL_MAX_SIMULTANEOUS_LOADS; i++)
  {
- CFE_TBL_Global.DumpControlBlocks[i].State = CFE_TBL_DUMP_FREE;
-
  /* Prevent Shared Buffers from being used until successfully allocated */
  CFE_TBL_Global.LoadBuffs[i].Taken = true;
  }
@@ -223,7 +221,7 @@ void CFE_TBL_InitRegistryRecord(CFE_TBL_RegistryRec_t *RegRecPtr)
  RegRecPtr->ValidateActiveId = CFE_TBL_NO_VALIDATION_PENDING;
  RegRecPtr->ValidateInactiveId = CFE_TBL_NO_VALIDATION_PENDING;
  RegRecPtr->CDSHandle = CFE_ES_CDS_BAD_HANDLE;
- RegRecPtr->DumpControlIndex = CFE_TBL_NO_DUMP_PENDING;
+ RegRecPtr->DumpControlId  = CFE_TBL_NO_DUMP_PENDING;
 
  CFE_TBL_HandleLinkInit(&RegRecPtr->AccessList);
 }
@@ -1021,19 +1019,22 @@ void CFE_TBL_ByteSwapUint32(uint32 *Uint32ToSwapPtr)
  *-----------------------------------------------------------------*/
 int32 CFE_TBL_CleanUpApp(CFE_ES_AppId_t AppId)
 {
- uint32 i;
- CFE_TBL_TxnState_t Txn;
+ uint32 i;
+ CFE_TBL_TxnState_t Txn;
+ CFE_TBL_DumpControl_t *DumpCtrlPtr;
 
  /* Scan Dump Requests to determine if any of the tables that */
  /* were to be dumped will be deleted */
  for (i = 0; i < CFE_PLATFORM_TBL_MAX_SIMULTANEOUS_LOADS; i++)
  {
+ DumpCtrlPtr = &CFE_TBL_Global.DumpControlBlocks[i];
+
  /* Check to see if the table to be dumped is owned by the App to be deleted */
- if ((CFE_TBL_Global.DumpControlBlocks[i].State != CFE_TBL_DUMP_FREE) &&
- CFE_RESOURCEID_TEST_EQUAL(CFE_TBL_Global.DumpControlBlocks[i].RegRecPtr->OwnerAppId, AppId))
+ if (CFE_TBL_DumpCtrlBlockIsUsed(DumpCtrlPtr) &&
+ CFE_RESOURCEID_TEST_EQUAL(DumpCtrlPtr->RegRecPtr->OwnerAppId, AppId))
  {
  /* If so, then remove the dump request */
- CFE_TBL_Global.DumpControlBlocks[i].State = CFE_TBL_DUMP_FREE;
+ CFE_TBL_DumpCtrlBlockSetFree(DumpCtrlPtr);
  }
  }
 

modules/tbl/fsw/src/cfe_tbl_resource.c

@@ -52,6 +52,18 @@ CFE_Status_t CFE_TBL_Handle_ToIndex(CFE_TBL_Handle_t TblHandle, uint32 *Idx)
  return CFE_SUCCESS;
 }
 
+/*----------------------------------------------------------------
+ *
+ * Implemented per public API
+ * See description in header file for argument/return detail
+ *
+ *-----------------------------------------------------------------*/
+CFE_Status_t CFE_TBL_DumpCtrlId_ToIndex(CFE_TBL_DumpCtrlId_t DumpCtrlId, uint32 *Idx)
+{
+ return CFE_ResourceId_ToIndex(CFE_RESOURCEID_UNWRAP(DumpCtrlId), CFE_TBL_DUMPCTRLID_BASE,
+ CFE_PLATFORM_TBL_MAX_SIMULTANEOUS_LOADS, Idx);
+}
+
 /*----------------------------------------------------------------
  *
  * Implemented per public API
@@ -152,6 +164,60 @@ CFE_TBL_Handle_t CFE_TBL_AccessDescriptorGetHandle(const CFE_TBL_AccessDescripto
  return (AccessDescPtr - CFE_TBL_Global.Handles);
 }
 
+/*----------------------------------------------------------------
+ *
+ * Application-scope internal function
+ * See description in header file for argument/return detail
+ *
+ *-----------------------------------------------------------------*/
+CFE_TBL_DumpControl_t *CFE_TBL_LocateDumpCtrlByID(CFE_TBL_DumpCtrlId_t BlockId)
+{
+ CFE_TBL_DumpControl_t *BlockPtr;
+ uint32 Idx;
+
+ if (CFE_TBL_DumpCtrlId_ToIndex(BlockId, &Idx) == CFE_SUCCESS)
+ {
+ BlockPtr = &CFE_TBL_Global.DumpControlBlocks[Idx];
+ }
+ else
+ {
+ BlockPtr = NULL;
+ }
+
+ return BlockPtr;
+}
+
+/*----------------------------------------------------------------
+ *
+ * Application-scope internal function
+ * See description in header file for argument/return detail
+ *
+ *-----------------------------------------------------------------*/
+bool CFE_TBL_CheckDumpCtrlSlotUsed(CFE_ResourceId_t CheckId)
+{
+ CFE_TBL_DumpControl_t *BlockPtr;
+
+ /*
+ * Note - The pointer here should never be NULL because the ID should always be
+ * within the expected range, but if it ever is NULL, this should return true
+ * such that the caller will _not_ attempt to use the record.
+ */
+ BlockPtr = CFE_TBL_LocateDumpCtrlByID(CFE_TBL_DUMPCTRLID_C(CheckId));
+ return (BlockPtr == NULL || CFE_TBL_DumpCtrlBlockIsUsed(BlockPtr));
+}
+
+/*----------------------------------------------------------------
+ *
+ * Application-scope internal function
+ * See description in header file for argument/return detail
+ *
+ *-----------------------------------------------------------------*/
+CFE_ResourceId_t CFE_TBL_GetNextDumpCtrlBlock(void)
+{
+ return CFE_ResourceId_FindNext(CFE_TBL_Global.LastDumpCtrlBlockId, CFE_PLATFORM_TBL_MAX_SIMULTANEOUS_LOADS,
+ CFE_TBL_CheckDumpCtrlSlotUsed);
+}
+
 /*----------------------------------------------------------------
  *
  * Application-scope internal function