Skip to content

Commit

Permalink
Fix #1440, Split up BinSemGetInfo() to avoid partial success returns
Browse files Browse the repository at this point in the history
  • Loading branch information
thnkslprpt committed Oct 6, 2024
1 parent 8270166 commit 74064dc
Show file tree
Hide file tree
Showing 14 changed files with 237 additions and 59 deletions.
42 changes: 36 additions & 6 deletions src/os/inc/osapi-binsem.h
Original file line number Diff line number Diff line change
Expand Up @@ -180,22 +180,52 @@ int32 OS_BinSemGetIdByName(osal_id_t *sem_id, const char *sem_name);

/*-------------------------------------------------------------------------------------*/
/**
* @brief Fill a property object buffer with details regarding the resource
* @brief Get the name of the binary semaphore
*
* This function will pass back a pointer to structure that contains
* all of the relevant info( name and creator) about the specified 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
* @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_BinSemGetInfo(osal_id_t sem_id, OS_bin_sem_prop_t *bin_prop);
int32 OS_BinSemGetValue(osal_id_t sem_id, OS_bin_sem_prop_t *bin_prop);

/**@}*/

Expand Down
2 changes: 1 addition & 1 deletion src/os/posix/src/os-impl-binsem.c
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ int32 OS_BinSemTimedWait_Impl(const OS_object_token_t *token, uint32 msecs)
* See prototype for argument/return detail
*
*-----------------------------------------------------------------*/
int32 OS_BinSemGetInfo_Impl(const OS_object_token_t *token, OS_bin_sem_prop_t *sem_prop)
int32 OS_BinSemGetValue_Impl(const OS_object_token_t *token, OS_bin_sem_prop_t *sem_prop)
{
OS_impl_binsem_internal_record_t *sem;

Expand Down
2 changes: 1 addition & 1 deletion src/os/rtems/src/os-impl-binsem.c
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ int32 OS_BinSemTimedWait_Impl(const OS_object_token_t *token, uint32 msecs)
* See prototype for argument/return detail
*
*-----------------------------------------------------------------*/
int32 OS_BinSemGetInfo_Impl(const OS_object_token_t *token, OS_bin_sem_prop_t *bin_prop)
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;
Expand Down
2 changes: 1 addition & 1 deletion src/os/shared/inc/os-shared-binsem.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,6 @@ int32 OS_BinSemDelete_Impl(const OS_object_token_t *token);
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);
int32 OS_BinSemGetValue_Impl(const OS_object_token_t *token, OS_bin_sem_prop_t *bin_prop);

#endif /* OS_SHARED_BINSEM_H */
62 changes: 60 additions & 2 deletions src/os/shared/src/osapi-binsem.c
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ int32 OS_BinSemGetIdByName(osal_id_t *sem_id, const char *sem_name)
* See description in API and header file for detail
*
*-----------------------------------------------------------------*/
int32 OS_BinSemGetInfo(osal_id_t sem_id, OS_bin_sem_prop_t *bin_prop)
int32 OS_BinSemGetName(osal_id_t sem_id, OS_bin_sem_prop_t *bin_prop)

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
{
OS_common_record_t *record;
OS_object_token_t token;
Expand All @@ -267,8 +267,66 @@ int32 OS_BinSemGetInfo(osal_id_t sem_id, OS_bin_sem_prop_t *bin_prop)
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)

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
{
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)

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
{
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
4 changes: 2 additions & 2 deletions src/os/vxworks/src/os-impl-binsem.c
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,8 @@ int32 OS_BinSemTimedWait_Impl(const OS_object_token_t *token, uint32 msecs)
* See prototype for argument/return detail
*
*-----------------------------------------------------------------*/
int32 OS_BinSemGetInfo_Impl(const OS_object_token_t *token, OS_bin_sem_prop_t *bin_prop)
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_SUCCESS;
return OS_ERR_NOT_IMPLEMENTED;
}
30 changes: 17 additions & 13 deletions src/tests/bin-sem-test/bin-sem-test.c
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,15 @@ 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);
Expand All @@ -93,7 +95,9 @@ void Test_BinSem(void)
/* 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);
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 +113,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 +145,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 +166,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
66 changes: 58 additions & 8 deletions src/unit-test-coverage/shared/src/coveragetest-binsem.c
Original file line number Diff line number Diff line change
Expand Up @@ -147,11 +147,11 @@ void Test_OS_BinSemGetIdByName(void)
OSAPI_TEST_FUNCTION_RC(OS_BinSemGetIdByName(&objid, NULL), OS_INVALID_POINTER);
}

void Test_OS_BinSemGetInfo(void)
void Test_OS_BinSemGetName(void)
{
/*
* Test Case For:
* int32 OS_BinSemGetInfo (uint32 sem_id, OS_bin_sem_prop_t *bin_prop)
* int32 OS_BinSemGetName (osal_id_t sem_id, OS_bin_sem_prop_t *bin_prop)
*/
int32 expected = OS_SUCCESS;
int32 actual = ~OS_SUCCESS;
Expand All @@ -161,16 +161,64 @@ void Test_OS_BinSemGetInfo(void)

OS_UT_SetupBasicInfoTest(OS_OBJECT_TYPE_OS_BINSEM, UT_INDEX_1, "ABC", UT_OBJID_OTHER);

actual = OS_BinSemGetInfo(UT_OBJID_1, &prop);
actual = OS_BinSemGetName(UT_OBJID_1, &prop);

UtAssert_True(actual == expected, "OS_BinSemGetInfo() (%ld) == OS_SUCCESS", (long)actual);
OSAPI_TEST_OBJID(prop.creator, ==, UT_OBJID_OTHER);
UtAssert_True(actual == expected, "OS_BinSemGetName() (%ld) == OS_SUCCESS", (long)actual);
UtAssert_True(strcmp(prop.name, "ABC") == 0, "prop.name (%s) == ABC", prop.name);

OSAPI_TEST_FUNCTION_RC(OS_BinSemGetInfo(UT_OBJID_1, NULL), OS_INVALID_POINTER);
OSAPI_TEST_FUNCTION_RC(OS_BinSemGetName(UT_OBJID_1, NULL), OS_INVALID_POINTER);

UT_SetDefaultReturnValue(UT_KEY(OS_ObjectIdGetById), OS_ERR_INVALID_ID);
OSAPI_TEST_FUNCTION_RC(OS_BinSemGetName(UT_OBJID_1, &prop), OS_ERR_INVALID_ID);
}

void Test_OS_BinSemGetCreator(void)
{
/*
* Test Case For:
* int32 OS_BinSemGetCreator (osal_id_t sem_id, OS_bin_sem_prop_t *bin_prop)
*/
int32 expected = OS_SUCCESS;
int32 actual = ~OS_SUCCESS;
OS_bin_sem_prop_t prop;

memset(&prop, 0, sizeof(prop));

OS_UT_SetupBasicInfoTest(OS_OBJECT_TYPE_OS_BINSEM, UT_INDEX_1, "ABC", UT_OBJID_OTHER);

actual = OS_BinSemGetCreator(UT_OBJID_1, &prop);

UtAssert_True(actual == expected, "OS_BinSemGetCreator() (%ld) == OS_SUCCESS", (long)actual);
OSAPI_TEST_OBJID(prop.creator, ==, UT_OBJID_OTHER);

OSAPI_TEST_FUNCTION_RC(OS_BinSemGetCreator(UT_OBJID_1, NULL), OS_INVALID_POINTER);

UT_SetDefaultReturnValue(UT_KEY(OS_ObjectIdGetById), OS_ERR_INVALID_ID);
OSAPI_TEST_FUNCTION_RC(OS_BinSemGetCreator(UT_OBJID_1, &prop), OS_ERR_INVALID_ID);
}

void Test_OS_BinSemGetValue(void)
{
/*
* Test Case For:
* int32 OS_BinSemGetValue (osal_id_t sem_id, OS_bin_sem_prop_t *bin_prop)
*/
int32 expected = OS_SUCCESS;
int32 actual = ~OS_SUCCESS;
OS_bin_sem_prop_t prop;

memset(&prop, 0, sizeof(prop));

OS_UT_SetupBasicInfoTest(OS_OBJECT_TYPE_OS_BINSEM, UT_INDEX_1, "ABC", UT_OBJID_OTHER);

actual = OS_BinSemGetValue(UT_OBJID_1, &prop);

UtAssert_True(actual == expected, "OS_BinSemGetValue() (%ld) == OS_SUCCESS", (long)actual);

OSAPI_TEST_FUNCTION_RC(OS_BinSemGetValue(UT_OBJID_1, NULL), OS_INVALID_POINTER);

UT_SetDefaultReturnValue(UT_KEY(OS_ObjectIdGetById), OS_ERR_INVALID_ID);
OSAPI_TEST_FUNCTION_RC(OS_BinSemGetInfo(UT_OBJID_1, &prop), OS_ERR_INVALID_ID);
OSAPI_TEST_FUNCTION_RC(OS_BinSemGetValue(UT_OBJID_1, &prop), OS_ERR_INVALID_ID);
}

/* Osapi_Test_Setup
Expand Down Expand Up @@ -204,5 +252,7 @@ void UtTest_Setup(void)
ADD_TEST(OS_BinSemFlush);
ADD_TEST(OS_BinSemTimedWait);
ADD_TEST(OS_BinSemGetIdByName);
ADD_TEST(OS_BinSemGetInfo);
ADD_TEST(OS_BinSemGetName);
ADD_TEST(OS_BinSemGetCreator);
ADD_TEST(OS_BinSemGetValue);
}
Original file line number Diff line number Diff line change
Expand Up @@ -77,19 +77,19 @@ int32 OS_BinSemFlush_Impl(const OS_object_token_t *token)

/*
* ----------------------------------------------------
* Generated stub function for OS_BinSemGetInfo_Impl()
* Generated stub function for OS_BinSemGetValue_Impl()
* ----------------------------------------------------
*/
int32 OS_BinSemGetInfo_Impl(const OS_object_token_t *token, OS_bin_sem_prop_t *bin_prop)
int32 OS_BinSemGetValue_Impl(const OS_object_token_t *token, OS_bin_sem_prop_t *bin_prop)
{
UT_GenStub_SetupReturnBuffer(OS_BinSemGetInfo_Impl, int32);
UT_GenStub_SetupReturnBuffer(OS_BinSemGetValue_Impl, int32);

UT_GenStub_AddParam(OS_BinSemGetInfo_Impl, const OS_object_token_t *, token);
UT_GenStub_AddParam(OS_BinSemGetInfo_Impl, OS_bin_sem_prop_t *, bin_prop);
UT_GenStub_AddParam(OS_BinSemGetValue_Impl, const OS_object_token_t *, token);
UT_GenStub_AddParam(OS_BinSemGetValue_Impl, OS_bin_sem_prop_t *, bin_prop);

UT_GenStub_Execute(OS_BinSemGetInfo_Impl, Basic, NULL);
UT_GenStub_Execute(OS_BinSemGetValue_Impl, Basic, NULL);

return UT_GenStub_GetReturnValue(OS_BinSemGetInfo_Impl, int32);
return UT_GenStub_GetReturnValue(OS_BinSemGetValue_Impl, int32);
}

/*
Expand Down
Loading

0 comments on commit 74064dc

Please sign in to comment.