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

Typo, copy/paste, comment clean-up #846

Closed
skliper opened this issue Mar 10, 2021 · 0 comments · Fixed by #918 or #927
Closed

Typo, copy/paste, comment clean-up #846

skliper opened this issue Mar 10, 2021 · 0 comments · Fixed by #918 or #927
Assignees
Milestone

Comments

@skliper
Copy link
Contributor

skliper commented Mar 10, 2021

Is your feature request related to a problem? Please describe.
General issue to capture typo, copy/paste cleanup from OSAL code review.

Extra space:

* Removes a directory from the structure.

Add comment to justify numbers: common pattern to represent 8 obtjects in a byte (single bit)

uint8 object_ids[(OS_MAX_NUM_OPEN_FILES + 7) / 8];

Clarify in comments if OS_TaskDelay is a "busy" wait or scheduled (sleep):

/*-------------------------------------------------------------------------------------*/
/**
* @brief Delay a task for specified amount of milliseconds
*
* Causes the current thread to be suspended from execution for the period of millisecond.
*
* @param[in] millisecond Amount of time to delay
*
* @return Execution status, see @ref OSReturnCodes
* @retval #OS_SUCCESS @copybrief OS_SUCCESS
* @retval #OS_ERROR if sleep fails or millisecond = 0
*/
int32 OS_TaskDelay(uint32 millisecond);

Document Input parameter as actually in/out:

/*----------------------------------------------------------------
* Function: OS_FdSet_ConvertOut_Impl
*
* Purpose: Local helper routine, not part of OSAL API.
*
* Convert a POSIX fd_set structure into an OSAL OS_FdSet
* which can then be returned back to the application.
*
* This actually un-sets any bits in the "Input" parameter
* which are also set in the "output" parameter.
*-----------------------------------------------------------------*/
static void OS_FdSet_ConvertOut_Impl(fd_set *output, OS_FdSet *Input)

Make capitalization consistent output -> Output:

static void OS_FdSet_ConvertOut_Impl(fd_set *output, OS_FdSet *Input)

Clarify comment - Explicitly zero for consistency in operations and to avoid confusion:

/* eliminates a false warning about possibly uninitialized use */

Clarify comment - Test for existence and is a directory

/* it exists, but not necessarily a directory */
if (stat(local_path, &st) == 0 && S_ISDIR(st.st_mode))
{
return_code = OS_SUCCESS;
}

Duplicate function comments:

/*---------------------------------------------------------------------------------------
Name: OS_CleanUpObject
Purpose: Implements a single API call that can delete ANY object
Will dispatch to the correct delete implementation for that object type
Returns: None
---------------------------------------------------------------------------------------*/
/*----------------------------------------------------------------
*
* Function: OS_CleanUpObject
*
* Purpose: Local helper routine, not part of OSAL API.
*
*-----------------------------------------------------------------*/

check_mode -> lock_mode:

/*
* The "ConvertToken" routine will return with the global lock
* in a state appropriate for returning to the caller, as indicated
* by the "check_mode" paramter.

Comment cleanup, vsnprintf also does the format, Call vsnprintf to format and determine the actual size of the string to write:

/*
* Call vsnprintf() to determine the actual size of the
* string we are going to write to the buffer after formatting.
*/
va_start(va, String);
actualsz = vsnprintf(msg_buffer, sizeof(msg_buffer), String, va);

data_size -> max_size:

int32 OS_QueueCreate(osal_id_t *queue_id, const char *queue_name, osal_blockcount_t queue_depth, size_t data_size,

Clarify constants in comments:

Set->object_ids[local_id >> 3] |= 1 << (local_id & 0x7);

Set->object_ids[local_id >> 3] &= ~(1 << (local_id & 0x7));

return ((Set->object_ids[local_id >> 3] >> (local_id & 0x7)) & 0x1);

Justify constant return - Return OK since called from taskSpawn (where it's not easily accessible), error is reported in debug message:

Clarify - locally scoped statically allocated buffer:

* could be replaced with a statically-allocated OSAL stack buffer.

Justify constants (+500/1000) rounds:

OS_SharedGlobalVars.MicroSecPerTick = (OS_ClockAccuracyNsec + 500) / 1000;

Describe the solution you'd like
Fix

Describe alternatives you've considered
None

Additional context
No impacts

Requester Info
Jacob Hageman - NASA/GSFC, OSAL code review

@skliper skliper added this to the 6.0.0 milestone Mar 10, 2021
@skliper skliper changed the title Typo and copy/paste clean-up Typo, copy/paste, comment clean-up Mar 10, 2021
@skliper skliper self-assigned this Mar 18, 2021
astrogeco added a commit that referenced this issue Mar 24, 2021
Fix #846, Minor clean up and clarification in comments/naming
jphickey pushed a commit to jphickey/osal that referenced this issue Aug 10, 2022
CFE_ES_LIB_ALREADY_LOADED and CFE_ES_ERR_SYS_LOG_TRUNCATED
jphickey pushed a commit to jphickey/osal that referenced this issue Aug 10, 2022
Fix nasa#846, Deconflict CFE_ES_LIB_ALREADY_LOADED and CFE_ES_ERR_SYS_LOG_TRUNCATED
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant