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

Fix #1440, Split up BinSemGetInfo() to avoid partial success returns #1480

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions src/os/inc/osapi-binsem.h
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,8 @@ int32 OS_BinSemDelete(osal_id_t sem_id);
*/
int32 OS_BinSemGetIdByName(osal_id_t *sem_id, const char *sem_name);

#ifdef OSAL_OMIT_DEPRECATED
#else
/*-------------------------------------------------------------------------------------*/
/**
* @brief Fill a property object buffer with details regarding the resource
Expand All @@ -196,6 +198,56 @@ int32 OS_BinSemGetIdByName(osal_id_t *sem_id, const char *sem_name);
* @retval #OS_ERR_NOT_IMPLEMENTED @copybrief OS_ERR_NOT_IMPLEMENTED
*/
int32 OS_BinSemGetInfo(osal_id_t sem_id, OS_bin_sem_prop_t *bin_prop);
#endif

/*-------------------------------------------------------------------------------------*/
/**
* @brief Get the name of the binary semaphore
*
* This function retrieves the name of the specified binary semaphore.
*
* @param[in] sem_id The object ID to operate on
* @param[out] bin_prop The property object buffer to fill @nonnull
*
* @return Execution status, see @ref OSReturnCodes
* @retval #OS_SUCCESS @copybrief OS_SUCCESS
* @retval #OS_ERR_INVALID_ID if the id passed in is not a valid semaphore
* @retval #OS_INVALID_POINTER if the bin_prop pointer is null
*/
int32 OS_BinSemGetName(osal_id_t sem_id, OS_bin_sem_prop_t *bin_prop);

/*-------------------------------------------------------------------------------------*/
/**
* @brief Get the creator of the binary semaphore
*
* This function retrieves the creator of the specified binary semaphore.
*
* @param[in] sem_id The object ID to operate on
* @param[out] bin_prop The property object buffer to fill @nonnull
*
* @return Execution status, see @ref OSReturnCodes
* @retval #OS_SUCCESS @copybrief OS_SUCCESS
* @retval #OS_ERR_INVALID_ID if the id passed in is not a valid semaphore
* @retval #OS_INVALID_POINTER if the bin_prop pointer is null
*/
int32 OS_BinSemGetCreator(osal_id_t sem_id, OS_bin_sem_prop_t *bin_prop);

/*-------------------------------------------------------------------------------------*/
/**
* @brief Get the value of the binary semaphore
*
* This function retrieves the value of the specified binary semaphore.
*
* @param[in] sem_id The object ID to operate on
* @param[out] bin_prop The property object buffer to fill @nonnull
*
* @return Execution status, see @ref OSReturnCodes
* @retval #OS_SUCCESS @copybrief OS_SUCCESS
* @retval #OS_ERR_INVALID_ID if the id passed in is not a valid semaphore
* @retval #OS_INVALID_POINTER if the bin_prop pointer is null
* @retval #OS_ERR_NOT_IMPLEMENTED @copybrief OS_ERR_NOT_IMPLEMENTED
*/
int32 OS_BinSemGetValue(osal_id_t sem_id, OS_bin_sem_prop_t *bin_prop);

/**@}*/

Expand Down
20 changes: 20 additions & 0 deletions src/os/posix/src/os-impl-binsem.c
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,8 @@
return (OS_GenericBinSemTake_Impl(token, &ts));
}

#ifdef OSAL_OMIT_DEPRECATED

Check notice

Code scanning / CodeQL

Conditional compilation Note

Use of conditional compilation must be kept to a minimum.
#else
/*----------------------------------------------------------------
*
* Purpose: Implemented per internal OSAL API
Expand All @@ -482,3 +484,21 @@
sem_prop->value = sem->current_value;
return OS_SUCCESS;
}
#endif

/*----------------------------------------------------------------
*
* Purpose: Implemented per internal OSAL API
* See prototype for argument/return detail
*
*-----------------------------------------------------------------*/
int32 OS_BinSemGetValue_Impl(const OS_object_token_t *token, OS_bin_sem_prop_t *sem_prop)
{
OS_impl_binsem_internal_record_t *sem;

sem = OS_OBJECT_TABLE_GET(OS_impl_bin_sem_table, *token);

/* put the info into the structure */
sem_prop->value = sem->current_value;
return OS_SUCCESS;
}
15 changes: 15 additions & 0 deletions src/os/rtems/src/os-impl-binsem.c
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,8 @@ int32 OS_BinSemTimedWait_Impl(const OS_object_token_t *token, uint32 msecs)
return OS_SUCCESS;
}

#ifdef OSAL_OMIT_DEPRECATED
#else
/*----------------------------------------------------------------
*
* Purpose: Implemented per internal OSAL API
Expand All @@ -277,3 +279,16 @@ int32 OS_BinSemGetInfo_Impl(const OS_object_token_t *token, OS_bin_sem_prop_t *b
/* RTEMS has no API for obtaining the current value of a semaphore */
return OS_ERR_NOT_IMPLEMENTED;
}
#endif

/*----------------------------------------------------------------
*
* Purpose: Implemented per internal OSAL API
* See prototype for argument/return detail
*
*-----------------------------------------------------------------*/
int32 OS_BinSemGetValue_Impl(const OS_object_token_t *token, OS_bin_sem_prop_t *bin_prop)
{
/* RTEMS has no API for obtaining the current value of a semaphore */
return OS_ERR_NOT_IMPLEMENTED;
}
12 changes: 12 additions & 0 deletions src/os/shared/inc/os-shared-binsem.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,24 @@ int32 OS_BinSemTimedWait_Impl(const OS_object_token_t *token, uint32 msecs);
------------------------------------------------------------------*/
int32 OS_BinSemDelete_Impl(const OS_object_token_t *token);


#ifdef OSAL_OMIT_DEPRECATED
#else
/*----------------------------------------------------------------

Purpose: Obtain OS-specific information about the semaphore

Returns: OS_SUCCESS on success, or relevant error code
------------------------------------------------------------------*/
int32 OS_BinSemGetInfo_Impl(const OS_object_token_t *token, OS_bin_sem_prop_t *bin_prop);
#endif

/*----------------------------------------------------------------

Purpose: Obtain the value of a semaphore (if possible/implemented in the relevent OS)

Returns: OS_SUCCESS on success, or relevant error code
------------------------------------------------------------------*/
int32 OS_BinSemGetValue_Impl(const OS_object_token_t *token, OS_bin_sem_prop_t *bin_prop);

#endif /* OS_SHARED_BINSEM_H */
92 changes: 91 additions & 1 deletion src/os/shared/src/osapi-binsem.c
Original file line number Diff line number Diff line change
Expand Up @@ -243,13 +243,45 @@
return return_code;
}

#ifdef OSAL_OMIT_DEPRECATED

Check notice

Code scanning / CodeQL

Conditional compilation Note

Use of conditional compilation must be kept to a minimum.
#else
/*----------------------------------------------------------------
*
* Purpose: Implemented per public OSAL API
* See description in API and header file for detail
*
*-----------------------------------------------------------------*/
int32 OS_BinSemGetInfo(osal_id_t sem_id, OS_bin_sem_prop_t *bin_prop)
{
OS_common_record_t *record;
OS_object_token_t token;
int32 return_code;
/* Check parameters */
OS_CHECK_POINTER(bin_prop);
memset(bin_prop, 0, sizeof(OS_bin_sem_prop_t));
/* Check Parameters */
return_code = OS_ObjectIdGetById(OS_LOCK_MODE_GLOBAL, LOCAL_OBJID_TYPE, sem_id, &token);
if (return_code == OS_SUCCESS)
{
record = OS_OBJECT_TABLE_GET(OS_global_bin_sem_table, token);

strncpy(bin_prop->name, record->name_entry, sizeof(bin_prop->name) - 1);
bin_prop->creator = record->creator;
return_code = OS_BinSemGetInfo_Impl(&token, bin_prop);

OS_ObjectIdRelease(&token);
}
return return_code;
}
#endif

