Fix nasa#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.
jphickey · Oct 29, 2020 · e77702b · e77702b
1 parent 5a8f0af
commit e77702b
Show file tree

Hide file tree

Showing 6 changed files with 41 additions and 63 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);