Skip to content

Commit

Permalink
Fix nasa#1073, refactor SB API for proper global locks
Browse files Browse the repository at this point in the history
Significant refactor of many SB API calls to address inconsistencies
with respect to locking and unlocking of global data structures.

First this updates the definition of CFE_SB_PipeId_t to use the
CFE_ES_ResourceID_t base type, and a new ID range.  Notably this
prevents direct access to the CFE_SB.PipeTbl global, forcing
code to go through the proper lookup routine, which should only
be done while locked.

All API implementations follow the same general pattern:

- Initial checks/queries while unlocked
- Lock SB global
- Lookups and/or modifications to the pipe table/routing info
- Unlock SB global
- Invoke other subsystems (e.g. OSAL)
- Re-lock SB global (if needed) do final update, and unlock again
- Send all events

All error counters should be updated at the end, while still locked.
All event processing is deferred to the end of each function, after
all other processing is done.
  • Loading branch information
jphickey committed Jan 15, 2021
1 parent 6b5f77a commit b17cd1e
Show file tree
Hide file tree
Showing 13 changed files with 1,852 additions and 1,240 deletions.
36 changes: 35 additions & 1 deletion fsw/cfe-core/src/inc/cfe_sb.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
#include "cfe_mission_cfg.h"
#include "ccsds.h"
#include "cfe_time.h"
#include "cfe_resourceid.h"


/*
Expand Down Expand Up @@ -122,6 +123,13 @@
#define CFE_CLR(i,x) ((i) &= ~CFE_BIT(x)) /**< \brief Clears bit x of i */
#define CFE_TST(i,x) (((i) & CFE_BIT(x)) != 0)/**< \brief true(non zero) if bit x of i is set */

/**
* \brief A CFE_SB_PipeId_t value which is always invalid
*
* This may be used as a safe initializer for CFE_SB_PipeId_t values
*/
#define CFE_SB_INVALID_PIPE CFE_ES_RESOURCEID_UNDEFINED

/*
** Pipe option bit fields.
*/
Expand Down Expand Up @@ -156,7 +164,7 @@ typedef CFE_MSG_TelemetryHeader_t CFE_SB_TlmHdr_t;
**
** Software Bus pipe identifier used in many SB APIs
*/
typedef uint8 CFE_SB_PipeId_t;
typedef CFE_ES_ResourceID_t CFE_SB_PipeId_t;

#ifndef CFE_OMIT_DEPRECATED_6_8
/** \brief Pointer to an SB Message */
Expand Down Expand Up @@ -256,6 +264,32 @@ CFE_Status_t CFE_SB_CreatePipe(CFE_SB_PipeId_t *PipeIdPtr, uint16 Depth, const
**/
CFE_Status_t CFE_SB_DeletePipe(CFE_SB_PipeId_t PipeId);

/**
* @brief Obtain an index value correlating to an SB Pipe ID
*
* This calculates a zero based integer value that may be used for indexing
* into a local resource table/array.
*
* Index values are only guaranteed to be unique for resources of the same
* type. For instance, the indices corresponding to two [valid] application
* IDs will never overlap, but the index of a pipe ID and an app ID
* may be the same. Furthermore, indices may be reused if a resource is
* deleted and re-created.
*
* @note There is no inverse of this function - indices cannot be converted
* back to the original PipeID value. The caller should retain the original ID
* for future use.
*
* @param[in] PipeID Pipe ID to convert
* @param[out] Idx Buffer where the calculated index will be stored
*
* @return Execution status, see @ref CFEReturnCodes
* @retval #CFE_SUCCESS @copybrief CFE_SUCCESS
* @retval #CFE_ES_ERR_RESOURCEID_NOT_VALID @copybrief CFE_ES_ERR_RESOURCEID_NOT_VALID
*/
CFE_Status_t CFE_SB_PipeId_ToIndex(CFE_SB_PipeId_t PipeID, uint32 *Idx);


/*****************************************************************************/
/**
** \brief Set options on a pipe.
Expand Down
4 changes: 2 additions & 2 deletions fsw/cfe-core/src/inc/cfe_sb_msg.h
Original file line number Diff line number Diff line change
Expand Up @@ -604,14 +604,14 @@ typedef struct CFE_SB_PipeDepthStats {

CFE_SB_PipeId_t PipeId;/**< \cfetlmmnemonic \SB_PDPIPEID
\brief Pipe Id associated with the stats below */
uint8 Spare;/**< \cfetlmmnemonic \SB_PDSPARE
\brief Spare byte to ensure alignment */
uint16 Depth;/**< \cfetlmmnemonic \SB_PDDEPTH
\brief Number of messages the pipe can hold */
uint16 InUse;/**< \cfetlmmnemonic \SB_PDINUSE
\brief Number of messages currently on the pipe */
uint16 PeakInUse;/**< \cfetlmmnemonic \SB_PDPKINUSE
\brief Peak number of messages that have been on the pipe */
uint16 Spare;/**< \cfetlmmnemonic \SB_PDSPARE
\brief Spare word to ensure alignment */

}CFE_SB_PipeDepthStats_t;

Expand Down
5 changes: 5 additions & 0 deletions fsw/cfe-core/src/inc/private/cfe_resourceid_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,17 @@
* @defgroup CFEResourceIDBase Resource ID base values
* @{
*/

/* ES managed resources */
#define CFE_ES_APPID_BASE (CFE_ES_RESOURCEID_MARK | ((OS_OBJECT_TYPE_USER+1) << CFE_ES_RESOURCEID_SHIFT))
#define CFE_ES_LIBID_BASE (CFE_ES_RESOURCEID_MARK | ((OS_OBJECT_TYPE_USER+2) << CFE_ES_RESOURCEID_SHIFT))
#define CFE_ES_COUNTID_BASE (CFE_ES_RESOURCEID_MARK | ((OS_OBJECT_TYPE_USER+3) << CFE_ES_RESOURCEID_SHIFT))
#define CFE_ES_POOLID_BASE (CFE_ES_RESOURCEID_MARK | ((OS_OBJECT_TYPE_USER+4) << CFE_ES_RESOURCEID_SHIFT))
#define CFE_ES_CDSBLOCKID_BASE (CFE_ES_RESOURCEID_MARK | ((OS_OBJECT_TYPE_USER+5) << CFE_ES_RESOURCEID_SHIFT))

/* SB managed resources */
#define CFE_SB_PIPEID_BASE (CFE_ES_RESOURCEID_MARK | ((OS_OBJECT_TYPE_USER+6) << CFE_ES_RESOURCEID_SHIFT))

/** @} */


Expand Down
Loading

0 comments on commit b17cd1e

Please sign in to comment.