/*----------------------------------------------------------------
*
* Purpose: Implemented per public OSAL API
* See description in API and header file for detail
*
*-----------------------------------------------------------------*/
int32 OS_BinSemGetName(osal_id_t sem_id, OS_bin_sem_prop_t *bin_prop)
{
OS_common_record_t *record;
OS_object_token_t token;
Expand All @@ -267,8 +299,66 @@
record = OS_OBJECT_TABLE_GET(OS_global_bin_sem_table, token);

strncpy(bin_prop->name, record->name_entry, sizeof(bin_prop->name) - 1);
bin_prop->name[sizeof(bin_prop->name) - 1] = '\0'; /* Ensure null termination */

OS_ObjectIdRelease(&token);
}

return return_code;
}

/*----------------------------------------------------------------
*
* Purpose: Implemented per public OSAL API
* See description in API and header file for detail
*
*-----------------------------------------------------------------*/
int32 OS_BinSemGetCreator(osal_id_t sem_id, OS_bin_sem_prop_t *bin_prop)
{
OS_common_record_t *record;
OS_object_token_t token;
int32 return_code;

/* Check parameters */
OS_CHECK_POINTER(bin_prop);

memset(bin_prop, 0, sizeof(OS_bin_sem_prop_t));

/* Check Parameters */
return_code = OS_ObjectIdGetById(OS_LOCK_MODE_GLOBAL, LOCAL_OBJID_TYPE, sem_id, &token);
if (return_code == OS_SUCCESS)
{
record = OS_OBJECT_TABLE_GET(OS_global_bin_sem_table, token);

bin_prop->creator = record->creator;
return_code = OS_BinSemGetInfo_Impl(&token, bin_prop);

OS_ObjectIdRelease(&token);
}

return return_code;
}

/*----------------------------------------------------------------
*
* Purpose: Implemented per public OSAL API
* See description in API and header file for detail
*
*-----------------------------------------------------------------*/
int32 OS_BinSemGetValue(osal_id_t sem_id, OS_bin_sem_prop_t *bin_prop)
{
OS_object_token_t token;
int32 return_code;

/* Check parameters */
OS_CHECK_POINTER(bin_prop);

memset(bin_prop, 0, sizeof(OS_bin_sem_prop_t));

/* Check Parameters */
return_code = OS_ObjectIdGetById(OS_LOCK_MODE_GLOBAL, LOCAL_OBJID_TYPE, sem_id, &token);
if (return_code == OS_SUCCESS)
{
return_code = OS_BinSemGetValue_Impl(&token, bin_prop);

OS_ObjectIdRelease(&token);
}
Expand Down
17 changes: 16 additions & 1 deletion src/os/vxworks/src/os-impl-binsem.c
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,8 @@ int32 OS_BinSemTimedWait_Impl(const OS_object_token_t *token, uint32 msecs)
return status;
}

#ifdef OSAL_OMIT_DEPRECATED
#else
/*----------------------------------------------------------------
*
* Purpose: Implemented per internal OSAL API
Expand All @@ -193,5 +195,18 @@ int32 OS_BinSemTimedWait_Impl(const OS_object_token_t *token, uint32 msecs)
int32 OS_BinSemGetInfo_Impl(const OS_object_token_t *token, OS_bin_sem_prop_t *bin_prop)
{
/* VxWorks has no API for obtaining the current value of a semaphore */
return OS_SUCCESS;
return OS_ERR_NOT_IMPLEMENTED;
}
#endif

/*----------------------------------------------------------------
*
* Purpose: Implemented per internal OSAL API
* See prototype for argument/return detail
*
*-----------------------------------------------------------------*/
int32 OS_BinSemGetValue_Impl(const OS_object_token_t *token, OS_bin_sem_prop_t *bin_prop)
{
/* VxWorks has no API for obtaining the current value of a semaphore */
return OS_ERR_NOT_IMPLEMENTED;
}
37 changes: 23 additions & 14 deletions src/tests/bin-sem-test/bin-sem-test.c
Original file line number Diff line number Diff line change
Expand Up @@ -77,23 +77,32 @@ void Test_BinSem(void)
char long_name[OS_MAX_API_NAME + 1];
OS_bin_sem_prop_t sem_prop;
uint32 test_val;
bool get_info_implemented;
bool get_value_implemented;

memset(&sem_prop, 0, sizeof(sem_prop));
memset(task_counter, 0, sizeof(task_counter));

/* Invalid id checks */
UtAssert_INT32_EQ(OS_BinSemGetInfo(OS_OBJECT_ID_UNDEFINED, &sem_prop), OS_ERR_INVALID_ID);
UtAssert_INT32_EQ(OS_BinSemGetName(OS_OBJECT_ID_UNDEFINED, &sem_prop), OS_ERR_INVALID_ID);
UtAssert_INT32_EQ(OS_BinSemGetCreator(OS_OBJECT_ID_UNDEFINED, &sem_prop), OS_ERR_INVALID_ID);
UtAssert_INT32_EQ(OS_BinSemGetValue(OS_OBJECT_ID_UNDEFINED, &sem_prop), OS_ERR_INVALID_ID);
UtAssert_INT32_EQ(OS_BinSemFlush(OS_OBJECT_ID_UNDEFINED), OS_ERR_INVALID_ID);
UtAssert_INT32_EQ(OS_BinSemGive(OS_OBJECT_ID_UNDEFINED), OS_ERR_INVALID_ID);
UtAssert_INT32_EQ(OS_BinSemTake(OS_OBJECT_ID_UNDEFINED), OS_ERR_INVALID_ID);
UtAssert_INT32_EQ(OS_BinSemTimedWait(OS_OBJECT_ID_UNDEFINED, 0), OS_ERR_INVALID_ID);
UtAssert_INT32_EQ(OS_BinSemDelete(OS_OBJECT_ID_UNDEFINED), OS_ERR_INVALID_ID);

/* Null checks */
/* OS_BinSemCreate NULL checks */
UtAssert_INT32_EQ(OS_BinSemCreate(NULL, "Test_Sem", 0, 0), OS_INVALID_POINTER);
UtAssert_INT32_EQ(OS_BinSemCreate(&sem_id[0], NULL, 0, 0), OS_INVALID_POINTER);
UtAssert_INT32_EQ(OS_BinSemGetInfo(sem_id[0], NULL), OS_INVALID_POINTER);

// Initialize sem_id[0] to a valid value for the following NULL checks
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests were not passing on 1/4 builds without this addition (Debug, Ubuntu 22.04).
Somehow, it behaves differently with the sem_id[0] not being set to a valid value on that target on the preceding line. With the added code though, all builds are fine.

sem_id[0] = OS_OBJECT_ID_UNDEFINED;

// OS_BinSemGet* NULL checks
UtAssert_INT32_EQ(OS_BinSemGetName(sem_id[0], NULL), OS_INVALID_POINTER);
UtAssert_INT32_EQ(OS_BinSemGetCreator(sem_id[0], NULL), OS_INVALID_POINTER);
UtAssert_INT32_EQ(OS_BinSemGetValue(sem_id[0], NULL), OS_INVALID_POINTER);
UtAssert_INT32_EQ(OS_BinSemGetIdByName(NULL, "Test_Sem"), OS_INVALID_POINTER);
UtAssert_INT32_EQ(OS_BinSemGetIdByName(&sem_id[0], NULL), OS_INVALID_POINTER);

Expand All @@ -109,15 +118,15 @@ void Test_BinSem(void)
/* Nonzero create */
UtAssert_INT32_EQ(OS_BinSemCreate(&sem_id[1], "Test_Sem_Nonzero", 1, 0), OS_SUCCESS);

/* Check get info implementation */
get_info_implemented = (OS_BinSemGetInfo(sem_id[0], &sem_prop) != OS_ERR_NOT_IMPLEMENTED);
/* Check get value implementation */
get_value_implemented = (OS_BinSemGetValue(sem_id[0], &sem_prop) != OS_ERR_NOT_IMPLEMENTED);

/* Validate values */
if (get_info_implemented)
if (get_value_implemented)
{
UtAssert_INT32_EQ(OS_BinSemGetInfo(sem_id[0], &sem_prop), OS_SUCCESS);
UtAssert_INT32_EQ(OS_BinSemGetValue(sem_id[0], &sem_prop), OS_SUCCESS);
UtAssert_INT32_EQ(sem_prop.value, 0);
UtAssert_INT32_EQ(OS_BinSemGetInfo(sem_id[1], &sem_prop), OS_SUCCESS);
UtAssert_INT32_EQ(OS_BinSemGetValue(sem_id[1], &sem_prop), OS_SUCCESS);
UtAssert_INT32_EQ(sem_prop.value, 1);
}

Expand All @@ -141,11 +150,11 @@ void Test_BinSem(void)
UtAssert_INT32_EQ(OS_BinSemTimedWait(sem_id[1], 0), OS_SUCCESS);

/* Validate zeros */
if (get_info_implemented)
if (get_value_implemented)
{
UtAssert_INT32_EQ(OS_BinSemGetInfo(sem_id[0], &sem_prop), OS_SUCCESS);
UtAssert_INT32_EQ(OS_BinSemGetValue(sem_id[0], &sem_prop), OS_SUCCESS);
UtAssert_INT32_EQ(sem_prop.value, 0);
UtAssert_INT32_EQ(OS_BinSemGetInfo(sem_id[1], &sem_prop), OS_SUCCESS);
UtAssert_INT32_EQ(OS_BinSemGetValue(sem_id[1], &sem_prop), OS_SUCCESS);
UtAssert_INT32_EQ(sem_prop.value, 0);
}
else
Expand All @@ -162,9 +171,9 @@ void Test_BinSem(void)
UtAssert_INT32_EQ(OS_TaskDelete(task_id[0]), OS_SUCCESS);
UtAssert_UINT32_EQ(task_counter[0], 0);

if (get_info_implemented)
if (get_value_implemented)
{
UtAssert_INT32_EQ(OS_BinSemGetInfo(sem_id[0], &sem_prop), OS_SUCCESS);
UtAssert_INT32_EQ(OS_BinSemGetValue(sem_id[0], &sem_prop), OS_SUCCESS);
UtAssert_INT32_EQ(sem_prop.value, 0);
}
else
Expand Down
2 changes: 1 addition & 1 deletion src/tests/bin-sem-timeout-test/bin-sem-timeout-test.c
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ void task_1(void)
if (status == OS_SUCCESS)
{
OS_printf("TASK 1: Doing some work: %d\n", (int)counter++);
status = OS_BinSemGetInfo(bin_sem_id, &bin_sem_prop);
status = OS_BinSemGetValue(bin_sem_id, &bin_sem_prop);
if (status == OS_SUCCESS)
{
if (bin_sem_prop.value > 1)
Expand Down
2 changes: 1 addition & 1 deletion src/tests/select-test/select-test.c
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ void BinSemSetup(void)
UtAssert_INT32_EQ(OS_BinSemCreate(&bin_sem_id, "BinSem1", 0, 0), OS_SUCCESS);
UtAssert_True(OS_ObjectIdDefined(bin_sem_id), "bin_sem_id (%lu) != UNDEFINED", OS_ObjectIdToInteger(bin_sem_id));

UtAssert_INT32_EQ(OS_BinSemGetInfo(bin_sem_id, &bin_sem_prop), OS_SUCCESS);
UtAssert_INT32_EQ(OS_BinSemGetValue(bin_sem_id, &bin_sem_prop), OS_SUCCESS);
UtPrintf("BinSem1 value=%d", (int)bin_sem_prop.value);
}

Expand Down
Loading
Loading