From 410447edbed78004d6c6e762bedd9fcc7c8a32a7 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Tue, 23 Mar 2021 15:12:36 -0400 Subject: [PATCH] Fix #1155, clean up zero copy API The separate zero copy handle type is removed. Adds two new simplified functions CFE_SB_AllocateMessageBuffer and CFE_SB_ReleaseMessageBuffer to replace CFE_SB_ZeroCopyGetPtr and CFE_SB_ZeroCopyGetPtr. These new functions do not use a separate Handle. Updates the CFE_SB_TransmitBuffer() API to also remove the handle. --- modules/core_api/fsw/inc/cfe_sb.h | 134 +++--------------- .../core_api/fsw/inc/cfe_sb_api_typedefs.h | 13 -- modules/core_api/ut-stubs/src/ut_sb_stubs.c | 50 ++----- modules/sb/fsw/src/cfe_sb_api.c | 89 ++++-------- modules/sb/fsw/src/cfe_sb_priv.h | 6 +- modules/sb/ut-coverage/sb_UT.c | 124 +++++++--------- modules/sb/ut-coverage/sb_UT.h | 6 +- 7 files changed, 118 insertions(+), 304 deletions(-) diff --git a/modules/core_api/fsw/inc/cfe_sb.h b/modules/core_api/fsw/inc/cfe_sb.h index c673e5329..4f1bb8dab 100644 --- a/modules/core_api/fsw/inc/cfe_sb.h +++ b/modules/core_api/fsw/inc/cfe_sb.h @@ -552,31 +552,26 @@ CFE_Status_t CFE_SB_RcvMsg(CFE_SB_Buffer_t **BufPtr, CFE_SB_PipeId_t PipeId, int ** This routine can be used to get a pointer to one of the software bus' ** internal memory buffers that are used for sending messages. The caller ** can use this memory buffer to build an SB message, then send it using -** the #CFE_SB_TransmitBuffer function. This interface is more complicated -** than the normal #CFE_SB_TransmitMsg interface, but it avoids an extra +** the CFE_SB_TransmitBuffer() function. This interface avoids an extra ** copy of the message from the user's memory buffer to the software bus -** internal buffer. The "zero copy" interface can be used to improve -** performance in high-rate, high-volume software bus traffic. +** internal buffer. ** ** \par Assumptions, External Events, and Notes: -** -# The pointer returned by #CFE_SB_ZeroCopyGetPtr is only good for one -** call to #CFE_SB_TransmitBuffer. -** -# Applications should be written as if #CFE_SB_ZeroCopyGetPtr is -** equivalent to a \c malloc() and #CFE_SB_TransmitBuffer is equivalent to -** a \c free(). +** -# The pointer returned by CFE_SB_AllocateMessageBuffer() is only good for one +** call to CFE_SB_TransmitBuffer(). +** -# Once a buffer has been successfully transmitted (as indicated by a successful +** return from CFE_SB_TransmitBuffer()) the buffer becomes owned by the SB application. +** It will automatically be freed by SB once all recipients have finished reading it. ** -# Applications must not de-reference the message pointer (for reading -** or writing) after the call to #CFE_SB_TransmitBuffer. +** or writing) after the call to CFE_SB_TransmitBuffer(). ** ** \param[in] MsgSize The size of the SB message buffer the caller wants ** (including the SB message header). ** -** \param[out] BufferHandle A handle that must be supplied when sending or releasing -** in zero copy mode. -** ** \return A pointer to a memory buffer that message data can be written to -** for use with #CFE_SB_TransmitBuffer. +** for use with CFE_SB_TransmitBuffer(). **/ -CFE_SB_Buffer_t *CFE_SB_ZeroCopyGetPtr(size_t MsgSize, CFE_SB_ZeroCopyHandle_t *BufferHandle); +CFE_SB_Buffer_t *CFE_SB_AllocateMessageBuffer(size_t MsgSize); /*****************************************************************************/ /** @@ -589,21 +584,18 @@ CFE_SB_Buffer_t *CFE_SB_ZeroCopyGetPtr(size_t MsgSize, CFE_SB_ZeroCopyHandle_t * ** \par Assumptions, External Events, and Notes: ** -# This function is not needed for normal "zero copy" transfers. It ** is needed only for cleanup when an application gets a pointer using -** #CFE_SB_ZeroCopyGetPtr, but (due to some error condition) never uses -** that pointer for a #CFE_SB_TransmitBuffer -** -** \param[in] Ptr2Release A pointer to the SB internal buffer. This must be a -** pointer returned by a call to #CFE_SB_ZeroCopyGetPtr, -** but never used in a call to #CFE_SB_TransmitBuffer. +** CFE_SB_AllocateMessageBuffer(), but (due to some error condition) never +** uses that pointer in a call to CFE_SB_TransmitBuffer(). ** -** \param[in] ZeroCopyHandle This must be the handle supplied with the pointer -** when #CFE_SB_ZeroCopyGetPtr was called. +** \param[in] BufPtr A pointer to the SB internal buffer. This must be a +** pointer returned by a call to CFE_SB_AllocateMessageBuffer(), +** but never used in a call to CFE_SB_TransmitBuffer(). ** ** \return Execution status, see \ref CFEReturnCodes ** \retval #CFE_SUCCESS \copybrief CFE_SUCCESS ** \retval #CFE_SB_BUFFER_INVALID \copybrief CFE_SB_BUFFER_INVALID **/ -CFE_Status_t CFE_SB_ZeroCopyReleasePtr(CFE_SB_Buffer_t *Ptr2Release, CFE_SB_ZeroCopyHandle_t ZeroCopyHandle); +CFE_Status_t CFE_SB_ReleaseMessageBuffer(CFE_SB_Buffer_t *BufPtr); /*****************************************************************************/ /** @@ -612,14 +604,14 @@ CFE_Status_t CFE_SB_ZeroCopyReleasePtr(CFE_SB_Buffer_t *Ptr2Release, CFE_SB_Zero ** \par Description ** This routine sends a message that has been created directly in an ** internal SB message buffer by an application (after a call to -** #CFE_SB_ZeroCopyGetPtr). This interface is more complicated than +** #CFE_SB_AllocateMessageBuffer). This interface is more complicated than ** the normal #CFE_SB_TransmitMsg interface, but it avoids an extra copy of ** the message from the user's memory buffer to the software bus ** internal buffer. The "zero copy" interface can be used to improve ** performance in high-rate, high-volume software bus traffic. ** ** \par Assumptions, External Events, and Notes: -** -# A handle returned by #CFE_SB_ZeroCopyGetPtr is "consumed" by +** -# A handle returned by #CFE_SB_AllocateMessageBuffer is "consumed" by ** a _successful_ call to #CFE_SB_TransmitBuffer. ** -# If this function returns CFE_SUCCESS, this indicates the zero copy handle is ** now owned by software bus, and is no longer owned by the calling application, @@ -627,7 +619,7 @@ CFE_Status_t CFE_SB_ZeroCopyReleasePtr(CFE_SB_Buffer_t *Ptr2Release, CFE_SB_Zero ** -# Howver if this function fails (returns any error status) it does not change ** the state of the buffer at all, meaning the calling application still owns it. ** (a failure means the buffer is left in the same state it was before the call). -** -# Applications should be written as if #CFE_SB_ZeroCopyGetPtr is +** -# Applications should be written as if #CFE_SB_AllocateMessageBuffer is ** equivalent to a \c malloc() and a successful call to #CFE_SB_TransmitBuffer ** is equivalent to a \c free(). ** -# Applications must not de-reference the message pointer (for reading @@ -636,7 +628,6 @@ CFE_Status_t CFE_SB_ZeroCopyReleasePtr(CFE_SB_Buffer_t *Ptr2Release, CFE_SB_Zero ** sequence counter if set to do so. ** ** \param[in] BufPtr A pointer to the buffer to be sent. -** \param[in] ZeroCopyHandle The handle supplied by the #CFE_SB_ZeroCopyGetPtr call ** \param[in] IncrementSequenceCount Boolean to increment the internally tracked ** sequence count and update the message if the ** buffer contains a telemetry message @@ -645,93 +636,8 @@ CFE_Status_t CFE_SB_ZeroCopyReleasePtr(CFE_SB_Buffer_t *Ptr2Release, CFE_SB_Zero ** \retval #CFE_SUCCESS \copybrief CFE_SUCCESS ** \retval #CFE_SB_BAD_ARGUMENT \copybrief CFE_SB_BAD_ARGUMENT ** \retval #CFE_SB_MSG_TOO_BIG \copybrief CFE_SB_MSG_TOO_BIG -** \retval #CFE_SB_BUF_ALOC_ERR \copybrief CFE_SB_BUF_ALOC_ERR **/ -CFE_Status_t CFE_SB_TransmitBuffer(CFE_SB_Buffer_t *BufPtr, CFE_SB_ZeroCopyHandle_t ZeroCopyHandle, - bool IncrementSequenceCount); - -#ifndef CFE_OMIT_DEPRECATED_6_8 -/*****************************************************************************/ -/** -** \brief DEPRECATED: Send an SB message in "zero copy" mode. -** \deprecated use CFE_SB_TransmitBuffer -** -** \par Description -** This routine sends a message that has been created directly in an -** internal SB message buffer by an application (after a call to -** #CFE_SB_ZeroCopyGetPtr). This interface is more complicated than -** the normal #CFE_SB_TransmitMsg interface, but it avoids an extra copy of -** the message from the user's memory buffer to the software bus -** internal buffer. The "zero copy" interface can be used to improve -** performance in high-rate, high-volume software bus traffic. -** -** \par Assumptions, External Events, and Notes: -** -# The pointer returned by #CFE_SB_ZeroCopyGetPtr is only good for -** one call to #CFE_SB_TransmitBuffer. -** -# Callers must not use the same SB message buffer for multiple sends. -** -# Applications should be written as if #CFE_SB_ZeroCopyGetPtr is -** equivalent to a \c malloc() and #CFE_SB_TransmitBuffer is equivalent -** to a \c free(). -** -# Applications must not de-reference the message pointer (for reading -** or writing) after the call to #CFE_SB_TransmitBuffer. -** -# This function tracks and increments the source sequence counter -** of a telemetry message. -** -** \param[in] BufPtr A pointer to the SB buffer to be sent. -** -** \param[in] BufferHandle The handle supplied with the #CFE_SB_ZeroCopyGetPtr call. -** -** \return Execution status, see \ref CFEReturnCodes -** \retval #CFE_SUCCESS \copybrief CFE_SUCCESS -** \retval #CFE_SB_BAD_ARGUMENT \copybrief CFE_SB_BAD_ARGUMENT -** \retval #CFE_SB_MSG_TOO_BIG \copybrief CFE_SB_MSG_TOO_BIG -** \retval #CFE_SB_BUF_ALOC_ERR \copybrief CFE_SB_BUF_ALOC_ERR -** \retval #CFE_SB_BUFFER_INVALID \copybrief CFE_SB_BUFFER_INVALID -**/ -CFE_Status_t CFE_SB_ZeroCopySend(CFE_SB_Buffer_t *BufPtr, CFE_SB_ZeroCopyHandle_t BufferHandle); - -/*****************************************************************************/ -/** -** \brief DEPRECATED: Pass an SB message in "zero copy" mode. -** \deprecated use CFE_SB_TransmitBuffer -** -** \par Description -** This routine sends a message that has been created directly in an -** internal SB message buffer by an application (after a call to -** #CFE_SB_ZeroCopyGetPtr). This interface is more complicated than -** the normal #CFE_SB_TransmitMsg interface, but it avoids an extra copy of -** the message from the user's memory buffer to the software bus -** internal buffer. The "zero copy" interface can be used to improve -** performance in high-rate, high-volume software bus traffic. This -** version is intended to pass messages not generated by the caller -** (to preserve the source sequence count). -** -** \par Assumptions, External Events, and Notes: -** -# The pointer returned by #CFE_SB_ZeroCopyGetPtr is only good for -** one call to #CFE_SB_TransmitBuffer or #CFE_SB_ZeroCopyPass. -** -# Callers must not use the same SB message buffer for multiple sends. -** -# Applications should be written as if #CFE_SB_ZeroCopyGetPtr is -** equivalent to a \c malloc() and #CFE_SB_ZeroCopyPass is equivalent -** to a \c free(). -** -# Applications must not de-reference the message pointer (for reading -** or writing) after the call to #CFE_SB_ZeroCopyPass. -** -# This routine will not modify the sequence counter in a telemetry -** message -** -** \param[in] BufPtr A pointer to the SB buffer to be sent. -** -** \param[in] BufferHandle The handle supplied with the #CFE_SB_ZeroCopyGetPtr call. -** -** \return Execution status, see \ref CFEReturnCodes -** \retval #CFE_SUCCESS \copybrief CFE_SUCCESS -** \retval #CFE_SB_BAD_ARGUMENT \copybrief CFE_SB_BAD_ARGUMENT -** \retval #CFE_SB_MSG_TOO_BIG \copybrief CFE_SB_MSG_TOO_BIG -** \retval #CFE_SB_BUF_ALOC_ERR \copybrief CFE_SB_BUF_ALOC_ERR -** \retval #CFE_SB_BUFFER_INVALID \copybrief CFE_SB_BUFFER_INVALID -**/ -CFE_Status_t CFE_SB_ZeroCopyPass(CFE_SB_Buffer_t *BufPtr, CFE_SB_ZeroCopyHandle_t BufferHandle); -/**@}*/ -#endif +CFE_Status_t CFE_SB_TransmitBuffer(CFE_SB_Buffer_t *BufPtr, bool IncrementSequenceCount); /** @defgroup CFEAPISBSetMessage cFE Setting Message Characteristics APIs * @{ diff --git a/modules/core_api/fsw/inc/cfe_sb_api_typedefs.h b/modules/core_api/fsw/inc/cfe_sb_api_typedefs.h index 3aca7caaf..04b967311 100644 --- a/modules/core_api/fsw/inc/cfe_sb_api_typedefs.h +++ b/modules/core_api/fsw/inc/cfe_sb_api_typedefs.h @@ -162,19 +162,6 @@ typedef CFE_MSG_Message_t *CFE_SB_MsgPtr_t; /** \brief CFE_SB_MsgPayloadPtr_t defined as an opaque pointer to a message Payload portion */ typedef uint8 *CFE_SB_MsgPayloadPtr_t; -#endif /* CFE_OMIT_DEPRECATED_6_8 */ - -/** \brief CFE_SB_ZeroCopyHandle_t to primitive type definition -** -** Software Zero Copy handle used in many SB APIs -*/ -typedef struct -{ - struct CFE_SB_BufferD *BufDscPtr; /* abstract descriptor reference (internal use) */ -} CFE_SB_ZeroCopyHandle_t; - -#ifndef CFE_OMIT_DEPRECATED_6_8 - #define CFE_SB_Default_Qos CFE_SB_DEFAULT_QOS /**< \deprecated use CFE_SB_DEFAULT_QOS */ #define CFE_SB_CMD_HDR_SIZE (sizeof(CFE_MSG_CommandHeader_t)) /**< \brief Size of command header */ diff --git a/modules/core_api/ut-stubs/src/ut_sb_stubs.c b/modules/core_api/ut-stubs/src/ut_sb_stubs.c index 296c18ed6..d9fd7a866 100644 --- a/modules/core_api/ut-stubs/src/ut_sb_stubs.c +++ b/modules/core_api/ut-stubs/src/ut_sb_stubs.c @@ -510,11 +510,9 @@ int32 CFE_SB_TransmitMsg(CFE_MSG_Message_t *MsgPtr, bool IncrementSequenceCount) ** Returns CFE_SUCCESS or overridden unit test value ** ******************************************************************************/ -int32 CFE_SB_TransmitBuffer(CFE_SB_Buffer_t *BufPtr, CFE_SB_ZeroCopyHandle_t ZeroCopyHandle, - bool IncrementSequenceCount) +int32 CFE_SB_TransmitBuffer(CFE_SB_Buffer_t *BufPtr, bool IncrementSequenceCount) { - UT_Stub_RegisterContext(UT_KEY(CFE_SB_TransmitBuffer), BufPtr); - UT_Stub_RegisterContextGenericArg(UT_KEY(CFE_SB_TransmitBuffer), ZeroCopyHandle); + UT_Stub_RegisterContextGenericArg(UT_KEY(CFE_SB_TransmitBuffer), BufPtr); UT_Stub_RegisterContextGenericArg(UT_KEY(CFE_SB_TransmitBuffer), IncrementSequenceCount); int32 status = CFE_SUCCESS; @@ -1159,60 +1157,30 @@ int32 CFE_SB_UnsubscribeLocal(CFE_SB_MsgId_t MsgId, CFE_SB_PipeId_t PipeId) return status; } -CFE_SB_Buffer_t *CFE_SB_ZeroCopyGetPtr(size_t MsgSize, CFE_SB_ZeroCopyHandle_t *BufferHandle) +CFE_SB_Buffer_t *CFE_SB_AllocateMessageBuffer(size_t MsgSize) { - UT_Stub_RegisterContextGenericArg(UT_KEY(CFE_SB_ZeroCopyGetPtr), MsgSize); - UT_Stub_RegisterContext(UT_KEY(CFE_SB_ZeroCopyGetPtr), BufferHandle); + UT_Stub_RegisterContextGenericArg(UT_KEY(CFE_SB_AllocateMessageBuffer), MsgSize); int32 status; CFE_SB_Buffer_t *SBBufPtr = NULL; - status = UT_DEFAULT_IMPL(CFE_SB_ZeroCopyGetPtr); + status = UT_DEFAULT_IMPL(CFE_SB_AllocateMessageBuffer); if (status == CFE_SUCCESS) { - UT_Stub_CopyToLocal(UT_KEY(CFE_SB_ZeroCopyGetPtr), &SBBufPtr, sizeof(SBBufPtr)); + UT_Stub_CopyToLocal(UT_KEY(CFE_SB_AllocateMessageBuffer), &SBBufPtr, sizeof(SBBufPtr)); } return SBBufPtr; } -#ifndef CFE_OMIT_DEPRECATED_6_8 -int32 CFE_SB_ZeroCopyPass(CFE_SB_Buffer_t *BufPtr, CFE_SB_ZeroCopyHandle_t BufferHandle) -{ - UT_Stub_RegisterContext(UT_KEY(CFE_SB_ZeroCopyPass), BufPtr); - UT_Stub_RegisterContextGenericArg(UT_KEY(CFE_SB_ZeroCopyPass), BufferHandle); - - int32 status; - - status = UT_DEFAULT_IMPL(CFE_SB_ZeroCopyPass); - - return status; -} -#endif - -int32 CFE_SB_ZeroCopyReleasePtr(CFE_SB_Buffer_t *Ptr2Release, CFE_SB_ZeroCopyHandle_t BufferHandle) -{ - UT_Stub_RegisterContext(UT_KEY(CFE_SB_ZeroCopyReleasePtr), Ptr2Release); - UT_Stub_RegisterContextGenericArg(UT_KEY(CFE_SB_ZeroCopyReleasePtr), BufferHandle); - - int32 status; - - status = UT_DEFAULT_IMPL(CFE_SB_ZeroCopyReleasePtr); - - return status; -} - -#ifndef CFE_OMIT_DEPRECATED_6_8 -int32 CFE_SB_ZeroCopySend(CFE_SB_Buffer_t *BufPtr, CFE_SB_ZeroCopyHandle_t BufferHandle) +int32 CFE_SB_ReleaseMessageBuffer(CFE_SB_Buffer_t *BufPtr) { - UT_Stub_RegisterContext(UT_KEY(CFE_SB_ZeroCopySend), BufPtr); - UT_Stub_RegisterContextGenericArg(UT_KEY(CFE_SB_ZeroCopySend), BufferHandle); + UT_Stub_RegisterContextGenericArg(UT_KEY(CFE_SB_ReleaseMessageBuffer), BufPtr); int32 status; - status = UT_DEFAULT_IMPL(CFE_SB_ZeroCopySend); + status = UT_DEFAULT_IMPL(CFE_SB_ReleaseMessageBuffer); return status; } -#endif diff --git a/modules/sb/fsw/src/cfe_sb_api.c b/modules/sb/fsw/src/cfe_sb_api.c index f96ba031d..3e0d0717a 100644 --- a/modules/sb/fsw/src/cfe_sb_api.c +++ b/modules/sb/fsw/src/cfe_sb_api.c @@ -2072,9 +2072,9 @@ int32 CFE_SB_ReceiveBuffer(CFE_SB_Buffer_t **BufPtr, CFE_SB_PipeId_t PipeId, int } /* - * Function: CFE_SB_ZeroCopyGetPtr - See API and header file for details + * Function: CFE_SB_AllocateMessageBuffer - See API and header file for details */ -CFE_SB_Buffer_t *CFE_SB_ZeroCopyGetPtr(size_t MsgSize, CFE_SB_ZeroCopyHandle_t *BufferHandle) +CFE_SB_Buffer_t *CFE_SB_AllocateMessageBuffer(size_t MsgSize) { CFE_ES_AppId_t AppId; CFE_SB_BufferD_t *BufDscPtr; @@ -2083,18 +2083,13 @@ CFE_SB_Buffer_t *CFE_SB_ZeroCopyGetPtr(size_t MsgSize, CFE_SB_ZeroCopyHandle_t * AppId = CFE_ES_APPID_UNDEFINED; BufDscPtr = NULL; BufPtr = NULL; + if (MsgSize > CFE_MISSION_SB_MAX_SB_MSG_SIZE) { CFE_ES_WriteToSysLog(" CFE_SB:ZeroCopyGetPtr-Failed, MsgSize is too large\n"); return NULL; } - if (BufferHandle == NULL) - { - CFE_ES_WriteToSysLog(" CFE_SB:ZeroCopyGetPtr-BufferHandle is NULL\n"); - return NULL; - } - /* get callers AppId */ if (CFE_ES_GetAppID(&AppId) == CFE_SUCCESS) { @@ -2128,34 +2123,37 @@ CFE_SB_Buffer_t *CFE_SB_ZeroCopyGetPtr(size_t MsgSize, CFE_SB_ZeroCopyHandle_t * memset(BufPtr, 0, MsgSize); } - /* Export both items (descriptor + msg buffer) to caller */ - BufferHandle->BufDscPtr = BufDscPtr; return BufPtr; -} /* CFE_SB_ZeroCopyGetPtr */ +} /* CFE_SB_AllocateMessageBuffer */ /* - * Helper functions to do sanity checks on the Zero Copy handle + Buffer combo. - * - * Note in a future CFE version the API can be simplified - - * only one of these pointers is strictly needed, since they - * should refer to the same buffer descriptor object. + * Helper function to do sanity checks on the Zero Copy Buffer and + * outputs the encapsulating descriptor if successful */ -int32 CFE_SB_ZeroCopyHandleValidate(CFE_SB_Buffer_t *BufPtr, CFE_SB_ZeroCopyHandle_t ZeroCopyHandle) +int32 CFE_SB_ZeroCopyBufferValidate(CFE_SB_Buffer_t *BufPtr, CFE_SB_BufferD_t **BufDscPtr) { + cpuaddr BufDscAddr; + /* * Sanity Check that the pointers are not NULL */ - if (BufPtr == NULL || ZeroCopyHandle.BufDscPtr == NULL) + if (BufPtr == NULL) { return CFE_SB_BAD_ARGUMENT; } + /* + * Calculate descriptor pointer from buffer pointer - + * The buffer is just a member (offset) in the descriptor + */ + BufDscAddr = (cpuaddr)BufPtr - offsetof(CFE_SB_BufferD_t, Content); + *BufDscPtr = (CFE_SB_BufferD_t *)BufDscAddr; + /* * Check that the descriptor is actually a "zero copy" type, - * and that it refers to same actual message buffer. */ - if (!CFE_RESOURCEID_TEST_DEFINED(ZeroCopyHandle.BufDscPtr->AppId) || (&ZeroCopyHandle.BufDscPtr->Content != BufPtr)) + if (!CFE_RESOURCEID_TEST_DEFINED((*BufDscPtr)->AppId)) { return CFE_SB_BUFFER_INVALID; } @@ -2165,46 +2163,43 @@ int32 CFE_SB_ZeroCopyHandleValidate(CFE_SB_Buffer_t *BufPtr, CFE_SB_ZeroCopyHand } /* - * Function: CFE_SB_ZeroCopyReleasePtr - See API and header file for details + * Function: CFE_SB_ReleaseMessageBuffer - See API and header file for details */ -int32 CFE_SB_ZeroCopyReleasePtr(CFE_SB_Buffer_t *Ptr2Release, CFE_SB_ZeroCopyHandle_t ZeroCopyHandle) +CFE_Status_t CFE_SB_ReleaseMessageBuffer(CFE_SB_Buffer_t *BufPtr) { - int32 Status; + CFE_SB_BufferD_t *BufDscPtr; + int32 Status; - Status = CFE_SB_ZeroCopyHandleValidate(Ptr2Release, ZeroCopyHandle); + Status = CFE_SB_ZeroCopyBufferValidate(BufPtr, &BufDscPtr); CFE_SB_LockSharedData(__func__, __LINE__); if (Status == CFE_SUCCESS) { /* Clear the ownership app ID and decrement use count (may also free) */ - ZeroCopyHandle.BufDscPtr->AppId = CFE_ES_APPID_UNDEFINED; - CFE_SB_DecrBufUseCnt(ZeroCopyHandle.BufDscPtr); + BufDscPtr->AppId = CFE_ES_APPID_UNDEFINED; + CFE_SB_DecrBufUseCnt(BufDscPtr); } CFE_SB_UnlockSharedData(__func__, __LINE__); return Status; -} /* end CFE_SB_ZeroCopyReleasePtr */ +} /* end CFE_SB_ReleaseMessageBuffer */ /* * Function CFE_SB_TransmitBuffer - See API and header file for details */ -int32 CFE_SB_TransmitBuffer(CFE_SB_Buffer_t *BufPtr, CFE_SB_ZeroCopyHandle_t ZeroCopyHandle, - bool IncrementSequenceCount) +int32 CFE_SB_TransmitBuffer(CFE_SB_Buffer_t *BufPtr, bool IncrementSequenceCount) { int32 Status; CFE_SB_BufferD_t *BufDscPtr; CFE_SBR_RouteId_t RouteId; - Status = CFE_SB_ZeroCopyHandleValidate(BufPtr, ZeroCopyHandle); + Status = CFE_SB_ZeroCopyBufferValidate(BufPtr, &BufDscPtr); if (Status == CFE_SUCCESS) { - /* Get actual buffer descriptor pointer from zero copy handle */ - BufDscPtr = ZeroCopyHandle.BufDscPtr; - /* Validate the content and get the MsgId, store it in the descriptor */ Status = CFE_SB_TransmitMsgValidate(&BufPtr->Msg, &BufDscPtr->MsgId, &BufDscPtr->ContentSize, &RouteId); @@ -2244,31 +2239,3 @@ int32 CFE_SB_TransmitBuffer(CFE_SB_Buffer_t *BufPtr, CFE_SB_ZeroCopyHandle_t Zer return Status; } - -#ifndef CFE_OMIT_DEPRECATED_6_8 -/* - * Function: CFE_SB_ZeroCopySend - See API and header file for details - */ -int32 CFE_SB_ZeroCopySend(CFE_SB_Buffer_t *BufPtr, CFE_SB_ZeroCopyHandle_t BufferHandle) -{ - int32 Status = 0; - - Status = CFE_SB_TransmitBuffer(BufPtr, BufferHandle, true); - - return Status; - -} /* end CFE_SB_ZeroCopySend */ - -/* - * Function: CFE_SB_ZeroCopyPass - See API and header file for details - */ -int32 CFE_SB_ZeroCopyPass(CFE_SB_Buffer_t *BufPtr, CFE_SB_ZeroCopyHandle_t BufferHandle) -{ - int32 Status = 0; - - Status = CFE_SB_TransmitBuffer(BufPtr, BufferHandle, false); - - return Status; - -} /* end CFE_SB_ZeroCopyPass */ -#endif diff --git a/modules/sb/fsw/src/cfe_sb_priv.h b/modules/sb/fsw/src/cfe_sb_priv.h index cb596fe25..e1533fd03 100644 --- a/modules/sb/fsw/src/cfe_sb_priv.h +++ b/modules/sb/fsw/src/cfe_sb_priv.h @@ -423,12 +423,12 @@ void CFE_SB_BroadcastBufferToRoute(CFE_SB_BufferD_t *BufDscPtr, CFE_SBR_RouteId_ /** * \brief Perform basic sanity check on the Zero Copy handle * - * \param[in] BufPtr pointer to the content buffer - * \param[in] ZeroCopyHandle Zero copy handle to check + * \param[in] BufPtr pointer to the content buffer + * \param[out] BufDscPtr Will be set to actual buffer descriptor * * \returns CFE_SUCCESS if validation passed, or error code. */ -int32 CFE_SB_ZeroCopyHandleValidate(CFE_SB_Buffer_t *BufPtr, CFE_SB_ZeroCopyHandle_t ZeroCopyHandle); +int32 CFE_SB_ZeroCopyBufferValidate(CFE_SB_Buffer_t *BufPtr, CFE_SB_BufferD_t **BufDscPtr); /** * \brief Add a destination node diff --git a/modules/sb/ut-coverage/sb_UT.c b/modules/sb/ut-coverage/sb_UT.c index 76d530570..85d2ea7cf 100644 --- a/modules/sb/ut-coverage/sb_UT.c +++ b/modules/sb/ut-coverage/sb_UT.c @@ -2720,15 +2720,15 @@ void Test_TransmitMsg_API(void) SB_UT_ADD_SUBTEST(Test_TransmitMsg_PipeFull); SB_UT_ADD_SUBTEST(Test_TransmitMsg_MsgLimitExceeded); SB_UT_ADD_SUBTEST(Test_TransmitMsg_GetPoolBufErr); - SB_UT_ADD_SUBTEST(Test_TransmitMsg_ZeroCopyGetPtr); SB_UT_ADD_SUBTEST(Test_TransmitBuffer_IncrementSeqCnt); SB_UT_ADD_SUBTEST(Test_TransmitBuffer_NoIncrement); - SB_UT_ADD_SUBTEST(Test_TransmitMsg_ZeroCopyHandleValidate); - SB_UT_ADD_SUBTEST(Test_TransmitMsg_ZeroCopyReleasePtr); + SB_UT_ADD_SUBTEST(Test_TransmitMsg_ZeroCopyBufferValidate); SB_UT_ADD_SUBTEST(Test_TransmitMsg_DisabledDestination); SB_UT_ADD_SUBTEST(Test_BroadcastBufferToRoute); SB_UT_ADD_SUBTEST(Test_TransmitMsgValidate_MaxMsgSizePlusOne); SB_UT_ADD_SUBTEST(Test_TransmitMsgValidate_NoSubscribers); + SB_UT_ADD_SUBTEST(Test_AllocateMessageBuffer); + SB_UT_ADD_SUBTEST(Test_ReleaseMessageBuffer); } /* end Test_TransmitMsg_API */ /* @@ -3030,17 +3030,16 @@ void Test_TransmitMsg_GetPoolBufErr(void) ** Test getting a pointer to a buffer for zero copy mode with buffer ** allocation failures */ -void Test_TransmitMsg_ZeroCopyGetPtr(void) +void Test_AllocateMessageBuffer(void) { - CFE_SB_ZeroCopyHandle_t ZeroCpyBufHndl; - uint16 MsgSize = 10; - uint32 MemUse; + uint16 MsgSize = 10; + uint32 MemUse; /* Have GetPoolBuf stub return error on its next call (buf descriptor * allocation failed) */ UT_SetDeferredRetcode(UT_KEY(CFE_ES_GetPoolBuf), 1, CFE_ES_ERR_MEM_BLOCK_SIZE); - ASSERT_TRUE(CFE_SB_ZeroCopyGetPtr(MsgSize, &ZeroCpyBufHndl) == NULL); + ASSERT_TRUE(CFE_SB_AllocateMessageBuffer(MsgSize) == NULL); EVTCNT(0); @@ -3053,7 +3052,7 @@ void Test_TransmitMsg_ZeroCopyGetPtr(void) CFE_SB_Global.StatTlmMsg.Payload.MemInUse = 0; CFE_SB_Global.StatTlmMsg.Payload.PeakMemInUse = MemUse + 10; CFE_SB_Global.StatTlmMsg.Payload.PeakSBBuffersInUse = CFE_SB_Global.StatTlmMsg.Payload.SBBuffersInUse + 2; - ASSERT_TRUE(CFE_SB_ZeroCopyGetPtr(MsgSize, &ZeroCpyBufHndl) != NULL); + ASSERT_TRUE(CFE_SB_AllocateMessageBuffer(MsgSize) != NULL); ASSERT_EQ(CFE_SB_Global.StatTlmMsg.Payload.PeakMemInUse, MemUse + 10); /* unchanged */ ASSERT_EQ(CFE_SB_Global.StatTlmMsg.Payload.MemInUse, MemUse); /* predicted value */ @@ -3064,46 +3063,37 @@ void Test_TransmitMsg_ZeroCopyGetPtr(void) } /* end Test_TransmitMsg_ZeroCopyGetPtr */ -void Test_TransmitMsg_ZeroCopyHandleValidate(void) +void Test_TransmitMsg_ZeroCopyBufferValidate(void) { - CFE_SB_Buffer_t * SendPtr; - CFE_SB_ZeroCopyHandle_t GoodZeroCpyBufHndl; - CFE_SB_ZeroCopyHandle_t BadZeroCpyBufHndl; - CFE_SB_ZeroCopyHandle_t NullZeroCpyBufHndl; - CFE_SB_BufferD_t TestBufDsc; + CFE_SB_Buffer_t * SendPtr; + CFE_SB_BufferD_t BadZeroCpyBuf; + CFE_SB_BufferD_t *BufDscPtr; /* Create a real/valid Zero Copy handle via the API */ - SendPtr = CFE_SB_ZeroCopyGetPtr(sizeof(SB_UT_Test_Tlm_t), &GoodZeroCpyBufHndl); + SendPtr = CFE_SB_AllocateMessageBuffer(sizeof(SB_UT_Test_Tlm_t)); if (SendPtr == NULL) { UtAssert_Failed("Unexpected NULL pointer returned from ZeroCopyGetPtr"); } - /* The NULL handle is just zero */ - NullZeroCpyBufHndl = (CFE_SB_ZeroCopyHandle_t) {0}; - /* Create an invalid Zero Copy handle that is not NULL but refers to a - * descriptor which is NOT from CFE_SB_ZeroCopyGetPtr(). */ - memset(&TestBufDsc, 0, sizeof(TestBufDsc)); - BadZeroCpyBufHndl = (CFE_SB_ZeroCopyHandle_t) {&TestBufDsc}; - - /* Good buffer pointer + Null Handle => BAD_ARGUMENT */ - ASSERT_EQ(CFE_SB_ZeroCopyHandleValidate(SendPtr, NullZeroCpyBufHndl), CFE_SB_BAD_ARGUMENT); - - /* Bad buffer pointer + Good Handle => BAD_ARGUMENT */ - ASSERT_EQ(CFE_SB_ZeroCopyHandleValidate(NULL, GoodZeroCpyBufHndl), CFE_SB_BAD_ARGUMENT); + * descriptor which is NOT from CFE_SB_AllocateMessageBuffer(). */ + memset(&BadZeroCpyBuf, 0, sizeof(BadZeroCpyBuf)); - /* Good buffer pointer + Non Zero-Copy Handle => CFE_SB_BUFFER_INVALID */ - ASSERT_EQ(CFE_SB_ZeroCopyHandleValidate(SendPtr, BadZeroCpyBufHndl), CFE_SB_BUFFER_INVALID); + /* Null Buffer => BAD_ARGUMENT */ + ASSERT_EQ(CFE_SB_ZeroCopyBufferValidate(NULL, &BufDscPtr), CFE_SB_BAD_ARGUMENT); - /* Mismatched buffer pointer + Good Handle => CFE_SB_BUFFER_INVALID */ - ASSERT_EQ(CFE_SB_ZeroCopyHandleValidate(SendPtr + 1, GoodZeroCpyBufHndl), CFE_SB_BUFFER_INVALID); + /* Non-null buffer pointer but Non Zero-Copy => CFE_SB_BUFFER_INVALID */ + ASSERT_EQ(CFE_SB_ZeroCopyBufferValidate(&BadZeroCpyBuf.Content, &BufDscPtr), CFE_SB_BUFFER_INVALID); /* Good buffer pointer + Good Handle => SUCCESS */ - ASSERT_EQ(CFE_SB_ZeroCopyHandleValidate(SendPtr, GoodZeroCpyBufHndl), CFE_SUCCESS); + ASSERT_EQ(CFE_SB_ZeroCopyBufferValidate(SendPtr, &BufDscPtr), CFE_SUCCESS); + + /* Confirm that the computed pointer was correct */ + ASSERT_TRUE(&BufDscPtr->Content == SendPtr); /* Clean-up */ - CFE_SB_ZeroCopyReleasePtr(SendPtr, GoodZeroCpyBufHndl); + CFE_SB_ReleaseMessageBuffer(SendPtr); } /* @@ -3117,7 +3107,6 @@ void Test_TransmitBuffer_IncrementSeqCnt(void) CFE_SB_PipeId_t PipeId; CFE_SB_MsgId_t MsgId = SB_UT_TLM_MID; uint32 PipeDepth = 10; - CFE_SB_ZeroCopyHandle_t ZeroCpyBufHndl; CFE_MSG_SequenceCount_t SeqCnt; CFE_MSG_Size_t Size = sizeof(SB_UT_Test_Tlm_t); CFE_MSG_Type_t Type = CFE_MSG_Type_Tlm; @@ -3130,7 +3119,7 @@ void Test_TransmitBuffer_IncrementSeqCnt(void) SETUP(CFE_SB_Subscribe(MsgId, PipeId)); /* Create a real/valid Zero Copy handle via the API */ - SendPtr = CFE_SB_ZeroCopyGetPtr(sizeof(SB_UT_Test_Tlm_t), &ZeroCpyBufHndl); + SendPtr = CFE_SB_AllocateMessageBuffer(sizeof(SB_UT_Test_Tlm_t)); if (SendPtr == NULL) { UtAssert_Failed("Unexpected NULL pointer returned from ZeroCopyGetPtr"); @@ -3141,7 +3130,7 @@ void Test_TransmitBuffer_IncrementSeqCnt(void) UT_SetDataBuffer(UT_KEY(CFE_MSG_GetType), &Type, sizeof(Type), false); /* Test a successful zero copy send */ - ASSERT(CFE_SB_TransmitBuffer(SendPtr, ZeroCpyBufHndl, true)); + ASSERT(CFE_SB_TransmitBuffer(SendPtr, true)); ASSERT(CFE_SB_ReceiveBuffer(&ReceivePtr, PipeId, CFE_SB_PEND_FOREVER)); @@ -3167,10 +3156,9 @@ void Test_TransmitBuffer_NoIncrement(void) CFE_SB_PipeId_t PipeId; CFE_SB_MsgId_t MsgId = SB_UT_TLM_MID; uint32 PipeDepth = 10; - CFE_SB_ZeroCopyHandle_t ZeroCpyBufHndl; - CFE_MSG_SequenceCount_t SeqCnt = 22; - CFE_MSG_Size_t Size = sizeof(SB_UT_Test_Tlm_t); - CFE_MSG_Type_t Type = CFE_MSG_Type_Tlm; + CFE_MSG_SequenceCount_t SeqCnt = 22; + CFE_MSG_Size_t Size = sizeof(SB_UT_Test_Tlm_t); + CFE_MSG_Type_t Type = CFE_MSG_Type_Tlm; /* Set up hook for checking CFE_MSG_SetSequenceCount calls */ UT_SetHookFunction(UT_KEY(CFE_MSG_SetSequenceCount), UT_CheckSetSequenceCount, &SeqCnt); @@ -3178,7 +3166,7 @@ void Test_TransmitBuffer_NoIncrement(void) SETUP(CFE_SB_CreatePipe(&PipeId, PipeDepth, "ZeroCpyPassTestPipe")); SETUP(CFE_SB_Subscribe(MsgId, PipeId)); - SendPtr = CFE_SB_ZeroCopyGetPtr(sizeof(SB_UT_Test_Tlm_t), &ZeroCpyBufHndl); + SendPtr = CFE_SB_AllocateMessageBuffer(sizeof(SB_UT_Test_Tlm_t)); if (SendPtr == NULL) { @@ -3190,7 +3178,7 @@ void Test_TransmitBuffer_NoIncrement(void) UT_SetDataBuffer(UT_KEY(CFE_MSG_GetType), &Type, sizeof(Type), false); /* Test a successful zero copy pass */ - ASSERT(CFE_SB_TransmitBuffer(SendPtr, ZeroCpyBufHndl, false)); + ASSERT(CFE_SB_TransmitBuffer(SendPtr, false)); ASSERT(CFE_SB_ReceiveBuffer(&ReceivePtr, PipeId, CFE_SB_PEND_FOREVER)); ASSERT_TRUE(SendPtr == ReceivePtr); @@ -3206,35 +3194,34 @@ void Test_TransmitBuffer_NoIncrement(void) /* ** Test releasing a pointer to a buffer for zero copy mode */ -void Test_TransmitMsg_ZeroCopyReleasePtr(void) +void Test_ReleaseMessageBuffer(void) { - CFE_SB_Buffer_t * ZeroCpyMsgPtr1 = NULL; - CFE_SB_Buffer_t * ZeroCpyMsgPtr2 = NULL; - CFE_SB_Buffer_t * ZeroCpyMsgPtr3 = NULL; - CFE_SB_ZeroCopyHandle_t ZeroCpyBufHndl1; - CFE_SB_ZeroCopyHandle_t ZeroCpyBufHndl2; - CFE_SB_ZeroCopyHandle_t ZeroCpyBufHndl3; - uint16 MsgSize = 10; + CFE_SB_Buffer_t *ZeroCpyMsgPtr1 = NULL; + CFE_SB_Buffer_t *ZeroCpyMsgPtr2 = NULL; + CFE_SB_Buffer_t *ZeroCpyMsgPtr3 = NULL; + CFE_SB_BufferD_t BadBufferDesc; + uint16 MsgSize = 10; - ZeroCpyMsgPtr1 = CFE_SB_ZeroCopyGetPtr(MsgSize, &ZeroCpyBufHndl1); - ZeroCpyMsgPtr2 = CFE_SB_ZeroCopyGetPtr(MsgSize, &ZeroCpyBufHndl2); - ZeroCpyMsgPtr3 = CFE_SB_ZeroCopyGetPtr(MsgSize, &ZeroCpyBufHndl3); - SETUP(CFE_SB_ZeroCopyReleasePtr(ZeroCpyMsgPtr2, ZeroCpyBufHndl2)); + ZeroCpyMsgPtr1 = CFE_SB_AllocateMessageBuffer(MsgSize); + ZeroCpyMsgPtr2 = CFE_SB_AllocateMessageBuffer(MsgSize); + ZeroCpyMsgPtr3 = CFE_SB_AllocateMessageBuffer(MsgSize); + SETUP(CFE_SB_ReleaseMessageBuffer(ZeroCpyMsgPtr2)); - /* Test response to an invalid buffer */ - ASSERT_EQ(CFE_SB_ZeroCopyReleasePtr(ZeroCpyMsgPtr2, ZeroCpyBufHndl2), CFE_SB_BUFFER_INVALID); + /* Test response to an invalid buffer (has been released already) */ + ASSERT_EQ(CFE_SB_ReleaseMessageBuffer(ZeroCpyMsgPtr2), CFE_SB_BUFFER_INVALID); /* Test response to a null message pointer */ - ASSERT_EQ(CFE_SB_ZeroCopyReleasePtr(NULL, ZeroCpyBufHndl3), CFE_SB_BAD_ARGUMENT); + ASSERT_EQ(CFE_SB_ReleaseMessageBuffer(NULL), CFE_SB_BAD_ARGUMENT); /* Test response to an invalid message pointer */ - ASSERT_EQ(CFE_SB_ZeroCopyReleasePtr(ZeroCpyMsgPtr1, ZeroCpyBufHndl3), CFE_SB_BUFFER_INVALID); + memset(&BadBufferDesc, 0, sizeof(BadBufferDesc)); + ASSERT_EQ(CFE_SB_ReleaseMessageBuffer(&BadBufferDesc.Content), CFE_SB_BUFFER_INVALID); /* Test successful release of the second buffer */ - ASSERT(CFE_SB_ZeroCopyReleasePtr(ZeroCpyMsgPtr3, ZeroCpyBufHndl3)); + ASSERT(CFE_SB_ReleaseMessageBuffer(ZeroCpyMsgPtr3)); /* Test successful release of the third buffer */ - ASSERT(CFE_SB_ZeroCopyReleasePtr(ZeroCpyMsgPtr1, ZeroCpyBufHndl1)); + ASSERT(CFE_SB_ReleaseMessageBuffer(ZeroCpyMsgPtr1)); EVTCNT(0); @@ -3510,11 +3497,10 @@ void Test_ReceiveBuffer_PendForever(void) */ void Test_CleanupApp_API(void) { - CFE_SB_PipeId_t PipeId; - CFE_SB_ZeroCopyHandle_t ZeroCpyBufHndl; - uint16 PipeDepth = 10; - CFE_ES_AppId_t AppID; - CFE_ES_AppId_t AppID2; + CFE_SB_PipeId_t PipeId; + uint16 PipeDepth = 10; + CFE_ES_AppId_t AppID; + CFE_ES_AppId_t AppID2; /* * Reset global descriptor list @@ -3525,15 +3511,15 @@ void Test_CleanupApp_API(void) AppID2 = CFE_ES_APPID_C(CFE_ResourceId_FromInteger(2)); SETUP(CFE_SB_CreatePipe(&PipeId, PipeDepth, "TestPipe")); - CFE_SB_ZeroCopyGetPtr(10, &ZeroCpyBufHndl); + CFE_SB_AllocateMessageBuffer(10); /* Mimic a different app ID getting a buffer */ UT_SetAppID(AppID2); - CFE_SB_ZeroCopyGetPtr(10, &ZeroCpyBufHndl); + CFE_SB_AllocateMessageBuffer(10); /* Original app gets a second buffer */ UT_SetAppID(AppID); - CFE_SB_ZeroCopyGetPtr(10, &ZeroCpyBufHndl); + CFE_SB_AllocateMessageBuffer(10); /* Set second application ID to provide complete branch path coverage */ CFE_SB_Global.PipeTbl[1].PipeId = SB_UT_PIPEID_1; diff --git a/modules/sb/ut-coverage/sb_UT.h b/modules/sb/ut-coverage/sb_UT.h index f7f08f63a..a61af0004 100644 --- a/modules/sb/ut-coverage/sb_UT.h +++ b/modules/sb/ut-coverage/sb_UT.h @@ -1916,7 +1916,7 @@ void Test_TransmitMsg_GetPoolBufErr(void); ** \returns ** This function does not return a value. ******************************************************************************/ -void Test_TransmitMsg_ZeroCopyGetPtr(void); +void Test_AllocateMessageBuffer(void); /*****************************************************************************/ /** @@ -1948,7 +1948,7 @@ void Test_TransmitBuffer_IncrementSeqCnt(void); ** \returns ** This function does not return a value. ******************************************************************************/ -void Test_TransmitMsg_ZeroCopyHandleValidate(void); +void Test_TransmitMsg_ZeroCopyBufferValidate(void); /*****************************************************************************/ /** @@ -1981,7 +1981,7 @@ void Test_TransmitBuffer_NoIncrement(void); ** \returns ** This function does not return a value. ******************************************************************************/ -void Test_TransmitMsg_ZeroCopyReleasePtr(void); +void Test_ReleaseMessageBuffer(void); /*****************************************************************************/ /**