From e77702be596b1537129b2898aa04ba91df062259 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Thu, 29 Oct 2020 10:24:20 -0400 Subject: [PATCH] Fix #635, add typedefs for osal stack and priority Adds typedefs for: - osal_priority_t - osal_stackptr_t Note that by using `uint8` as the priority type, all values are valid and it is no longer possible to pass a value which is out of range. Checks/tests for this are no longer valid and would cause compiler warnings. --- src/os/inc/osapi-os-core.h | 28 +++++++++---- src/os/shared/inc/os-shared-task.h | 4 +- src/os/shared/src/osapi-task.c | 39 +++++++------------ .../shared/src/coveragetest-task.c | 8 +--- .../oscore-test/ut_oscore_task_test.c | 19 --------- src/ut-stubs/osapi-utstub-task.c | 6 +-- 6 files changed, 41 insertions(+), 63 deletions(-) diff --git a/src/os/inc/osapi-os-core.h b/src/os/inc/osapi-os-core.h index a6fa0d47c..74b1feea6 100644 --- a/src/os/inc/osapi-os-core.h +++ b/src/os/inc/osapi-os-core.h @@ -86,15 +86,29 @@ */ #define OS_ERROR_NAME_LENGTH 35 +/** + * @brief Type to be used for OSAL task priorities. + * + * OSAL priorities are in reverse order, and range + * from 0 (highest; will preempt all other tasks) to + * 255 (lowest; will not preempt any other task). + */ +typedef uint8_t osal_priority_t; + +/** + * @brief Type to be used for OSAL stack pointer. + */ +typedef void* osal_stackptr_t; + /* Object property structures */ /** @brief OSAL task properties */ typedef struct { - char name[OS_MAX_API_NAME]; - osal_id_t creator; - uint32 stack_size; - uint32 priority; + char name[OS_MAX_API_NAME]; + osal_id_t creator; + uint32 stack_size; + osal_priority_t priority; } OS_task_prop_t; /** @brief OSAL queue properties */ @@ -617,8 +631,8 @@ int32 OS_RegisterEventHandler(OS_EventHandler_t handler); * @retval #OS_ERR_NAME_TAKEN if the name specified is already used by a task * @retval #OS_ERROR if an unspecified/other error occurs */ -int32 OS_TaskCreate(osal_id_t *task_id, const char *task_name, osal_task_entry function_pointer, uint32 *stack_pointer, - uint32 stack_size, uint32 priority, uint32 flags); +int32 OS_TaskCreate(osal_id_t *task_id, const char *task_name, osal_task_entry function_pointer, osal_stackptr_t stack_pointer, + uint32 stack_size, osal_priority_t priority, uint32 flags); /*-------------------------------------------------------------------------------------*/ /** @@ -687,7 +701,7 @@ int32 OS_TaskDelay(uint32 millisecond); * @retval #OS_ERR_INVALID_PRIORITY if the priority is greater than the max allowed * @retval #OS_ERROR if the OS call to change the priority fails */ -int32 OS_TaskSetPriority(osal_id_t task_id, uint32 new_priority); +int32 OS_TaskSetPriority(osal_id_t task_id, osal_priority_t new_priority); /*-------------------------------------------------------------------------------------*/ /** diff --git a/src/os/shared/inc/os-shared-task.h b/src/os/shared/inc/os-shared-task.h index 4adb87fe2..5976519e3 100644 --- a/src/os/shared/inc/os-shared-task.h +++ b/src/os/shared/inc/os-shared-task.h @@ -35,11 +35,11 @@ typedef struct { char task_name[OS_MAX_API_NAME]; uint32 stack_size; - uint32 priority; + osal_priority_t priority; osal_task_entry entry_function_pointer; osal_task_entry delete_hook_pointer; void * entry_arg; - uint32 * stack_pointer; + osal_stackptr_t stack_pointer; } OS_task_internal_record_t; /* diff --git a/src/os/shared/src/osapi-task.c b/src/os/shared/src/osapi-task.c index 05817443e..e0ff72c36 100644 --- a/src/os/shared/src/osapi-task.c +++ b/src/os/shared/src/osapi-task.c @@ -186,19 +186,13 @@ int32 OS_TaskAPI_Init(void) * See description in API and header file for detail * *-----------------------------------------------------------------*/ -int32 OS_TaskCreate(osal_id_t *task_id, const char *task_name, osal_task_entry function_pointer, uint32 *stack_pointer, - uint32 stack_size, uint32 priority, uint32 flags) +int32 OS_TaskCreate(osal_id_t *task_id, const char *task_name, osal_task_entry function_pointer, + osal_stackptr_t stack_pointer, uint32 stack_size, osal_priority_t priority, uint32 flags) { OS_common_record_t *record; int32 return_code; uint32 local_id; - /* Check for bad priority */ - if (priority > OS_MAX_TASK_PRIORITY) - { - return OS_ERR_INVALID_PRIORITY; - } - /* Check for NULL pointers */ if (task_name == NULL || task_id == NULL || function_pointer == NULL) { @@ -331,33 +325,26 @@ int32 OS_TaskDelay(uint32 millisecond) * See description in API and header file for detail * *-----------------------------------------------------------------*/ -int32 OS_TaskSetPriority(osal_id_t task_id, uint32 new_priority) +int32 OS_TaskSetPriority(osal_id_t task_id, osal_priority_t new_priority) { OS_common_record_t *record; int32 return_code; uint32 local_id; - if (new_priority > OS_MAX_TASK_PRIORITY) - { - return_code = OS_ERR_INVALID_PRIORITY; - } - else + return_code = OS_ObjectIdGetById(OS_LOCK_MODE_GLOBAL, LOCAL_OBJID_TYPE, task_id, &local_id, &record); + if (return_code == OS_SUCCESS) { - return_code = OS_ObjectIdGetById(OS_LOCK_MODE_GLOBAL, LOCAL_OBJID_TYPE, task_id, &local_id, &record); + return_code = OS_TaskSetPriority_Impl(local_id, new_priority); + if (return_code == OS_SUCCESS) { - return_code = OS_TaskSetPriority_Impl(local_id, new_priority); - - if (return_code == OS_SUCCESS) - { - /* Use the abstracted priority, not the OS one */ - /* Change the priority in the table as well */ - OS_task_table[local_id].priority = new_priority; - } - - /* Unlock the global from OS_ObjectIdGetAndLock() */ - OS_Unlock_Global(LOCAL_OBJID_TYPE); + /* Use the abstracted priority, not the OS one */ + /* Change the priority in the table as well */ + OS_task_table[local_id].priority = new_priority; } + + /* Unlock the global from OS_ObjectIdGetAndLock() */ + OS_Unlock_Global(LOCAL_OBJID_TYPE); } return return_code; diff --git a/src/unit-test-coverage/shared/src/coveragetest-task.c b/src/unit-test-coverage/shared/src/coveragetest-task.c index f1e640c6d..538028246 100644 --- a/src/unit-test-coverage/shared/src/coveragetest-task.c +++ b/src/unit-test-coverage/shared/src/coveragetest-task.c @@ -113,8 +113,6 @@ void Test_OS_TaskCreate(void) OSAPI_TEST_FUNCTION_RC(OS_TaskCreate(NULL, NULL, NULL, NULL, 0, 0, 0), OS_INVALID_POINTER); OSAPI_TEST_FUNCTION_RC(OS_TaskCreate(&objid, "UT", UT_TestHook, NULL, 0, 0, 0), OS_ERROR); - OSAPI_TEST_FUNCTION_RC(OS_TaskCreate(&objid, "UT", UT_TestHook, NULL, 128, 10 + OS_MAX_TASK_PRIORITY, 0), - OS_ERR_INVALID_PRIORITY); UT_SetForceFail(UT_KEY(OCS_strlen), 10 + OS_MAX_API_NAME); OSAPI_TEST_FUNCTION_RC(OS_TaskCreate(&objid, "UT", UT_TestHook, NULL, 128, 0, 0), OS_ERR_NAME_TOO_LONG); } @@ -183,8 +181,6 @@ void Test_OS_TaskSetPriority(void) int32 actual = OS_TaskSetPriority(UT_OBJID_1, 1); UtAssert_True(actual == expected, "OS_TaskSetPriority() (%ld) == OS_SUCCESS", (long)actual); - - OSAPI_TEST_FUNCTION_RC(OS_TaskSetPriority(UT_OBJID_1, 10 + OS_MAX_TASK_PRIORITY), OS_ERR_INVALID_PRIORITY); } void Test_OS_TaskRegister(void) { @@ -256,7 +252,7 @@ void Test_OS_TaskGetInfo(void) utrec.creator = UT_OBJID_OTHER; utrec.name_entry = "ABC"; OS_task_table[1].stack_size = 222; - OS_task_table[1].priority = 333; + OS_task_table[1].priority = 133; UT_SetDataBuffer(UT_KEY(OS_ObjectIdGetById), &local_index, sizeof(local_index), false); UT_SetDataBuffer(UT_KEY(OS_ObjectIdGetById), &rptr, sizeof(rptr), false); actual = OS_TaskGetInfo(UT_OBJID_1, &task_prop); @@ -266,7 +262,7 @@ void Test_OS_TaskGetInfo(void) UtAssert_True(strcmp(task_prop.name, "ABC") == 0, "task_prop.name (%s) == ABC", task_prop.name); UtAssert_True(task_prop.stack_size == 222, "task_prop.stack_size (%lu) == 222", (unsigned long)task_prop.stack_size); - UtAssert_True(task_prop.priority == 333, "task_prop.priority (%lu) == 333", (unsigned long)task_prop.priority); + UtAssert_True(task_prop.priority == 133, "task_prop.priority (%lu) == 133", (unsigned long)task_prop.priority); OS_task_table[1].stack_size = 0; OS_task_table[1].priority = 0; diff --git a/src/unit-tests/oscore-test/ut_oscore_task_test.c b/src/unit-tests/oscore-test/ut_oscore_task_test.c index 7f0cd5ae3..0cc4052cd 100644 --- a/src/unit-tests/oscore-test/ut_oscore_task_test.c +++ b/src/unit-tests/oscore-test/ut_oscore_task_test.c @@ -37,9 +37,6 @@ #define UT_TASK_STACK_SIZE 0x2000 #define UT_TASK_PRIORITY 111 -/* This is not global in the OSAL */ -#define MAX_PRIORITY 255 - /*--------------------------------------------------------------------------------* ** Data types **--------------------------------------------------------------------------------*/ @@ -167,16 +164,6 @@ void UT_os_task_create_test() else UT_OS_TEST_RESULT(testDesc, UTASSERT_CASETYPE_FAILURE); - /*-----------------------------------------------------*/ - testDesc = "#5 Invalid-priority"; - - res = OS_TaskCreate(&g_task_ids[5], g_task_names[5], generic_test_task, g_task_stacks[5], UT_TASK_STACK_SIZE, - MAX_PRIORITY + 1, 0); - if (res == OS_ERR_INVALID_PRIORITY) - UT_OS_TEST_RESULT(testDesc, UTASSERT_CASETYPE_PASS); - else - UT_OS_TEST_RESULT(testDesc, UTASSERT_CASETYPE_FAILURE); - /*-----------------------------------------------------*/ testDesc = "#6 No-free-IDs"; @@ -624,12 +611,6 @@ void UT_os_task_set_priority_test() } else { - res = OS_TaskSetPriority(g_task_ids[2], MAX_PRIORITY + 1); - if (res == OS_ERR_INVALID_PRIORITY) - UT_OS_TEST_RESULT(testDesc, UTASSERT_CASETYPE_PASS); - else - UT_OS_TEST_RESULT(testDesc, UTASSERT_CASETYPE_FAILURE); - /* Delay to let child task run */ OS_TaskDelay(500); diff --git a/src/ut-stubs/osapi-utstub-task.c b/src/ut-stubs/osapi-utstub-task.c index 097cf8083..977e68708 100644 --- a/src/ut-stubs/osapi-utstub-task.c +++ b/src/ut-stubs/osapi-utstub-task.c @@ -52,8 +52,8 @@ UT_DEFAULT_STUB(OS_TaskAPI_Init, (void)) ** Returns either OS_SUCCESS or OS_ERROR. ** ******************************************************************************/ -int32 OS_TaskCreate(osal_id_t *task_id, const char *task_name, osal_task_entry function_pointer, uint32 *stack_pointer, - uint32 stack_size, uint32 priority, uint32 flags) +int32 OS_TaskCreate(osal_id_t *task_id, const char *task_name, osal_task_entry function_pointer, + osal_stackptr_t stack_pointer, uint32 stack_size, osal_priority_t priority, uint32 flags) { UT_Stub_RegisterContext(UT_KEY(OS_TaskCreate), task_id); UT_Stub_RegisterContext(UT_KEY(OS_TaskCreate), task_name); @@ -165,7 +165,7 @@ int32 OS_TaskDelay(uint32 millisecond) * Stub function for OS_TaskSetPriority() * *****************************************************************************/ -int32 OS_TaskSetPriority(osal_id_t task_id, uint32 new_priority) +int32 OS_TaskSetPriority(osal_id_t task_id, osal_priority_t new_priority) { UT_Stub_RegisterContextGenericArg(UT_KEY(OS_TaskSetPriority), task_id); UT_Stub_RegisterContextGenericArg(UT_KEY(OS_TaskSetPriority), new_priority);