Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update OS_ObjectIdToArrayIndex documentation related to it being an API, inconsistent documentation in implementation #984

Closed
skliper opened this issue May 5, 2021 · 1 comment · Fixed by #1031
Assignees
Labels
Milestone

Comments

@skliper
Copy link
Contributor

skliper commented May 5, 2021

Is your feature request related to a problem? Please describe.
'OS_ObjectIdToArrayIndex is an external API:

/*-------------------------------------------------------------------------------------*/
/**
* @brief Converts an abstract ID into a number suitable for use as an array index.
*
* This will return a unique zero-based integer number in the range of [0,MAX) for
* any valid object ID. This may be used by application code as an array index
* for indexing into local tables.
*
* This routine operates on a specific object type, and returns a value based on the
* maximum number of objects for that type.
*
* If the idtype is passed as #OS_OBJECT_TYPE_UNDEFINED, then object type verification
* is skipped and any object ID will be accepted and converted to an index. In this
* mode, the range of the output depends on the actual passed-in object type.
*
* If the idtype is passed as any other value, the passed-in ID value is first
* confirmed to be the correct type. This check will guarantee that the output
* is within an expected range; for instance, if the type is passed as
* #OS_OBJECT_TYPE_OS_TASK, then the output index is guaranteed to be between 0 and
* #OS_MAX_TASKS-1 after successful conversion.
*
* @param[in] idtype The object type to convert
* @param[in] object_id The object ID to operate on
* @param[out] *ArrayIndex The Index to return
*
* @return Execution status, see @ref OSReturnCodes
* @retval #OS_SUCCESS @copybrief OS_SUCCESS
* @retval #OS_ERR_INCORRECT_OBJ_TYPE @copybrief OS_ERR_INCORRECT_OBJ_TYPE
* */
int32 OS_ObjectIdToArrayIndex(osal_objtype_t idtype, osal_id_t object_id, osal_index_t *ArrayIndex);

Yet internally there's a few style patterns and references that treat it like it's internal:

/*----------------------------------------------------------------
*
* Function: OS_ObjectIdToArrayIndex
*
* Purpose: Convert an object ID (which must be of the given type) to a number suitable
* for use as an array index. The array index will be in the range of:
* 0 <= ArrayIndex < OS_MAX_<OBJTYPE>
*
* If the passed-in ID type is OS_OBJECT_TYPE_UNDEFINED, then any type
* is allowed.
*
* returns: If the passed-in ID is not of the proper type, OS_ERROR is returned
* Otherwise OS_SUCCESS is returned.
*
*-----------------------------------------------------------------*/

/* just pass to the generic internal conversion routine */

Note the implementation file itself has some inconsistencies:

* NOTE: The only exception is OS_ConvertToArrayIndex() as this is necessary to
* assist applications when storing OSAL IDs in a table.

/*
*********************************************************************************
* IDENTIFIER MAP / UNMAP FUNCTIONS
*********************************************************************************
*/

/*----------------------------------------------------------------
*
* Function: OS_ObjectIdInit
*
* Purpose: Local helper routine, not part of OSAL API.
* clears the entire table and brings it to a proper initial state
*
*-----------------------------------------------------------------*/

/**************************************************************
* LOCAL HELPER FUNCTIONS
* (not used outside of this unit)
**************************************************************/

Describe the solution you'd like
Update comments, make sure headers are in the right place

Describe alternatives you've considered
None

Additional context
None

Requester Info
Jacob Hageman - NASA/GSFC

@skliper skliper added the docs label May 5, 2021
@skliper skliper added this to the 6.0.0 milestone May 5, 2021
@jphickey
Copy link
Contributor

jphickey commented May 5, 2021

If I remember correctly, this originally was an internal routine, but it was deemed useful because it actually confirms the object ID is of the expected/intended object type, unlike OS_ConvertToArrayIndex which will accept any object type. The latter was not removed/deprecated though, since it is an easy drop-in to fix legacy code which used osal ID as an array index directly. But new code should prefer the better error checking provided by OS_ObjectIdToArrayIndex.

skliper added a commit to skliper/osal that referenced this issue May 19, 2021
astrogeco added a commit that referenced this issue May 27, 2021
…comments

Fix #984, Document OS_ObjectIdToArrayIndex as public
@skliper skliper self-assigned this Jun 3, 2021
pepepr08 pushed a commit to pepepr08/osal that referenced this issue Jun 9, 2021
jphickey pushed a commit to jphickey/osal that referenced this issue Aug 10, 2022
…econds

Fix nasa#983 Updated the CFE_MISSION_TIME_DEF_LEAPS to 37 seconds
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants