Skip to content

Commit

Permalink
Fix nasa#797, Update SB subsystem for abstract IDs
Browse files Browse the repository at this point in the history 
Change SB to get its task ID from ES rather than getting
it directly from OSAL.  Update syslog calls to use the
ES-supplied conversion to integer rather than direct cast.
  • Loading branch information
@jphickey
jphickey committed Sep 2, 2020
1 parent dfe8abc commit 4d901e8
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 44 deletions.
57 changes: 32 additions & 25 deletions fsw/cfe-core/src/sb/cfe_sb_api.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ int32 CFE_SB_CreatePipe(CFE_SB_PipeId_t *PipeIdPtr, uint16 Depth, const char *
/* get callers AppId */
CFE_ES_GetAppID(&AppId);

/* get callers TaskId */
CFE_ES_GetTaskID(&TskId);

/* get callers name */
CFE_ES_GetAppName(AppName, AppId, OS_MAX_API_NAME);

Expand All @@ -101,8 +104,6 @@ int32 CFE_SB_CreatePipe(CFE_SB_PipeId_t *PipeIdPtr, uint16 Depth, const char *
/* take semaphore to prevent a task switch during this call */
CFE_SB_LockSharedData(__func__,__LINE__);

TskId = OS_TaskGetId();

/* set user's pipe id value to 'invalid' for error cases below */
if(PipeIdPtr != NULL){
*PipeIdPtr = CFE_SB_INVALID_PIPE;
Expand Down Expand Up @@ -272,12 +273,12 @@ int32 CFE_SB_DeletePipeFull(CFE_SB_PipeId_t PipeId,uint32 AppId)
CFE_SB_DestinationD_t *DestPtr = NULL;
char FullName[(OS_MAX_API_NAME * 2)];

/* get TaskId of caller for events */
CFE_ES_GetTaskID(&TskId);

/* take semaphore to prevent a task switch during this call */
CFE_SB_LockSharedData(__func__,__LINE__);

/* get TaskId of caller for events */
TskId = OS_TaskGetId();

/* check input parameter */
PipeTblIdx = CFE_SB_GetPipeIdx(PipeId);
RtnFromVal = CFE_SB_ValidatePipeId(PipeId);
Expand Down Expand Up @@ -400,12 +401,17 @@ int32 CFE_SB_SetPipeOpts(CFE_SB_PipeId_t PipeId, uint8 Opts)
return Status;
}

/* get TaskId of caller for events */
Status = CFE_ES_GetTaskID(&TskId);
if(Status != CFE_SUCCESS)
{
/* shouldn't happen... */
return Status;
}

/* take semaphore to prevent a task switch during this call */
CFE_SB_LockSharedData(__func__,__LINE__);

/* get TaskId of caller for events */
TskId = OS_TaskGetId();

/* check input parameter */
PipeTblIdx = CFE_SB_GetPipeIdx(PipeId);
RtnFromVal = CFE_SB_ValidatePipeId(PipeId);
Expand Down Expand Up @@ -459,6 +465,9 @@ int32 CFE_SB_GetPipeOpts(CFE_SB_PipeId_t PipeId, uint8 *OptsPtr)
uint32 TskId = 0;
char FullName[(OS_MAX_API_NAME * 2)];

/* get TaskId of caller for events */
CFE_ES_GetTaskID(&TskId);

if(OptsPtr == NULL)
{
CFE_SB.HKTlmMsg.Payload.PipeOptsErrorCounter++;
Expand All @@ -471,9 +480,6 @@ int32 CFE_SB_GetPipeOpts(CFE_SB_PipeId_t PipeId, uint8 *OptsPtr)
/* take semaphore to prevent a task switch during this call */
CFE_SB_LockSharedData(__func__,__LINE__);

/* get TaskId of caller for events */
TskId = OS_TaskGetId();

/* check input parameter */
PipeTblIdx = CFE_SB_GetPipeIdx(PipeId);
RtnFromVal = CFE_SB_ValidatePipeId(PipeId);
Expand Down Expand Up @@ -506,12 +512,14 @@ int32 CFE_SB_GetPipeName(char *PipeNameBuf, size_t PipeNameSize, CFE_SB_PipeId_t
char FullName[(OS_MAX_API_NAME * 2)];

if(PipeNameBuf == NULL || PipeNameSize == 0) {
CFE_ES_GetTaskID(&TskId);
CFE_EVS_SendEventWithAppID(CFE_SB_GETPIPENAME_NULL_PTR_EID, CFE_EVS_EventType_ERROR,
CFE_SB.AppId, "Pipe Name Error:NullPtr,Requestor %s",
CFE_SB_GetAppTskName(TskId,FullName));

Status = CFE_SB_BAD_ARGUMENT;
} else if(PipeId >= CFE_PLATFORM_SB_MAX_PIPES){
CFE_ES_GetTaskID(&TskId);
CFE_EVS_SendEventWithAppID(CFE_SB_GETPIPENAME_ID_ERR_EID, CFE_EVS_EventType_ERROR,
CFE_SB.AppId, "Pipe Id Error:Bad Argument,Id=%d,Requestor %s",
PipeId,CFE_SB_GetAppTskName(TskId,FullName));
Expand Down Expand Up @@ -556,6 +564,9 @@ int32 CFE_SB_GetPipeIdByName(CFE_SB_PipeId_t *PipeIdPtr, const char *PipeName)
uint32 QueueId = 0;
char FullName[(OS_MAX_API_NAME * 2)];

/* get TaskId of caller for events */
CFE_ES_GetTaskID(&TskId);

if(PipeName == NULL || PipeIdPtr == NULL)
{
CFE_SB.HKTlmMsg.Payload.GetPipeIdByNameErrorCounter++;
Expand All @@ -568,9 +579,6 @@ int32 CFE_SB_GetPipeIdByName(CFE_SB_PipeId_t *PipeIdPtr, const char *PipeName)
}
else
{
/* get TaskId of caller for events */
TskId = OS_TaskGetId();

RtnFromVal = OS_QueueGetIdByName(&QueueId, PipeName);

if(RtnFromVal == OS_SUCCESS)
Expand Down Expand Up @@ -726,15 +734,15 @@ int32 CFE_SB_SubscribeFull(CFE_SB_MsgId_t MsgId,

CFE_SB_GetPipeName(PipeName, sizeof(PipeName), PipeId);

/* take semaphore to prevent a task switch during this call */
CFE_SB_LockSharedData(__func__,__LINE__);

/* get task id for events */
TskId = OS_TaskGetId();

/* get the callers Application Id */
CFE_ES_GetAppID(&AppId);

/* get TaskId of caller for events */
CFE_ES_GetTaskID(&TskId);

/* take semaphore to prevent a task switch during this call */
CFE_SB_LockSharedData(__func__,__LINE__);

/* check that the pipe has been created */
PipeIdx = CFE_SB_GetPipeIdx(PipeId);
if(PipeIdx==CFE_SB_INVALID_PIPE){
Expand Down Expand Up @@ -1015,13 +1023,12 @@ int32 CFE_SB_UnsubscribeFull(CFE_SB_MsgId_t MsgId,CFE_SB_PipeId_t PipeId,
CFE_SB_DestinationD_t *DestPtr = NULL;
char FullName[(OS_MAX_API_NAME * 2)];

/* get TaskId of caller for events */
CFE_ES_GetTaskID(&TskId);

/* take semaphore to prevent a task switch during this call */
CFE_SB_LockSharedData(__func__,__LINE__);

/* get task id for events */
TskId = OS_TaskGetId();

/* check that the pipe has been created */
PipeIdx = CFE_SB_GetPipeIdx(PipeId);
if(PipeIdx==CFE_SB_INVALID_PIPE){
Expand Down Expand Up @@ -1190,7 +1197,7 @@ int32 CFE_SB_SendMsgFull(CFE_SB_Msg_t *MsgPtr,
SBSndErr.EvtsToSnd = 0;

/* get task id for events and Sender Info*/
TskId = OS_TaskGetId();
CFE_ES_GetTaskID(&TskId);

/* check input parameter */
if(MsgPtr == NULL){
Expand Down Expand Up @@ -1514,7 +1521,7 @@ int32 CFE_SB_RcvMsg(CFE_SB_MsgPtr_t *BufPtr,
char FullName[(OS_MAX_API_NAME * 2)];

/* get task id for events */
TskId = OS_TaskGetId();
CFE_ES_GetTaskID(&TskId);

/* Check input parameters */
if((BufPtr == NULL)||(TimeOut < (-1))){
Expand Down
28 changes: 19 additions & 9 deletions fsw/cfe-core/src/sb/cfe_sb_priv.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -288,8 +288,8 @@ void CFE_SB_LockSharedData(const char *FuncName, int32 LineNumber){

CFE_ES_GetAppID(&AppId);

CFE_ES_WriteToSysLog("SB SharedData Mutex Take Err Stat=0x%x,App=%d,Func=%s,Line=%d\n",
(unsigned int)Status,(int)AppId,FuncName,(int)LineNumber);
CFE_ES_WriteToSysLog("SB SharedData Mutex Take Err Stat=0x%x,App=%lu,Func=%s,Line=%d\n",
(unsigned int)Status,CFE_ES_ResourceID_ToInteger(AppId),FuncName,(int)LineNumber);

}/* end if */

Expand Down Expand Up @@ -323,8 +323,8 @@ void CFE_SB_UnlockSharedData(const char *FuncName, int32 LineNumber){

CFE_ES_GetAppID(&AppId);

CFE_ES_WriteToSysLog("SB SharedData Mutex Give Err Stat=0x%x,App=%d,Func=%s,Line=%d\n",
(unsigned int)Status,(int)AppId,FuncName,(int)LineNumber);
CFE_ES_WriteToSysLog("SB SharedData Mutex Give Err Stat=0x%x,App=%lu,Func=%s,Line=%d\n",
(unsigned int)Status,CFE_ES_ResourceID_ToInteger(AppId),FuncName,(int)LineNumber);

}/* end if */

Expand Down Expand Up @@ -673,17 +673,22 @@ char *CFE_SB_GetAppTskName(uint32 TaskId,char *FullName){
*/
uint32 CFE_SB_RequestToSendEvent(uint32 TaskId, uint32 Bit){

OS_ConvertToArrayIndex(TaskId, &TaskId);
uint32 Indx;

if (CFE_ES_TaskID_ToIndex(TaskId, &Indx) != CFE_SUCCESS)
{
return CFE_SB_DENIED;
}

/* if bit is set... */
if(CFE_TST(CFE_SB.StopRecurseFlags[TaskId],Bit))
if(CFE_TST(CFE_SB.StopRecurseFlags[Indx],Bit))
{

return CFE_SB_DENIED;

}else{

CFE_SET(CFE_SB.StopRecurseFlags[TaskId],Bit);
CFE_SET(CFE_SB.StopRecurseFlags[Indx],Bit);
return CFE_SB_GRANTED;

}/* end if */
Expand All @@ -705,10 +710,15 @@ uint32 CFE_SB_RequestToSendEvent(uint32 TaskId, uint32 Bit){
*/
void CFE_SB_FinishSendEvent(uint32 TaskId, uint32 Bit){

OS_ConvertToArrayIndex(TaskId, &TaskId);
uint32 Indx;

if (CFE_ES_TaskID_ToIndex(TaskId, &Indx) != CFE_SUCCESS)
{
return;
}

/* clear the bit so the task may send this event again */
CFE_CLR(CFE_SB.StopRecurseFlags[TaskId],Bit);
CFE_CLR(CFE_SB.StopRecurseFlags[Indx],Bit);
}/* end CFE_SB_RequestToSendEvent */


Expand Down
35 changes: 25 additions & 10 deletions fsw/cfe-core/unit-test/sb_UT.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,15 @@ const CFE_SB_MsgId_t SB_UT_ALTERNATE_INVALID_MID = CFE_SB_MSGID_WRAP_VALUE(CFE_P
const CFE_SB_MsgId_t SB_UT_BARE_CMD_MID3 = CFE_SB_MSGID_WRAP_VALUE(0x1003);
const CFE_SB_MsgId_t SB_UT_BARE_TLM_MID3 = CFE_SB_MSGID_WRAP_VALUE(0x0003);

/*
* Helper function to "corrupt" a resource ID value in a consistent/predicatble way,
* which can also be un-done easily.
*/
uint32 UT_SB_ResourceID_Modify(uint32 InitialID, int32 Modifier)
{
uint32 NewValue = InitialID + Modifier;
return (NewValue);
}

/*
** Functions
Expand Down Expand Up @@ -1731,7 +1740,7 @@ void Test_DeletePipe_InvalidPipeOwner(void)
RealOwner = CFE_SB.PipeTbl[PipedId].AppId;

/* Choose a value that is sure not to be owner */
CFE_SB.PipeTbl[PipedId].AppId = RealOwner + 1;
CFE_SB.PipeTbl[PipedId].AppId = UT_SB_ResourceID_Modify(RealOwner, 1);
ASSERT_EQ(CFE_SB_DeletePipe(PipedId), CFE_SB_BAD_ARGUMENT);

EVTCNT(2);
Expand Down Expand Up @@ -2391,15 +2400,15 @@ void Test_Subscribe_InvalidPipeOwner(void)
CFE_SB_PipeId_t PipeId;
CFE_SB_MsgId_t MsgId = SB_UT_TLM_MID;
uint16 PipeDepth = 10;
int32 RealOwner;
uint32 RealOwner;

SETUP(CFE_SB_CreatePipe(&PipeId, PipeDepth, "TestPipe"));

/* Change owner of pipe through memory corruption */
RealOwner = CFE_SB.PipeTbl[PipeId].AppId;

/* Choose a value that is sure not to be owner */
CFE_SB.PipeTbl[PipeId].AppId = RealOwner + 1;
CFE_SB.PipeTbl[PipeId].AppId = UT_SB_ResourceID_Modify(RealOwner, 1);
CFE_SB_Subscribe(MsgId, PipeId);

EVTCNT(3);
Expand Down Expand Up @@ -2583,7 +2592,7 @@ void Test_Unsubscribe_InvalidPipeOwner(void)
RealOwner = CFE_SB.PipeTbl[PipeId].AppId;

/* Choose a value that is sure not be owner */
CFE_SB.PipeTbl[PipeId].AppId = RealOwner + 1;
CFE_SB.PipeTbl[PipeId].AppId = UT_SB_ResourceID_Modify(RealOwner, 1);
ASSERT_EQ(CFE_SB_Unsubscribe(MsgId, PipeId), CFE_SB_BAD_ARGUMENT);

EVTCNT(4);
Expand Down Expand Up @@ -3533,24 +3542,27 @@ void Test_CleanupApp_API(void)
CFE_SB_PipeId_t PipeId;
CFE_SB_ZeroCopyHandle_t ZeroCpyBufHndl = 0;
uint16 PipeDepth = 50;
uint32 AppID;

CFE_ES_GetAppID(&AppID);

SETUP(CFE_SB_CreatePipe(&PipeId, PipeDepth, "TestPipe"));
CFE_SB_ZeroCopyGetPtr(PipeDepth, &ZeroCpyBufHndl);
CFE_SB_ZeroCopyGetPtr(PipeDepth, &ZeroCpyBufHndl);

/* Set second application ID to provide complete branch path coverage */
CFE_SB.PipeTbl[1].InUse = CFE_SB_IN_USE;
CFE_SB.PipeTbl[1].AppId = 1;
CFE_SB.PipeTbl[1].AppId = AppID;

ASSERT_TRUE(CFE_SB.ZeroCopyTail != NULL);

/* Attempt with a bad application ID first in order to get full branch path
* coverage in CFE_SB_ZeroCopyReleaseAppId
*/
CFE_SB_CleanUpApp(1);
CFE_SB_CleanUpApp(0);

/* Attempt again with a valid application ID */
CFE_SB_CleanUpApp(0);
CFE_SB_CleanUpApp(AppID);

ASSERT_TRUE(CFE_SB.ZeroCopyTail == NULL);

Expand Down Expand Up @@ -3901,13 +3913,14 @@ void Test_OS_MutSem_ErrLogic(void)
*/
void Test_ReqToSendEvent_ErrLogic(void)
{
uint32 TaskId = 13;
uint32 TaskId;
uint32 Bit = 5;

/* Clear task bits, then call function, which should set the bit for
* the specified task
*/
CFE_SB.StopRecurseFlags[TaskId] = 0x0000;
CFE_ES_GetTaskID(&TaskId);
CFE_SB.StopRecurseFlags[0] = 0x0000;
ASSERT_EQ(CFE_SB_RequestToSendEvent(TaskId, Bit), CFE_SB_GRANTED);

/* Call the function a second time; the result should indicate that the
Expand Down Expand Up @@ -3992,13 +4005,15 @@ void Test_CFE_SB_BadPipeInfo(void)
CFE_SB_PipeId_t PipeId;
uint16 PipeDepth = 10;
CFE_SB_Qos_t CFE_SB_Default_Qos;
uint32 AppID;

SETUP(CFE_SB_CreatePipe(&PipeId, PipeDepth, "TestPipe1"));

/* Set the pipe ID to an erroneous value and attempt to delete the pipe */
CFE_SB.PipeTbl[0].PipeId = 1;
CFE_SB.PipeTbl[0].InUse = 1;
ASSERT_EQ(CFE_SB_DeletePipeFull(0, 0), CFE_SB_BAD_ARGUMENT);
CFE_ES_GetAppID(&AppID);
ASSERT_EQ(CFE_SB_DeletePipeFull(0, AppID), CFE_SB_BAD_ARGUMENT);

EVTCNT(2);

Expand Down

0 comments on commit 4d901e8

Please sign in to comment.