Skip to content

Commit

Permalink
Fix nasa#2550, Use resource ID for dump control index
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@jphickey
jphickey committed Apr 19, 2024
1 parent 4dc37e2 commit c27f114
Show file tree
Hide file tree
Showing 10 changed files with 496 additions and 185 deletions.
2 changes: 2 additions & 0 deletions modules/resourceid/fsw/inc/cfe_core_resourceid_basevalues.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,

};

Expand All @@ -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),

};

Expand Down
48 changes: 23 additions & 25 deletions modules/tbl/fsw/src/cfe_tbl_api.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
17 changes: 9 additions & 8 deletions modules/tbl/fsw/src/cfe_tbl_internal.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
}

Expand Down
66 changes: 66 additions & 0 deletions modules/tbl/fsw/src/cfe_tbl_resource.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit c27f114

Please sign in to comment.